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

Fix #2556 by keeping callback errors from interfering with cleanup #2753

Merged
merged 2 commits into from Jun 20, 2022

Conversation

prust
Copy link
Contributor

@prust prust commented May 31, 2022

@brianc and @charmander: This pull request fixes #2556 (and possibly also #1105) by ensuring that errors thrown in user-supplied callbacks do not prevent the node-postgres library from doing necessary cleanup.

This is important for assert-driven testing (such as mocha and jest). Without this fix, one test failure in a query() callback will create a cascade of subsequent test failures.

This implementation should:

  • be in keeping with the rest of the library (nextTick() is already used a few times in client.js to defer exception handling)
  • make minimal changes to the existing behavior (the behavior only changes if an exception is thrown -- and even then, the only difference is that the re-throwing of the exception is deferred to the next tick)
  • include a regression test

For the repro, see #2556 (comment), and for a description of potential solutions and workarounds, see #2556 (comment).

@brianc
Copy link
Owner

brianc commented Jun 20, 2022

Ah yeah this makes sense. Thanks for the PR! I'll get this merged in & released asap. 😄

@brianc brianc merged commit 68160a2 into brianc:master Jun 20, 2022
@prust prust deleted the always-cleanup-after-callback branch June 24, 2022 16:14
@prust
Copy link
Contributor Author

prust commented Jun 24, 2022

Thanks so much @brianc, I appreciate it!

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 this pull request may close these issues.

Cannot read property 'handleRowDescription' of null
2 participants