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

Notices are Errors, not Messages #1982

Closed
ab-pm opened this issue Sep 26, 2019 · 3 comments
Closed

Notices are Errors, not Messages #1982

ab-pm opened this issue Sep 26, 2019 · 3 comments
Labels
Milestone

Comments

@ab-pm
Copy link

ab-pm commented Sep 26, 2019

(related to #1735)

I think parseN/parseE are broken

// same thing, different name
Connection.prototype.parseN = function (buffer, length) {
var msg = this.parseE(buffer, length)
msg.name = 'notice'
return msg
}
// 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.

@charmander charmander added the bug label Sep 27, 2019
@charmander
Copy link
Collaborator

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 – length is meaningless to library users, and name almost so.

@charmander
Copy link
Collaborator

charmander commented 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.)

@charmander charmander added this to the pg@8.0 milestone Jan 30, 2020
@brianc
Copy link
Owner

brianc commented Jan 30, 2020

Good idea!

brianc added a commit that referenced this issue Jan 30, 2020
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
brianc added a commit that referenced this issue Jan 30, 2020
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
brianc added a commit that referenced this issue Feb 19, 2020
* 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
@brianc brianc closed this as completed in aafd8ac Mar 30, 2020
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants