Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make notice messages not an instance of Error #2090

Merged
merged 6 commits into from Feb 19, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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()
})
})