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

[veeceey]

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

[veeceey]

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

[veeceey]

bumping this one — is there anything on my end that needs updating before review?

[veeceey]

gentle ping - would love to get some feedback on this when you get a chance

[metcoder95]

can you add some test to cover it

[codecov-commenter]

Codecov Report

❌ Patch coverage is 93.10345% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.91%. Comparing base (1867fb7) to head (094160c).
⚠️ Report is 45 commits behind head on main.

Files with missing lines	Patch %	Lines
lib/dispatcher/client.js	93.10%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4834      +/-   ##
==========================================
- Coverage   93.19%   92.91%   -0.29%     
==========================================
  Files         109      112       +3     
  Lines       34228    35643    +1415     
==========================================
+ Hits        31900    33116    +1216     
- Misses       2328     2527     +199     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Invader444]

Have been unable to upgrade undici because of this and other fixes are piling up... can we please get this merged?

[Uzlopak]

@Invader444

Sure. Can you provide a unit test?

[veeceey]

@Uzlopak @metcoder95 yep, I should've added a test from the start — that's on me. I'll put together a unit test that reproduces the hang scenario (bad TLS config causing kConnector to throw, second request hanging). will push it shortly.

[veeceey]

pushed the tests — two cases covering the sync throw scenario:

1. client does not hang when connector throws synchronously — verifies both requests get the error instead of hanging
2. client recovers after connector stops throwing — confirms the client can still make successful requests after the connector is fixed

both use a custom connect function that throws instead of calling the callback.

[mcollina]

lgtm

[Invader444]

How can we retrigger the build? Curious if that was a test timeout or related directly to this change 🤔 @veeceey can you push a no-op commit perhaps?