Report connection resets during the TLS handshake as ECONNRESET instead of a certificate error - #31950
Report connection resets during the TLS handshake as ECONNRESET instead of a certificate error#31950robobun wants to merge 10 commits into
Conversation
|
Updated 1:41 PM PT - Aug 16th, 2026
✅ @robobun, your commit f4fb31307cfe71e20c0b151088b3901028a6846d passed in 🧪 To try this PR locally: bunx bun-pr 31950That installs a local version of the PR into your bun-31950 --bun |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughMaps negative uSockets TLS handshake verification error codes to ECONNRESET, updates fetch error messaging for handshake resets, and adds regression tests for fetch and bun install that simulate mid-handshake socket reset/close and assert ECONNRESET behavior. ChangesTLS Handshake Error Code Mapping
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/js/web/fetch/fetch.tls.test.ts`:
- Around line 309-313: The test starts a server using Promise.withResolvers()
(variables listening and onListening) and only resolves via server.listen's
callback, so startup errors can leave await listening hanging; attach an error
handler (e.g., const onError = (err) => reject(err)) with server.once("error",
onError) before calling server.listen(0, "127.0.0.1", onListening) and then,
after the await (in a finally block), call server.removeListener("error",
onError) to ensure the error handler is removed and listening is rejected on
startup errors; reference Promise.withResolvers, listening, onListening,
server.listen and server.once/server.removeListener to locate the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2b138ef7-5d75-4edf-b427-1cc6ef2d0227
📒 Files selected for processing (4)
src/http/lib.rssrc/runtime/webcore/fetch/FetchTasklet.rstest/cli/install/bun-install-stalled-tls.test.tstest/js/web/fetch/fetch.tls.test.ts
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
test/cli/install/bun-install-stalled-tls.test.ts (1)
81-132: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueConsider adding explicit timeout for consistency and safety.
The second test lacks an explicit timeout argument (unlike the first test's 60-second timeout on line 73). Although this test should fail fast with
BUN_CONFIG_HTTP_RETRY_COUNT: "0", adding an explicit timeout (e.g., 30 seconds) would provide a safety bound and maintain consistency with the first test.🛡️ Suggested timeout addition
for (const s of sockets) s.destroy(); await new Promise<void>(resolve => server.close(() => resolve())); } -}); +}, 30_000);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/cli/install/bun-install-stalled-tls.test.ts` around lines 81 - 132, The test "bun install reports ECONNRESET when the registry resets the connection during the TLS handshake" lacks an explicit timeout; update its test(...) invocation (the top-level anonymous function for that test) to include an explicit timeout (e.g., 30000 or match the other test's 60000) as the third argument so the test framework enforces a safety bound when running the async test; locate the test by its description string and add the numeric timeout argument to that test call.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/cli/install/bun-install-stalled-tls.test.ts`:
- Around line 91-94: Add a short explanatory comment above the server
error-handler/listen block that mirrors the note in
test/js/web/fetch/fetch.tls.test.ts: explain that the server.once("error",
onListenError) is intentionally left attached until after await listening to
catch racey startup errors even if listen succeeds, so we must not remove or
prematurely detach the handler; reference the Promise.withResolvers usage (const
{ promise: listening, resolve: onListening, reject: onListenError } =
Promise.withResolvers<void()) and the server.listen(0, "127.0.0.1", onListening)
call to make clear why the handler remains.
---
Outside diff comments:
In `@test/cli/install/bun-install-stalled-tls.test.ts`:
- Around line 81-132: The test "bun install reports ECONNRESET when the registry
resets the connection during the TLS handshake" lacks an explicit timeout;
update its test(...) invocation (the top-level anonymous function for that test)
to include an explicit timeout (e.g., 30000 or match the other test's 60000) as
the third argument so the test framework enforces a safety bound when running
the async test; locate the test by its description string and add the numeric
timeout argument to that test call.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 43eac8b0-559a-4af3-9c82-aa7f9b6a3ee8
📒 Files selected for processing (2)
test/cli/install/bun-install-stalled-tls.test.tstest/js/web/fetch/fetch.tls.test.ts
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — narrow error-label remap with the sendfile collision now correctly gated on TLS.
Extended reasoning...
Overview
This PR fixes a misleading error label: when a TLS connection is reset/closed mid-handshake, uSockets synthesizes a verify error with error_no = -46 (ssl_trigger_handshake_econnreset in packages/bun-usockets/src/crypto/openssl.c), and get_cert_error_from_no previously let that fall through to UNKNOWN_CERTIFICATE_VERIFICATION_ERROR. The fix is a single early-return in src/http/lib.rs mapping negative error_no to ECONNRESET (X509_V_ERR_* codes are all non-negative, so this cannot collide with a real cert error), plus a new match arm in FetchTasklet::on_reject that emits Node's handshake-reset message. Three new regression tests cover both fetch() (RST and FIN variants) and bun install, and an unrelated pre-existing flaky test is fixed by binding to 127.0.0.1 explicitly.
Security risks
None. This is purely an error-label/message change on an already-failing path; it does not alter certificate validation, retry logic, or any success/failure decision. The error_no < 0 guard cannot mask a real X509 verification failure since those codes are defined as non-negative.
Level of scrutiny
Moderate — it touches the shared HTTP client error-mapping helper used by HTTPContext::on_handshake and both ProxyTunnel handshake sites, plus the fetch rejection path. But the change is additive and well-scoped: only the label of a sentinel value that was previously misclassified is changed, and the new message arm is gated on used_tls (request URL or proxy is https) so the plain-HTTP sendfile ECONNRESET keeps the generic message. I verified SendFile::is_eligible requires url.is_http() and the call site panics on IS_SSL, so the two producers of the interned ECONNRESET tag are cleanly partitioned by the gate.
Other factors
My prior inline finding (the sendfile/plain-HTTP collision) was addressed in 791c349 with the used_tls gate, and both CodeRabbit threads are resolved. No CODEOWNER paths are touched. The CI failure on musl shards is an unrelated LTO data-layout linker issue, not caused by this diff. Retry behavior is unaffected per the PR description (manifest retries key on missing metadata, not error name).
There was a problem hiding this comment.
Thanks for the sendfile-path fix in 791c349 — the used_tls gating addresses my concern. I didn't find anything else, but since this changes the shared TLS handshake error mapping used by both fetch() and bun install, leaving final sign-off to a human.
Extended reasoning...
Overview
This PR fixes a diagnostic mislabel: connections reset/closed mid-TLS-handshake were reported as UNKNOWN_CERTIFICATE_VERIFICATION_ERROR instead of ECONNRESET. Two source changes: src/http/lib.rs adds a 3-line guard in get_cert_error_from_no to return ECONNRESET for negative error_no (uSockets synthesizes -46 for this case; X509 codes are non-negative, verified against packages/bun-usockets/src/crypto/openssl.c:792), and src/runtime/webcore/fetch/FetchTasklet.rs adds a Node-parity error message gated on the request or its proxy using TLS. Three new regression tests plus a flakiness fix (hostname: "127.0.0.1") to a pre-existing test.
Prior feedback
I previously flagged that the new ECONNRESET match arm would also catch the plain-HTTP sendfile body path (SendFile::write → raw errno → same interned tag), producing a misleading TLS message for non-TLS uploads. The author fixed this in 791c349 by gating the message on used_tls (URL or proxy is HTTPS), with a clear comment documenting both producers. The fix is correct for the case I raised. All CodeRabbit threads are also resolved.
Security risks
None identified. The change only affects which error label/message is surfaced when the handshake has already failed; it does not touch certificate verification, trust decisions, or whether a connection proceeds. If anything, replacing a cert-error label with ECONNRESET makes transient network failures more likely to be retried, which is the intended behavior.
Level of scrutiny
Medium. The diff is small (~30 source lines) and well-reasoned, but it touches the shared get_cert_error_from_no helper used by every handshake handler (HTTPContext::on_handshake, both ProxyTunnel::on_handshake sites) and changes user-visible error semantics for fetch() and bun install. The used_tls gating also folds in proxy-scheme logic (http_proxy.is_https()), which interacts with the sendfile eligibility rules in ways I have not exhaustively traced (e.g., http:// URL through an https:// proxy). That, plus the assertion in the description that install retry behavior is unchanged because retries key on missing metadata rather than error name, is the kind of cross-cutting claim a maintainer familiar with the install/HTTP retry paths should confirm.
Other factors
Test coverage is good: parameterized RST/FIN fetch tests and a bun install end-to-end test that asserts both the presence of ECONNRESET and the absence of the old mislabel. The localhost → 127.0.0.1 binding fix to the existing timeout test is a clean, well-commented flakiness fix unrelated to the core change.
|
Re the untraced corner in the last review (http:// URL through an https:// proxy): sendfile is only selected when there is no proxy at all ( |
|
CI summary for whoever picks this up: the diff is green on every lane that runs it. Final state of build 73222 (commit 9cb5e14, the complete rebased PR): the only failure is test/js/node/test/parallel/test-net-connect-memleak.js on the two Alpine 3.23 x64 lanes. That file is a ported Node GC/memory test untouched by this PR; plain net.connect never reaches the HTTP client handshake path this PR changes. All other lanes passed, including every lane running the PR's test files (fetch.tls.test.ts with the RST/FIN/EPROTO cases, and bun-install-stalled-tls.test.ts). History, for context: the pre-rebase rounds are summarized in the earlier paragraphs below; since then the PR was rebased over the thiserror error-enum refactor (#33909), adapted to the upstream raw-close-on-RST behavior (#32681), and extended to map the -71/EPROTO parked-reason sentinel (#33390) separately from ECONNRESET, with regression tests for each case. claude[bot]'s review verified all findings addressed at 9cb5e14 and deferred final sign-off to a maintainer. |
The -46 ECONNRESET sentinel (peer closed mid-handshake, sent no invalid bytes) is a different failure class and should not inherit the "responded with data that is not valid TLS" message. Leaving it to get_cert_error_from_no also lets #31950's -46 mapping apply on the direct-fetch path.
a030815 to
042558b
Compare
There was a problem hiding this comment.
All four of my prior findings are now addressed — I verified 9cb5e14 restored the -71 → EPROTO mapping in get_cert_error_from_no alongside the < 0 → ECONNRESET fallback, and the comment now documents both sentinel producers. No new issues found this pass. Given the iteration history and that this changes user-facing .code values in the HTTP client / fetch / bun install error paths, deferring final sign-off to a maintainer.
What was reviewed:
- Confirmed both negative-sentinel producers in
openssl.c(ssl_dispatch_parked_reason→ -71,ssl_trigger_handshake_econnreset→ -46) are now mapped distinctly. - Re-checked the
used_tlsgate inFetchTasklet::on_rejectcovers both direct-https and https-proxy hops; the sendfile/plain-HTTP collision remains excluded. - Checked that no other
get_cert_error_from_nocaller bypasses the negative-sentinel mapping — all three sites (HTTPContext + both ProxyTunnel handlers) go through the shared helper. - The
hostname: "localhost"→"127.0.0.1"change to the pre-existing timeout test is a legitimate IPv6-bind flake fix; assertions unchanged.
Extended reasoning...
Overview
The PR maps uSockets' negative handshake-error sentinels (-46/ECONNRESET for mid-handshake connection close, -71/EPROTO for fatal TLS protocol errors) to their proper errno-style codes in get_cert_error_from_no (src/http/lib.rs), instead of letting them fall through to UNKNOWN_CERTIFICATE_VERIFICATION_ERROR. FetchTasklet::on_reject gains a match arm that emits Node's exact handshake-reset message, gated on used_tls so the plain-HTTP sendfile ECONNRESET path keeps its generic message. Three new tests cover fetch (RST, FIN, EPROTO) and one covers bun install. A pre-existing timeout test's fixture is switched from localhost to 127.0.0.1 to fix an IPv6-bind flake.
Security risks
None identified. This is error-label remapping — it changes which .code and message a rejection carries, not whether verification happens or whether requests are transmitted. The rejectUnauthorized / cert-verification paths are untouched. The change makes error reporting more accurate (users no longer chase CA-store problems for network resets), which is a net positive for operational security.
Level of scrutiny
Medium-high. The runtime change is ~30 lines across two files, but it sits in the HTTP client's handshake-failure path shared by fetch, bun install, and proxy tunneling, and it changes user-observable .code values that callers may branch on. The PR went through four correction rounds during my review (sendfile collision, stray tarball, EPROTO swallowed by blanket < 0, and the EPROTO runtime half not staged), each of which the author fixed promptly and correctly. That history — plus the rebase over two upstream refactors (#33909 error enums, #32681 raw-close-on-RST, #33390 parked-reason sentinel) that the PR had to adapt to — argues for a maintainer confirming the final shape rather than a bot approval.
Other factors
- All four of my prior inline threads are resolved and I verified the fixes are present at HEAD (9cb5e14).
- The bug-hunting system found nothing this run.
- CI build #73222 for the latest commit was triggered ~20 minutes ago; the previous green build predates the EPROTO runtime fix.
- Test coverage is thorough for the fix's surface (RST/FIN/EPROTO × fetch, FIN × install), with platform-specific message assertions correctly scoped to POSIX.
- The blanket
error_no < 0 → ECONNRESET(after the-71carve-out) means any future negative sentinel added to openssl.c will report as ECONNRESET until explicitly mapped. That's a reasonable default (better thanUNKNOWN_CERTIFICATE_VERIFICATION_ERROR) but worth a maintainer being aware of.
|
Independent confirmation from a second, unrelated context, plus a verification of this PR's build — posting since this has been sitting at Hit in the wild outside
|
| time | request | result |
|---|---|---|
| 19:23:25, 19:28:26 | short GET → host/backend-api |
ok |
| 19:30:17 → 19:31:14 (×5) | streaming POST → same host | unknown certificate verification error |
| 19:34:48 | short GET → same host | ok |
A genuine trust-store or chain problem cannot pass the short GETs on either side of that window. Independently, openssl s_client to the same host through the same proxy returned Verify return code: 0 (ok) with a full valid chain. So: no CA knob could ever have helped, exactly as the issue reports.
This is worth noting because the symptom is not specific to bun install's 64-way parallelism — any fetch() over a route that occasionally drops connections mid-handshake reproduces it, and the error text points every user at the wrong subsystem.
Minimal repro, zero certificates involved
import { createServer } from "node:net";
function serve(onHello: (s: any) => void): Promise<number> {
return new Promise((res) => {
const srv = createServer((sock) => { sock.once("data", () => onHello(sock)); });
srv.listen(0, "127.0.0.1", () => res((srv.address() as any).port));
});
}
async function probe(label: string, port: number) {
try { await fetch(`https://127.0.0.1:${port}/`); console.log(label, "-> unexpected success"); }
catch (e: any) { console.log(label, "->", e.code, "|", e.message); }
}
console.log("bun", Bun.version, Bun.revision.slice(0, 10));
await probe("RST mid-handshake", await serve((s) => s.resetAndDestroy()));
await probe("FIN mid-handshake", await serve((s) => s.end()));Before / after, same machine
bun 1.3.14 (0d9b296af3):
RST mid-handshake -> UNKNOWN_CERTIFICATE_VERIFICATION_ERROR | unknown certificate verification error
FIN mid-handshake -> UNKNOWN_CERTIFICATE_VERIFICATION_ERROR | unknown certificate verification error
bunx bun-pr 31950 → bun 1.4.0 (9cb5e1444b):
RST mid-handshake -> ECONNRESET | The socket connection was closed unexpectedly. For more information, pass `verbose: true` in the second argument to fetch()
FIN mid-handshake -> ECONNRESET | Client network socket disconnected before secure TLS connection was established
Both match what the PR description predicts, including the FIN variant landing on the Node-parity message.
One datapoint for anyone bisecting
The rebase notes say #32681 made peer-RST raw-close before the SSL layer, so the RST variant no longer exercises the sentinel. That holds on this PR's base, but on the 1.3.14 release the RST variant still mislabels as a certificate error (above). So users on current stable hit the bad label through both paths, not just FIN — which widens the blast radius of the released behavior somewhat beyond what the test-adjustment discussion implies.
I have not reviewed the Rust changes themselves; this is a behavioral verification only.
|
Thanks for the careful verification, especially the in-the-wild table: short GETs succeeding on both sides of failing streaming POSTs, in one process with one CA set, is the cleanest field demonstration that no trust-store knob could ever have helped. Your 1.3.14 datapoint is consistent with the history here. The raw-close-on-RST routing (#32681) landed after 1.3.14, so on current stable both RST and FIN still reach the handshake sentinel and both mislabel as a certificate error; my original fail-before runs on the pre-rebase base showed the same thing. This PR fixes the label wherever the sentinel fires (ECONNRESET for resets, EPROTO for parked protocol errors), and on top of that the post-#32681 RST path independently stopped producing the certificate label, which is why the RST variant now reports the generic close message with the same ECONNRESET code you observed. Agreed that the released blast radius covers both paths and any fetch(), not just bun install. Your before/after output matches the intended behavior exactly; no changes needed from this report. |
9cb5e14 to
a01d202
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
…andshake A TCP reset/close before the TLS handshake completes is delivered by uSockets as a synthesized verify error (error -46, code ECONNRESET). The handshake handlers funneled any nonzero error_no through get_cert_error_from_no, so the sentinel fell through to UNKNOWN_CERTIFICATE_VERIFICATION_ERROR even though no certificate was ever exchanged. Map negative error_no (never a valid X509_V_ERR_* code) to ECONNRESET, matching what Node reports for the same case. Fixes #31949
On Windows CI the mid-handshake reset surfaces with the ECONNRESET code but a different message than the Node-parity text asserted here (observed on windows-2019-x64 and windows-11-aarch64; the bun install variant, which asserts the error name, passes on those lanes). Keep the code assertion on every platform and the exact message on POSIX.
A peer RST now raw-closes the socket before the SSL layer runs (the POLL_TYPE_SOCKET error arm in loop.c), so only a FIN reaches the mid-handshake sentinel. Assert the ECONNRESET code for both fetch variants, scope the sentinel's exact message to the FIN path, and drive the install test with a FIN while accepting either connection-error name.
uSockets emits a second negative verify sentinel, -71/EPROTO, when a fatal TLS protocol error was parked for the handshake dispatch (a peer answering the ClientHello with non-TLS bytes, a version mismatch, a fatal alert). The blanket negative check labeled it ECONNRESET; report it as EPROTO, matching Node.
The previous commit staged only the regression test; the runtime carve-out for -71/EPROTO was lost to a checkout during local verification. Restore it: a fatal TLS protocol error parked by ssl_dispatch_parked_reason now reports EPROTO, matching Node, while the reset sentinel keeps ECONNRESET.
http_proxy moved to HTTPClient as pub(crate) in the per-hop proxy refactor, so FetchTasklet can no longer read it directly. Add an AsyncHTTP helper covering both hops (target URL https, or proxy connection https) and use it for the handshake-message gate.
a01d202 to
f4fb313
Compare
There was a problem hiding this comment.
I reviewed this PR and the current bug-hunting pass found no issues. All four earlier findings (sendfile ECONNRESET collision, stray tarball, -71/EPROTO sentinel, and the unstaged runtime half of that fix) are addressed at HEAD. Because the change sits in the HTTP client's TLS handshake error path and went through several rebase-driven adjustments, a maintainer sign-off would still be worthwhile.
What was reviewed:
get_cert_error_from_no: -71→EPROTO carve-out precedes the< 0→ECONNRESET arm; verified both sentinels inpackages/bun-usockets/src/crypto/openssl.cand thatX509_V_ERR_*codes are non-negative.used_tls()gate: covers https target, https proxy, and https-through-http-proxy (viaself.url.is_https()); sendfile isproxy.is_none() && url.is_http()so cannot collide whenused_tlsis true..codederivation:Error::Sys(e)→<&str>::from(e)yields"ECONNRESET"/"EPROTO"directly, so the EPROTO test needs no dedicated message arm.- Tests: hermetic (loopback TCP/Bun.serve), drain pipes concurrently, wire error/close handlers, no fixed sleeps.
Extended reasoning...
Overview
The PR maps two uSockets negative handshake sentinels (-46/ECONNRESET and -71/EPROTO) to their Node-parity errno codes in get_cert_error_from_no (src/http/lib.rs) instead of letting them fall through to UNKNOWN_CERTIFICATE_VERIFICATION_ERROR. It adds AsyncHTTP::used_tls() (src/http/AsyncHTTP.rs) and a gated Error::Sys(ECONNRESET) if used_tls message arm in FetchTasklet::on_reject (src/runtime/webcore/fetch/FetchTasklet.rs) to emit Node's "Client network socket disconnected before secure TLS connection was established" only for TLS-using requests. Regression tests are added to test/js/web/fetch/fetch.tls.test.ts (RST/FIN mid-handshake, EPROTO from plain-HTTP peer) and test/cli/install/bun-install-stalled-tls.test.ts (install against a registry that closes mid-handshake).
Security risks
None identified. The change is error-label mapping only — certificate verification, rejectUnauthorized, and checkServerIdentity paths are untouched. The used_tls gate exists to keep the plain-HTTP sendfile ECONNRESET from receiving a TLS-specific message; it does not relax or bypass any check. The negative-error_no carve-out is safe because X509_V_ERR_* codes are all non-negative, so no real cert error can be masked as a connection error.
Level of scrutiny
Medium-high. The runtime diff is small (~30 lines), but it lives in the shared handshake error helper feeding HTTPContext::on_handshake and both ProxyTunnel::on_handshake sites, and the PR's history shows two real correctness catches (the sendfile ECONNRESET collision, and the -71/EPROTO sentinel that landed upstream mid-PR and was initially swallowed by the blanket < 0). Both are now fixed and tested. The tests look robust (loopback listeners, Promise.withResolvers for readiness, error handlers wired, subprocess pipes drained concurrently).
Other factors
CI was reported green on all lanes running the PR's tests (build 73222 per robobun's summary). An independent user verified before/after behavior on macOS with a minimal repro. The four comment-cop flags were resolved with justifications (rustdoc contract, cross-module sentinel documentation) that seem reasonable — the comments document non-local invariants that took multiple review rounds to establish. No human maintainer has reviewed the Rust changes (hhh2210 explicitly disclaimed doing so), which is why I'm deferring rather than approving.
2 participants
Fixes #31949
Symptom
bun installagainst a registry whose connections die mid-TLS-handshake reports:while
curl,openssl s_client,npm, and a singlefetch()against the same host all verify the chain fine, and no CA knob (--use-system-ca,NODE_USE_SYSTEM_CA,NODE_EXTRA_CA_CERTS) changes anything.Root cause
This is not a certificate problem. When a connection is reset or closed before the TLS handshake completes, uSockets synthesizes a verify error with
error = -46and codeECONNRESET(ssl_trigger_handshake_econnresetinpackages/bun-usockets/src/crypto/openssl.c). The HTTP client's handshake handlers treat any nonzeroerror_noas an X509 verify code and funnel it throughget_cert_error_from_no, where-46falls through toUNKNOWN_CERTIFICATE_VERIFICATION_ERROR.That label sent the reporter on a multi-day CA-store hunt for what is actually a network appliance killing connections (their hardened CI;
bun installopens up to 64 parallel connections while curl/npm/fetch open one, which is why only install tripped it).Reproducible with no certificates involved at all, with a plain TCP server that accepts, reads the ClientHello, and resets the socket:
Node reports the identical scenario as
ECONNRESETwith "Client network socket disconnected before secure TLS connection was established".Fix
src/http/lib.rs:get_cert_error_from_nomaps negativeerror_notoECONNRESET.X509_V_ERR_*codes are all non-negative, so a negative value is never a certificate error; the only producer is the uSockets mid-handshake reset sentinel. This is the shared helper behind all handshake handlers (HTTPContext::on_handshakeand bothProxyTunnel::on_handshakesites), so every caller is fixed at once.src/runtime/webcore/fetch/FetchTasklet.rs: map theECONNRESETfailure to Node's message for this case, "Client network socket disconnected before secure TLS connection was established". The message is gated on the request (or its proxy) using TLS, because the plain-HTTP sendfile body path also surfaces a rawECONNRESETerrno for mid-upload resets; those keep the generic message. The errorcodeisECONNRESETin both cases.After the fix:
bun installretry behavior is unchanged (manifest retries key on missing metadata, not the error name). The WebSocket client's handshake handler already reports a generic TLS handshake failure rather than a certificate error, so it is not touched.Tests
test/js/web/fetch/fetch.tls.test.ts: fetch against a server that resets (RST) or closes (FIN) the connection during the handshake must reject withcode: "ECONNRESET"; the FIN variant additionally asserts the Node-parity message on POSIX. The two variants take different paths: a FIN reaches the SSL close path and the mid-handshake sentinel this PR fixes, while a peer RST raw-closes the socket before the SSL layer runs (the POLL_TYPE_SOCKET error arm in packages/bun-usockets/src/loop.c) and surfaces as the generic connection-closed failure, which already carried the ECONNRESET code.test/cli/install/bun-install-stalled-tls.test.ts:bun installagainst a registry that closes the connection mid-handshake must print a connection error (ECONNRESETvia the sentinel, orConnectionClosedon platforms whose event loop raw-closes first) and neverUNKNOWN_CERTIFICATE_VERIFICATION_ERROR.The FIN-driven tests fail on the unfixed build with exactly the mislabel from the issue and pass with this change.
Rebase notes
Rebased over the error-handling refactor that replaced interned error names with per-crate thiserror enums (#33909). The fix is the same shape in the new idiom:
get_cert_error_from_noreturnsError::Sys(SystemErrno::ECONNRESET)for negativeerror_no, and the fetch message arm matches that variant. The sendfile collision gate is unchanged (that path now produces the sameError::Sysvalue viabun_errno::from_errno). Upstream also changed peer-RST handling to raw-close the socket before the SSL layer runs (#32681), so the RST variants no longer exercise the sentinel; the tests were adjusted as described above, and fail-before was re-verified against the new base.A later rebase dropped the timeout-test fixture commit (main adopted an equivalent 127.0.0.1 binding fix) and adapted the message gate to the per-hop proxy refactor:
http_proxymoved toHTTPClientas crate-private, so the gate now uses a smallAsyncHTTP::used_tls()helper covering both the target URL and the proxy hop.The rebase also picked up a second negative sentinel added upstream in #33390: -71/EPROTO for fatal TLS protocol errors (
ssl_dispatch_parked_reason). It is mapped to EPROTO (Node parity) rather than being swallowed by the reset mapping, with a regression test (https fetch against a plain-HTTP listener must reject withcode: "EPROTO", not a certificate error).One pre-existing test in the same file,
fetch timeout works on tls, failed on some hosts independently of this change: its server usedhostname: "localhost", which binds ::1 only on hosts whose resolver returns the IPv6 loopback first, while fetch connects to 127.0.0.1, so the request died with ConnectionRefused before the timeout under test could fire. The fixture now binds 127.0.0.1 explicitly; the assertions are unchanged. With that, both test files pass fully.no test proof · iteration 10 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/web/fetch/fetch.tls.test.ts