From 1d480517fe69e5bb15fa04618c6d8e377ee86e4c Mon Sep 17 00:00:00 2001
From: Brian C <brian.m.carlson@gmail.com>
Date: Wed, 19 Feb 2020 12:16:17 -0600
Subject: [PATCH] Make notice messages not an instance of Error (#2090)

* 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
---
 .travis.yml                                   |  2 +-
 packages/pg/lib/connection.js                 |  8 ++---
 .../test/integration/client/notice-tests.js   | 32 ++++++++++++-------
 3 files changed, 26 insertions(+), 16 deletions(-)

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"
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..a6fc8a56f 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,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()
   })
 })