Skip to content

fix: wrap kConnector call in try/catch to prevent client hang#4834

Open
veeceey wants to merge 1 commit intonodejs:mainfrom
veeceey:fix/issue-4697-connector-throw-hang
Open

fix: wrap kConnector call in try/catch to prevent client hang#4834
veeceey wants to merge 1 commit intonodejs:mainfrom
veeceey:fix/issue-4697-connector-throw-hang

Conversation

@veeceey
Copy link

@veeceey veeceey commented Feb 23, 2026

When client[kConnector]() throws synchronously (e.g. from tls.connect with a bad cert/key), the error goes unhandled and leaves the client in a bad state where all subsequent client.request() calls hang indefinitely.

This wraps the client[kConnector](...) invocation in a try/catch that performs the same cleanup as the error callback path — calling handleConnectError and client[kResume]() — so the client recovers properly and further requests can proceed.

Repro from the issue:

const client = new Client('https://example.com', {
  connect: { key: 'bad key', cert: 'bad cert' }
})

// First request throws, but client gets stuck
await client.request({ method: 'GET', path: '/' }).catch(() => {})

// This hangs forever without the fix
await client.request({ method: 'GET', path: '/' })

Tested locally — both requests now throw the expected error and the client stays usable.

Fixes #4697

When client[kConnector]() throws synchronously (e.g. from tls.connect
with bad cert/key), the error was unhandled and left the client in a
bad state where subsequent requests would hang indefinitely.

Wrapping the call in try/catch with the same cleanup as the error
callback (handleConnectError + kResume) restores proper error recovery.

Fixes: nodejs#4697
@veeceey
Copy link
Author

veeceey commented Feb 23, 2026

Manual test results

Tested with the repro from the issue:

$ node test-repro.js
First request error (expected): error:0480006C:PEM routines::no start line
Second request error (expected): error:0480006C:PEM routines::no start line
PASS: Both requests completed without hanging

Before the fix, the second request would hang indefinitely. Now both requests throw the expected TLS error and the client stays in a usable state.

Existing test suites pass:

  • client-connect.js — 1/1 pass
  • client-request.js — 47/47 pass
  • client-reconnect.js — 1/1 pass

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.

Client.request will hang indefinitely if Client[kConnector] throws an error

1 participant