Skip to content

fix(pg): settle query_timeout once via handleError and clear timer - #3815

Open
mikemasam wants to merge 1 commit into
brianc:masterfrom
mikemasam:fix/emit-error-with-timeout
Open

mikemasam wants to merge 1 commit into
brianc:masterfrom
mikemasam:fix/emit-error-with-timeout

Conversation

@mikemasam

Copy link
Copy Markdown

Problem
With query_timeout = 100:

// *** callback API — called twice
client.query('SELECT pg_sleep(5)', (err) => {
  console.log(err.message) // 'Query read timeout' x2
})

//*** event API — never called
client.query('SELECT pg_sleep(5)')
  .on('error', (e) => console.log(e.message)) // never fires

Caused by this in client.js:

queryCallback(error)       // direct call
query.handleError(error)   // second call

query.callback = wrapper   // event query now looks like
                           // a callback query, so emit is skipped

Fix

  • Timeout settles once through handleError, which picks callback or emit.
  • Query emits a new internal cleanup event on success/error to clear the timer.
  • Callback is blanked after, so a late database answer is dropped.
// callback API — once
client.query('SELECT pg_sleep(5)', (err) => {
  console.log(err.message) // 'Query read timeout' x1
})

// event API — once
client.query('SELECT pg_sleep(5)')
  .on('error', (e) => console.log(e.message)) // 'Query read timeout' x1

@mikemasam
mikemasam marked this pull request as draft October 11, 2026 15:17
@mikemasam
mikemasam marked this pull request as ready for review October 11, 2026 15:17

@brianc brianc left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like that you're trying to fix this problem, and I appreciate it - good test too! One thing though is I wouldn't recommend using event emitters internally on the client to handle control flow or operations. Personally I kinda regret even making the client an event emitter at all. Is there a way to do this w/o introducing another event (cleanup)?

@mikemasam
mikemasam requested a review from brianc October 12, 2026 11:10
@mikemasam

Copy link
Copy Markdown
Author

Hi @brianc, thanks for the feedback. I've updated query.js to use an optional onEndRead hook to clear the timeout, so there's no need for a separate internal cleanup event anymore.

@mikemasam
mikemasam force-pushed the fix/emit-error-with-timeout branch from 3b732d1 to efeee8a Compare October 12, 2026 11:41
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.

2 participants