Skip to content

fix(listener): avoid uncaught error when force-closing a non-standard socket - #383

Open
mohamedramadan14 wants to merge 1 commit into
honojs:mainfrom
mohamedramadan14:fix/drain-force-close-mocked-socket
Open

fix(listener): avoid uncaught error when force-closing a non-standard socket#383
mohamedramadan14 wants to merge 1 commit into
honojs:mainfrom
mohamedramadan14:fix/drain-force-close-mocked-socket

Conversation

@mohamedramadan14

Copy link
Copy Markdown

Closes #374.

forceClose guards with !socket.destroyed, which is undefined on any socket that is not a real net.Socket — so the guard passes and socket.destroySoon() throws. It runs on a timer, so the TypeError becomes an uncaughtException the caller cannot catch.

This feature-detects the teardown method instead:

if (typeof socket.destroySoon === 'function') {
  socket.destroySoon()
} else if (typeof socket.destroy === 'function') {
  socket.destroy()
}

net.Socket and tls.TLSSocket both define destroySoon, so real connections keep the existing graceful-close behaviour unchanged; the check only ever diverges for sockets that could not have serviced the call. cleanup() still runs first on every path, so the timer and listeners are released regardless of which branch is taken.

This covers both entry points into forceClose — the DRAIN_TIMEOUT_MS timer and the MAX_DRAIN_BYTES overrun.

Tests drive getRequestListener with a mocked request whose socket is a bare EventEmitter, matching light-my-request's MockSocket: one asserts the drain timeout no longer throws, the other that a socket exposing only destroy is still torn down.

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.

getRequestListener: uncaught socket.destroySoon is not a function in forceClose when the socket is mocked (e.g. fastify.inject via MCP SDK)

1 participant