Repository navigation
Closed
Labels
Milestone
Description
(related to #1735)
I think parseN/parseE are broken
node-postgres/lib/connection.js
Lines 635 to 640 in e4578d2
| // same thing, different name | |
| Connection.prototype.parseN = function (buffer, length) { | |
| var msg = this.parseE(buffer, length) | |
| msg.name = 'notice' | |
| return msg | |
| } |
node-postgres/lib/connection.js
Lines 592 to 604 in e4578d2
| // parses error | |
| Connection.prototype.parseE = function (buffer, length) { | |
| var fields = {} | |
| var msg, item | |
| var input = new Message('error', length) | |
| var fieldType = this.readString(buffer, 1) | |
| while (fieldType !== '\0') { | |
| fields[fieldType] = this.parseCString(buffer) | |
| fieldType = this.readString(buffer, 1) | |
| } | |
| if (input.name === 'error') { | |
| // the msg is an Error instance | |
| msg = new Error(fields.M) |
It looks like the one should create a Message and the other should create an Error, depending on the .name. However, parseE does always construct the input message with .name === 'error', so the else path is never hit.
Activity
charmander commentedon Dec 5, 2019
Rediscovered this for #2020. I don’t think the change was intentional, but it’s existed since 2.4.0 so it’ll have to stay that way in 7.x at least. It doesn’t really make sense to expose
Message-compatible objects either –lengthis meaningless to library users, andnamealmost so.charmander commentedon Jan 30, 2020
Interested in changing this as part of 8.0, @brianc? (Adding it to the milestone so it’s not forgotten, but it doesn’t have to stay there.)
brianc commentedon Jan 30, 2020
Good idea!
Make notice messages not an instance of Error
Make notice messages not an instance of Error
Make notice messages not an instance of Error (#2090)
8.0 Release (brianc#2117)