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

Export DatabaseError in pg package #2378

Closed
golergka opened this issue Oct 12, 2020 · 3 comments · Fixed by #2445
Closed

Export DatabaseError in pg package #2378

golergka opened this issue Oct 12, 2020 · 3 comments · Fixed by #2445

Comments

@golergka
Copy link

I wanted to catch errors I got from pg in order to improve my project's reporting, but found out that I needed to import pg-protocol package to get DatabaseError type.

This seems a little bit illogical: I think that if I interact with a package, it should export not only the type definitions, but also the errors it's going to throw up to me.

Another reason to do this is that now I have both node-postgres and pg-protocol in my package.json, and, for example, if I update node-postgres to a new version and forget to update pg-protocol, the actual runtime type of the DatabaseError I catch may change under me, and my code that's depending on the DatabaseError type imported from outdated pg-protocol package would silently cease to work.

I also wanted to point out that documentation seems to be a bit lacking on that regard, and doesn't mention any errors that I should be catching as a user of pg package.

If you approve such changes in principle, I would be happy to contribute PR implementing them, both re-export and docs (will create a separate issue to track docs progress).

@brianc
Copy link
Owner

brianc commented Oct 12, 2020

so you want the main pg package to export DatabaseError? that sounds reasonable

@fabienwarniez
Copy link

I am trying to read the code property of the error object returned when something fails, how can I do that in typescript using pg? Is this what this issue is referring to? Is there a workaround in the mean time?

cakoose added a commit to cakoose/node-postgres that referenced this issue Jan 17, 2021
Before, users would have to import DatabaseError from 'pg-protocol'.  If
there are multiple versions of 'pg-protocol', you might end up using the
wrong one.

Closes brianc#2378
@cakoose
Copy link
Contributor

cakoose commented Jan 17, 2021

@fabienwarniez: Right now, you can do this:

import {DatabaseError} from 'pg-protocol';

try {
    ...
} catch (err) {
    if (err instanceof DatabaseProtocol and err.code === '...') {
        ...
    }
    throw err;
}

This issue is to allow importing DatabaseError from 'pg' instead of 'pg-protocol'.

cakoose added a commit to cakoose/node-postgres that referenced this issue Jan 18, 2021
Before, users would have to import DatabaseError from 'pg-protocol'.  If
there are multiple versions of 'pg-protocol', you might end up using the
wrong one.

Closes brianc#2378
cakoose added a commit to cakoose/node-postgres that referenced this issue Jan 18, 2021
Before, users would have to import DatabaseError from 'pg-protocol'.  If
there are multiple versions of 'pg-protocol', you might end up using the
wrong one.

Closes brianc#2378
cakoose added a commit to cakoose/node-postgres that referenced this issue Jan 18, 2021
Before, users would have to import DatabaseError from 'pg-protocol'.  If
there are multiple versions of 'pg-protocol', you might end up using the
wrong one.

Closes brianc#2378
brianc added a commit that referenced this issue Mar 22, 2021
* pg: Re-export DatabaseError from 'pg-protocol'

Before, users would have to import DatabaseError from 'pg-protocol'.  If
there are multiple versions of 'pg-protocol', you might end up using the
wrong one.

Closes #2378

* Update error-handling-tests.js

* Update query-error-handling-tests.js

Co-authored-by: Brian C <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants