Repository navigation
Standardizing on a single error type #972
Description
Hi Brian - we just got bit by #938 and I am hoping to fix the issue.
Per your comment here:
This is probably something that should be normalized within node-postgres itself rather than here in the native lib. If you wanna take a crack at it over there I'll happily review the PR!
It sounds like your proposed solution is to keep the node-postgres error API consistent, and transform error objects coming from pg-native, instead of:
- changing the error objects here to match the ones in the pg-native library
- changing the error objects in pg-native to match the error here.
does that sound right? i'll start working on a patch.
Activity
vitaly-t commentedon Mar 29, 2016
Aligning error reporting between
pgandpg.nativewould be a substantial breaking change.In fact, I think it would be so serious, the library would need to change over to version 5.0.0 to accommodate the change, or else it will break the hell of people's code.
If we are to try this within 4.x branch, this universal error reporting should be made optional, off by default.
kevinburkeshyp commentedon Mar 29, 2016
I also think this would require a major version bump. I think it's worth it, though, especially when the library advertises that
'pg-native'is a drop-in replacement.vitaly-t commentedon Mar 29, 2016
Unfortunately, this isn't true, and not just because of the error handling, but some most important modules, like pg-query-stream do not even support the native bindings. In fact, last I tried - the connection pool didn't work for me, so I gave up on the native version long ago.
The differences are too substantial.
kevinburkeshyp commentedon Mar 29, 2016
can you elaborate on this?
vitaly-t commentedon Mar 29, 2016
It just doesn't work at all, what's more elaborate than that? :)Remove pg-native
Remove pg-native
vitaly-t commentedon Apr 6, 2016
@kevinburkeshyp I'd like to retract my previous statement here.
See: brianc/node-pg-native#46
vitaly-t commentedon Apr 6, 2016
After having added
pg-nativesupport into pg-promise, I could see how many differences do exist, especially within the area of errors reporting. In fact, errors are completely different.And if someone uses errors and moves from one version of the library to another - that will be a pain.
vitaly-t commentedon Apr 9, 2016
@kevinburke any progress here? I think this will make one useful addition to the library.
I've done a lot of implementation for custom errors. See here some nice code: https://github.com/vitaly-t/pg-promise/blob/master/lib/errors.js
In the same way, you would have:
toStringto provide nice formatting for string concatenationinspectto return.toString(), to provide the same nice output into the console.just to give you a good example of how to write custom errors for Node.js ;)
vitaly-t commentedon Apr 9, 2016
@kevinburkeshyp if someone decides to implement this feature, the following projects should be consulted: pg-rethrow + pg-error-codes.
@brianc There seems to be a lot of helpers around to unite all this under one roof and make consistent across the entire platform. Would you consider a PR for this? If so, please label it as a feature request.
rkaw92 commentedon Jul 12, 2016
Got bitten by this peculiarity today after replacing
require('pg')withrequire('pg').native. I had had some error handlers that checked theerror.codeproperty for known values, because the code expected some tables to already exist and treated a failedCREATE TABLElike a nominal case, but only for specified error codes ("SQL states").Now I place
error.code === '42P07' || error.sqlState === '42P07'in my handlers...brianc commentedon Jun 6, 2017
Since this is a breaking change I needed to hold off until doing a new major version bump. I've added this to the
pg@7.0milestone so we can fix it & normalize these in the new release.Normalize native error properties
Remove fallbacks for unsupported Node versions (#1304) (#1313)
brianc commentedon Jun 9, 2017
I have this fixed on the 7.0 branch. Fix will be released in 7.0.
Remove fallbacks for unsupported Node versions (#1304) (#1313)