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

Attach the original Postgres error to unique errors #180

Merged
merged 1 commit into from Aug 20, 2015

Conversation

4 participants
@kevinburkeshyp
Copy link

kevinburkeshyp commented Aug 15, 2015

Currently it gets clobbered, which makes it difficult to determine which
uniqueness constraint failed. This way, you can always check err.originalError
to get the original node-postgres error message, with information about the
constraint, PG error code and table that failed.

Attach the original Postgres error to unique errors
Currently it gets clobbered, which makes it difficult to determine which
uniqueness constraint failed. This way, you can always check err.originalError
to get the original node-postgres error message, with information about the
constraint, PG error code and table that failed.
@kevinburkeshyp

This comment has been minimized.

Copy link
Author

kevinburkeshyp commented Aug 15, 2015

Tried to add a test here, balderdashy/waterline-adapter-tests#89, though not sure whether you can write db-specific tests there. I have tests for this in my own code

@kevinburkeshyp

This comment has been minimized.

Copy link
Author

kevinburkeshyp commented Aug 15, 2015

Looks like this is failing on a different test, I'm not sure why. https://travis-ci.org/balderdashy/sails-postgresql/jobs/75692004#L849

(coincidentally, you can see an example of https://github.com/balderdashy/waterline/issues/1118 in the error message)

@kevinburkeshyp

This comment has been minimized.

Copy link
Author

kevinburkeshyp commented Aug 15, 2015

just realized if people are using blueprints this may not be the best idea, since it might expose database implementation details to the end user ... ? i don't know exactly how blueprints work.

@dmarcelino

This comment has been minimized.

Copy link
Member

dmarcelino commented Aug 15, 2015

The changes look good but awkwardly travis build with node 0.11 is breaking, https://travis-ci.org/balderdashy/sails-postgresql/jobs/75692004#L849:

  1) autoIncrement attribute feature should auto generate unique values even when values have been set:
     [Error (E_VALIDATION) 1 attribute is invalid] Invalid attributes sent to undefined:

Not really sure what's going on 😕

@kevinburkeshyp

This comment has been minimized.

Copy link
Author

kevinburkeshyp commented Aug 17, 2015

Might be a race issue since I just re-ran the tests and they suddenly pass. I'm not sure what's going on; I believe we don't get a full stack trace because an E_VALIDATION error object does not inherit from Error.

We had a bunch of issues with create([array of records]) returning results out of order; I'm not sure if that's an issue here. https://github.com/balderdashy/sails-postgresql/issues/128

@kevinburkeshyp

This comment has been minimized.

Copy link
Author

kevinburkeshyp commented Aug 17, 2015

Reading through the code in sails-postgresql maybe it's possible it's generating/incrementing ID's in memory? Not super familiar with that code, but that seems like it could be race-y.

Sorry I don't have a huge amount of time to dig into it :(

particlebanana added a commit that referenced this pull request Aug 20, 2015

Merge pull request #180 from Shyp/pass-thru-error
Attach the original Postgres error to unique errors

@particlebanana particlebanana merged commit fbaf410 into balderdashy:master Aug 20, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.