Skip to content

Commit

Permalink
Make notice messages not an instance of Error
Browse files Browse the repository at this point in the history
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
  • Loading branch information
brianc committed Jan 30, 2020
1 parent 94fbb24 commit 9b8b7ec
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 13 deletions.
8 changes: 4 additions & 4 deletions packages/pg/lib/connection.js
Original file line number Diff line number Diff line change
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
25 changes: 16 additions & 9 deletions packages/pg/test/integration/client/notice-tests.js
Original file line number Diff line number Diff line change
@@ -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,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
$$;
`
Expand All @@ -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()
})
})

0 comments on commit 9b8b7ec

Please sign in to comment.