Skip to content

Commit

Permalink
Make notice messages not an instance of Error (#2090)
Browse files Browse the repository at this point in the history
* 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

* skip notice test in travis

* Pin node@13.6 for regression in async iterators

* Check and see if node 13.8 is still borked on async iterator

* Yeah, node still has changed edge case behavior on stream

* Emit notice messages on travis
  • Loading branch information
brianc committed Feb 19, 2020
1 parent 94fbb24 commit 1d48051
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 16 deletions.
2 changes: 1 addition & 1 deletion .travis.yml
Expand Up @@ -10,7 +10,7 @@ env:
node_js:
- lts/dubnium
- lts/erbium
- 13
- 13.6

addons:
postgresql: "10"
Expand Down
8 changes: 4 additions & 4 deletions packages/pg/lib/connection.js
Expand Up @@ -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') {
Expand All @@ -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
Expand All @@ -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
}
Expand Down
32 changes: 21 additions & 11 deletions 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
Expand All @@ -32,25 +33,34 @@ 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
$$;
`
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)
// 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()
})
})

0 comments on commit 1d48051

Please sign in to comment.