Skip to content

node:net: route throws from 'data'/'connection' listeners to uncaughtException and keep the socket alive - #35347

Open
robobun wants to merge 1 commit into
mainfrom
farm/737186d7/net-listener-throws-uncaught
Open

node:net: route throws from 'data'/'connection' listeners to uncaughtException and keep the socket alive#35347
robobun wants to merge 1 commit into
mainfrom
farm/737186d7/net-listener-throws-uncaught

node:net: route throws from 'data'/'connection' listeners to uncaught…

73454fa
Select commit
Loading
Failed to load commit list.
Claude / Claude Code Review completed Jul 24, 2026 in 25m 34s

Code review found 1 potential issue

Found 1 candidates, confirmed 1. See review comments for details.

Details

Severity Count
🔴 Important 0
🟡 Nit 1
🟣 Pre-existing 0
Severity File:Line Issue
🟡 Nit src/js/node/net.ts:1207-1211 TLS 'secureConnection' emit is the SSL sibling of the wrapped 'connection' emit and retains the same misrouting

Annotations

Check warning on line 1211 in src/js/node/net.ts

See this annotation in the file changed.

@claude claude / Claude Code Review

TLS 'secureConnection' emit is the SSL sibling of the wrapped 'connection' emit and retains the same misrouting

The TLS sibling of this emit — `server.emit("secureConnection", self)` in `ServerHandlers.handshake` (net.ts:968), plus the adjacent `emit("secure")`/`emit("secureConnect")` — is left bare, so a throw from a `tls.createServer(handler)` connection listener still escapes into native `on_handshake`, is caught, and is routed to `call_error_handler` exactly as `'connection'` was before this change. Pre-existing, but per REVIEW.md's "fix the whole class … SSL/non-SSL variants" rule the same `try { emi