rightfold / purescript-postgresql-client

https://pursuit.purescript.org/packages/purescript-postgresql-client
BSD 3-Clause "New" or "Revised" License
35 stars 20 forks source link

`UnhandledPromiseRejectionWarning` on integrity errors #24

Closed akheron closed 5 years ago

akheron commented 5 years ago

src/Database/PostgreSQL.js has this:

                var q = client.query({
                        text: sql,
                        values: values,
                        rowMode: 'array',
                    }).catch(function(err) {
                        onError(err);
                    }).then(function(result) {
                        onSuccess(result.rows);
                    });

The catch and then handlers are in such an order that then is called even if an error was thrown, and as such result is undefined so result.rows throws an error. As a consequence, upon integrity error this is logged to console:

(node:23941) UnhandledPromiseRejectionWarning: TypeError: Cannot read property 'rows' of undefined
    at .../output/Database.PostgreSQL/foreign.js:43:42
    at process._tickCallback (internal/process/next_tick.js:68:7)
(node:23941) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1)
(node:23941) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

The fix seems to be to change the code to have then before catch. I'm just wondering whether they are in that order for a reason?

paluh commented 5 years ago

I'm just wondering whether they are in that order for a reason?

It seems that this is a bug. I'll try provide fix + regression for this case later today. @akheron Thanks a lot!

akheron commented 5 years ago

I actually already have a patch, but cannot figure out a way to check that unhandled promise rejections don't happen in the node "main loop" :thinking: