Skip to content

fix: route raw socket 'error' events through the errorHandler option - #373

Merged
Tony133 merged 3 commits into
fastify:mainfrom
xianjianlf2:fix/route-socket-errors-through-error-handler-371
Jul 21, 2026
Merged

fix: route raw socket 'error' events through the errorHandler option#373
Tony133 merged 3 commits into
fastify:mainfrom
xianjianlf2:fix/route-socket-errors-through-error-handler-371

Conversation

@xianjianlf2

Copy link
Copy Markdown
Contributor

What

Errors emitted on the established websocket (e.g. ERR_STREAM_PREMATURE_CLOSE / ECONNRESET from a client disconnecting uncleanly) were caught by a listener hardcoded in handleUpgrade that logged unconditionally via fastify.log.error, so the errorHandler plugin option could never see them and applications had no supported way to classify expected disconnect errors.

This PR removes the hardcoded listener and attaches it in the route handler instead, where the fastify request/reply are in scope, invoking errorHandler with the same (error, socket, request, reply) signature already used when a wsHandler throws or rejects. The listener is still attached before the user's handler runs, so the socket always has an 'error' listener and the existing Node.js crash protection is preserved. The default errorHandler behavior (log + socket.terminate()) is safe for an already-errored socket.

Checklist

  • failing test added first (uses the suite's existing invalid-frame trigger), passes with the fix
  • full suite green with 100% coverage, tstyche type tests pass, lint clean
  • README errorHandler section updated

Closes #371

xianjianlf2 and others added 2 commits July 14, 2026 17:11
The listener attached in handleUpgrade logged socket errors
unconditionally at error level via fastify.log.error, so the
errorHandler option could never see errors emitted on the established
connection (e.g. ERR_STREAM_PREMATURE_CLOSE / ECONNRESET from unclean
client disconnects) and applications had no supported seam to classify
expected disconnect errors.

Attach the listener in the route handler instead, where the fastify
request and reply are in scope, and invoke errorHandler with the same
(error, socket, request, reply) signature used for wsHandler failures.
The socket still always has an 'error' listener attached before the
user handler runs, preserving the crash protection the old listener
provided.

Closes fastify#371
'Should invoke the correct handler depending on the headers' asserted on
the raw client's first 'data' chunk, assuming the 101 handshake response
and the frame from socket.send() (two separate writes) always arrive
coalesced. On CI (macOS arm64, Node 24) they arrived in separate
segments, so the first chunk contained only the 101 headers and the
match failed; the thrown assertion also skipped wsClient.end(), leaving
teardown to wait out the 30s ws closeTimeout.

Accumulate received data across 'data' events before asserting instead.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@mcollina mcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm

@Tony133
Tony133 merged commit e3fb43f into fastify:main Jul 21, 2026
14 checks passed
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.

errorHandler option cannot reach the raw socket 'error' listener attached in handleUpgrade — no way to classify expected disconnect errors

3 participants