From 9b8b7ec204d0dc0ca8b31101b47e47d18bd83587 Mon Sep 17 00:00:00 2001 From: "Brian M. Carlson" Date: Thu, 30 Jan 2020 09:55:07 -0600 Subject: [PATCH 1/6] Make notice messages not an instance of Error Slight API cleanup to make a notice instance the same shape as it was, but not be an instance of error. This is a backwards incompatible change though I expect the impact to be minimal. Closes #1982 --- packages/pg/lib/connection.js | 8 +++--- .../test/integration/client/notice-tests.js | 25 ++++++++++++------- 2 files changed, 20 insertions(+), 13 deletions(-) diff --git a/packages/pg/lib/connection.js b/packages/pg/lib/connection.js index 435c1a965..ca904432c 100644 --- a/packages/pg/lib/connection.js +++ b/packages/pg/lib/connection.js @@ -597,7 +597,7 @@ Connection.prototype._readValue = function (buffer) { } // parses error -Connection.prototype.parseE = function (buffer, length) { +Connection.prototype.parseE = function (buffer, length, isNotice) { var fields = {} var fieldType = this.readString(buffer, 1) while (fieldType !== '\0') { @@ -606,10 +606,10 @@ Connection.prototype.parseE = function (buffer, length) { } // the msg is an Error instance - var msg = new Error(fields.M) + var msg = isNotice ? { message: fields.M } : new Error(fields.M) // for compatibility with Message - msg.name = 'error' + msg.name = isNotice ? 'notice' : 'error' msg.length = length msg.severity = fields.S @@ -633,7 +633,7 @@ Connection.prototype.parseE = function (buffer, length) { // same thing, different name Connection.prototype.parseN = function (buffer, length) { - var msg = this.parseE(buffer, length) + var msg = this.parseE(buffer, length, true) msg.name = 'notice' return msg } diff --git a/packages/pg/test/integration/client/notice-tests.js b/packages/pg/test/integration/client/notice-tests.js index f3dc5090e..6f1441c69 100644 --- a/packages/pg/test/integration/client/notice-tests.js +++ b/packages/pg/test/integration/client/notice-tests.js @@ -1,12 +1,13 @@ 'use strict' -var helper = require('./test-helper') +const helper = require('./test-helper') +const assert = require('assert') const suite = new helper.Suite() suite.test('emits notify message', function (done) { - var client = helper.client() + const client = helper.client() client.query('LISTEN boom', assert.calls(function () { - var otherClient = helper.client() - var bothEmitted = -1 + const otherClient = helper.client() + let bothEmitted = -1 otherClient.query('LISTEN boom', assert.calls(function () { assert.emits(client, 'notification', function (msg) { // make sure PQfreemem doesn't invalidate string pointers @@ -32,17 +33,17 @@ suite.test('emits notify message', function (done) { }) // this test fails on travis due to their config -suite.test('emits notice message', false, function (done) { +suite.test('emits notice message', function (done) { if (helper.args.native) { - console.error('need to get notice message working on native') + console.error('notice messages do not work curreintly with node-libpq') return done() } - // TODO this doesn't work on all versions of postgres - var client = helper.client() + + const client = helper.client() const text = ` DO language plpgsql $$ BEGIN - RAISE NOTICE 'hello, world!'; + RAISE NOTICE 'hello, world!' USING ERRCODE = '23505', DETAIL = 'this is a test'; END $$; ` @@ -51,6 +52,12 @@ $$; }) assert.emits(client, 'notice', function (notice) { assert.ok(notice != null) + // notice messages should not be error instances + assert(notice instanceof Error === false) + assert.strictEqual(notice.name, 'notice') + assert.strictEqual(notice.message, 'hello, world!') + assert.strictEqual(notice.detail, 'this is a test') + assert.strictEqual(notice.code, '23505') done() }) }) From 305a83244ae2bdd0d6577f6c807e943a0e89d87d Mon Sep 17 00:00:00 2001 From: "Brian M. Carlson" Date: Thu, 30 Jan 2020 10:14:15 -0600 Subject: [PATCH 2/6] skip notice test in travis --- packages/pg/test/integration/client/notice-tests.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/pg/test/integration/client/notice-tests.js b/packages/pg/test/integration/client/notice-tests.js index 6f1441c69..8edabee7c 100644 --- a/packages/pg/test/integration/client/notice-tests.js +++ b/packages/pg/test/integration/client/notice-tests.js @@ -32,8 +32,11 @@ suite.test('emits notify message', function (done) { })) }) +const isInTravis = process.env.CI === 'true' +const skip = !isInTravis + // this test fails on travis due to their config -suite.test('emits notice message', function (done) { +suite.test('emits notice message', skip, function (done) { if (helper.args.native) { console.error('notice messages do not work curreintly with node-libpq') return done() From ffdb3fb36afdfd9d8265843901b9bb0c9c5f63ad Mon Sep 17 00:00:00 2001 From: "Brian M. Carlson" Date: Thu, 30 Jan 2020 10:30:22 -0600 Subject: [PATCH 3/6] Pin node@13.6 for regression in async iterators --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 61a7a79af..7ea347e9b 100644 --- a/.travis.yml +++ b/.travis.yml @@ -10,7 +10,7 @@ env: node_js: - lts/dubnium - lts/erbium - - 13 + - 13.6 addons: postgresql: "10" From 80c046dc4f8c21d2d0d2cfee5b80cbd9d15d8585 Mon Sep 17 00:00:00 2001 From: "Brian M. Carlson" Date: Wed, 12 Feb 2020 08:28:58 -0800 Subject: [PATCH 4/6] Check and see if node 13.8 is still borked on async iterator --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 7ea347e9b..61a7a79af 100644 --- a/.travis.yml +++ b/.travis.yml @@ -10,7 +10,7 @@ env: node_js: - lts/dubnium - lts/erbium - - 13.6 + - 13 addons: postgresql: "10" From c1f69c336b0fffd1f6125a77d0e9aa030dba4b81 Mon Sep 17 00:00:00 2001 From: "Brian M. Carlson" Date: Wed, 12 Feb 2020 08:44:13 -0800 Subject: [PATCH 5/6] Yeah, node still has changed edge case behavior on stream --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 61a7a79af..7ea347e9b 100644 --- a/.travis.yml +++ b/.travis.yml @@ -10,7 +10,7 @@ env: node_js: - lts/dubnium - lts/erbium - - 13 + - 13.6 addons: postgresql: "10" From 17266c1804b99594ca208bfa8901f90baa897fc0 Mon Sep 17 00:00:00 2001 From: brianc Date: Fri, 14 Feb 2020 08:34:33 -0800 Subject: [PATCH 6/6] Emit notice messages on travis --- packages/pg/test/integration/client/notice-tests.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/pg/test/integration/client/notice-tests.js b/packages/pg/test/integration/client/notice-tests.js index 8edabee7c..a6fc8a56f 100644 --- a/packages/pg/test/integration/client/notice-tests.js +++ b/packages/pg/test/integration/client/notice-tests.js @@ -32,11 +32,8 @@ suite.test('emits notify message', function (done) { })) }) -const isInTravis = process.env.CI === 'true' -const skip = !isInTravis - // this test fails on travis due to their config -suite.test('emits notice message', skip, function (done) { +suite.test('emits notice message', function (done) { if (helper.args.native) { console.error('notice messages do not work curreintly with node-libpq') return done() @@ -50,8 +47,11 @@ BEGIN END $$; ` - client.query(text, () => { - client.end() + client.query('SET SESSION client_min_messages=notice', (err) => { + assert.ifError(err) + client.query(text, () => { + client.end() + }) }) assert.emits(client, 'notice', function (notice) { assert.ok(notice != null)