diff --git a/packages/pg-native/index.js b/packages/pg-native/index.js index 451daeb7d..442efb1ab 100644 --- a/packages/pg-native/index.js +++ b/packages/pg-native/index.js @@ -188,23 +188,23 @@ Client.prototype._emitResult = function (pq) { case 'PGRES_TUPLES_OK': case 'PGRES_COMMAND_OK': case 'PGRES_EMPTY_QUERY': + case 'PGRES_COPY_OUT': + case 'PGRES_COPY_IN': + case 'PGRES_COPY_BOTH': { const result = this._consumeQueryResults(this.pq) this.emit('result', result) } break - case 'PGRES_COPY_OUT': - case 'PGRES_COPY_BOTH': { - break - } - case 'PGRES_PIPELINE_SYNC': case 'PGRES_PIPELINE_ABORTED': break default: - this._readError('unrecognized command status: ' + status) + this._queryError = new Error( + this.pq.resultErrorMessage() || this.pq.errorMessage() || 'unrecognized command status: ' + status + ) break } return status diff --git a/packages/pg-native/test/unhandled-result-status.js b/packages/pg-native/test/unhandled-result-status.js new file mode 100644 index 000000000..910f89f86 --- /dev/null +++ b/packages/pg-native/test/unhandled-result-status.js @@ -0,0 +1,82 @@ +const assert = require('assert') +const EventEmitter = require('events').EventEmitter +const types = require('pg-types') +const Client = require('../') + +function stubClient(pq) { + const client = Object.create(Client.prototype) + EventEmitter.call(client) + client.pq = pq + client._types = types + client.arrayMode = false + client._resultCount = 0 + client._queryError = undefined + client._results = undefined + client._rows = undefined + client.on('result', client._onResult.bind(client)) + return client +} + +function emptyResultPq(status, errorMessage) { + return { + resultStatus: () => status, + resultErrorMessage: () => errorMessage || '', + errorMessage: () => errorMessage || '', + cmdStatus: () => 'COPY 0', + cmdTuples: () => '0', + nfields: () => 0, + ntuples: () => 0, + } +} + +describe('unhandled libpq result statuses', () => { + it('emits a Result for COPY_OUT so the query callback is not left with undefined', (done) => { + const client = stubClient(emptyResultPq('PGRES_COPY_OUT')) + client._queryCallback = (err, rows, results) => { + assert.ifError(err) + assert.deepEqual(rows, []) + assert(results) + assert.deepEqual(results.rows, []) + done() + } + client._emitResult(client.pq) + client._onReadyForQuery() + }) + + it('emits a Result for COPY_IN', (done) => { + const client = stubClient(emptyResultPq('PGRES_COPY_IN')) + client._queryCallback = (err, rows, results) => { + assert.ifError(err) + assert(results) + assert.deepEqual(results.rows, []) + done() + } + client._emitResult(client.pq) + client._onReadyForQuery() + }) + + it('passes unrecognized statuses to the query callback instead of the client error event', (done) => { + const client = stubClient(emptyResultPq('PGRES_WEIRD', 'server said no')) + client.on('error', () => { + done(new Error('should not emit error on the client')) + }) + client._queryCallback = (err) => { + assert(err instanceof Error) + assert.match(err.message, /server said no/) + done() + } + client._emitResult(client.pq) + client._onReadyForQuery() + }) + + it('falls back to the status name when libpq has no error message', (done) => { + const client = stubClient(emptyResultPq('PGRES_WEIRD')) + client._queryCallback = (err) => { + assert(err instanceof Error) + assert.match(err.message, /unrecognized command status: PGRES_WEIRD/) + done() + } + client._emitResult(client.pq) + client._onReadyForQuery() + }) +}) diff --git a/packages/pg/lib/native/query.js b/packages/pg/lib/native/query.js index 8cb561979..105211f06 100644 --- a/packages/pg/lib/native/query.js +++ b/packages/pg/lib/native/query.js @@ -99,6 +99,12 @@ NativeQuery.prototype.submit = function (client) { return self.handleError(err) } + // COPY and other unhandled libpq statuses used to complete with no result + // object. Treat that as an error so callers never see success plus undefined. + if (results == null) { + return self.handleError(new Error('Native query completed without a result')) + } + // emit row events for each row in the result if (self._emitRowEvents) { if (results.length > 1) { diff --git a/packages/pg/test/unit/native-query-missing-result-tests.js b/packages/pg/test/unit/native-query-missing-result-tests.js new file mode 100644 index 000000000..320d2731d --- /dev/null +++ b/packages/pg/test/unit/native-query-missing-result-tests.js @@ -0,0 +1,48 @@ +'use strict' +const helper = require('./test-helper') +const assert = require('assert') +const NativeQuery = require('../../lib/native/query') +const suite = new helper.Suite() +const test = suite.test.bind(suite) + +function submitWithNativeResult(nativeCbArgs, queryCb) { + const query = new NativeQuery({ text: 'SELECT 1', callback: queryCb }) + const client = { + namedQueries: {}, + native: { + arrayMode: false, + pq: { resultErrorFields: () => null }, + query: function (text, valuesOrCb, maybeCb) { + const cb = typeof valuesOrCb === 'function' ? valuesOrCb : maybeCb + setImmediate(() => cb.apply(null, nativeCbArgs)) + }, + }, + } + query.submit(client) +} + +test('missing native result is an error, not success with undefined', (done) => { + submitWithNativeResult([null, [], undefined], (err, res) => { + assert(err instanceof Error) + assert.strictEqual(res, undefined) + assert.match(err.message, /without a result/) + done() + }) +}) + +test('normal native result still succeeds', (done) => { + const result = { rows: [{ n: 1 }], fields: [], command: 'SELECT', rowCount: 1 } + submitWithNativeResult([null, result.rows, result], (err, res) => { + assert.ifError(err) + assert.strictEqual(res, result) + done() + }) +}) + +test('native query error is still forwarded', (done) => { + const boom = new Error('boom') + submitWithNativeResult([boom], (err) => { + assert.strictEqual(err, boom) + done() + }) +})