Repository navigation
fix(pg-cursor): clear query_timeout when the cursor finishes - #3808
Open
DylanMerigaud wants to merge 1 commit into
Open
DylanMerigaud wants to merge 1 commit into
DylanMerigaud wants to merge 1 commit into
Conversation
pg-cursor never called the Submittable callback, so the timer that Client#query starts for query_timeout was never cleared. It fired after the cursor had finished, called the callback with 'Query read timeout', and kept the cursor and the process alive until then. Call the callback once when the cursor ends or errors, as pg-query-stream already does. Fixes brianc#3497
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #3497
When a client has
query_timeoutset,Client#querywraps the query'scallbackso the timer is cleared once the query finishes.pg-cursornever callscallback, so for a cursor the timer always ran to the end. When it fired, it called the callback withQuery read timeouteven though the cursor had already been read and closed and the client had ended. Until then the timer kept the cursor, its rows and the process alive, which is the retention reported in the issue.With this change the cursor calls
callbackonce: with its result when it ends, with the error when it fails.pg-query-streamalready does the same on its cursor'sendanderrorevents. Code that passes no callback sees no difference; a callback passed toclient.query(cursor, callback)is now called when the cursor finishes instead of only when the timeout fires.Testing
packages/pg-cursor/test/query-timeout.js. One test reads and closes a cursor, the other makes it fail withSELECT 1 / 0. Both wait past a 100 msquery_timeoutand check that the callback ran exactly once, without a timeout error. Both fail on master withQuery read timeoutand pass with this change.yarn lintpasses.yarn lerna exec --concurrency 1 --ignore pg-native yarn testpasses on Node 24.10 against PostgreSQL 17.6 set up like CI (SSL on, the SCRAM test roles). Thepg-nativesuite crashes with SIGSEGV on my machine, on master too, so I left it out; this change does not touch it.AI assistance
I drafted this with Claude Opus 5.5, read the diff line by line and can answer for every change.