Handle bad message ordering - make it catchable. Fixes 3174
The biggest issue was when messages came in out of order sometimes the client would throw a null ref error in an uncatchable place in the code & crash the node process. As far as I know, this PR should totally fix that issue, changing the unhandled null ref error in the background into a catchable error emitted by the client (or pool if you use pool.query).
Wrote a fake postgres backend to send messages out of order to test these weird edge cases outlined in #3174 where proxies or pg-bouncer or the like is getting messages mixed up. Can extend the test suite as needed and improve other edge cases. The native driver only emits a notice event when it receives an out of order message.
Deploying node-postgres with
Cloudflare Pages
| Latest commit: |
93b9fad
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://de73bc38.node-postgres.pages.dev |
| Branch Preview URL: | https://bmc-fix-for-3174.node-postgres.pages.dev |
Hey @brianc, here to add my two cents, first wanna say thanks for your attention to the issue report and thanks to @charmander as well for weighing in! I can see that this PR as is addresses _handleParseComplete() being called on a idle client and that makes good sense to me, however if you review this comment on 3174 you will see that we also have a history of seeing _handleCommandComplete being called on (most likely) an idle client and crashing the lib as well.
I think it makes sense that whatever is decided to be done for _handleParseComplete can be done within _handleCommandComplete as well, right? And on the subject of there being a second case is there a third or fourth kind of protocol message which could show up unexpected on an idle client and maybe if there is then there could be a place further up in the call stack to check if the client is idle and emit errors from there?
Thanks!