Repository navigation
Add listener error handling, outgoing event, and session addresses - #15
Merged
Merged
Conversation
…stener exceptions
'outgoing' is emitted synchronously from send() for every pdu written to
the socket, once the sequence_number is assigned and before the write, so
for a request it always precedes the 'pdu' event of its response. 'send'
comes from the asynchronous write callback and could follow a response
that arrived in the same chunk. It covers requests, responses and the
automatic enquire_link, enquire_link_resp and unbind_resp, and is typed
in SessionEventMap.
session.localAddress/localPort are set next to remoteAddress/remotePort
(connect on a client session, construction on a server session, from
rootSocket() on TLS) and kept after close, when the socket has forgotten
them. The client connect handler fell back to remoteAddress for
remotePort; it now falls back to the previous remotePort.
auto_unbind_response (default false, as handlers that already answer
unbind would double the response) answers an incoming unbind with
unbind_resp ESME_ROK and ends the connection, before emitting 'pdu' and
'unbind', like auto_enquire_link_response. session.autoUnbindResponse
switches it at runtime.
A listener throwing out of the read loop left _busy set, after which the
session silently stopped reading the socket; a throwing debug/metrics
listener was reported as a stream 'error' with the same effect, and a
throwing 'send' listener skipped the sendCallback. _busy is now reset in
a finally, only the parsing stays in the stream-error try, and every
listener and callback called while handling a pdu goes through _guard():
the exception is emitted as 'listener_error' { error, event, pdu }, or
rethrown asynchronously when nothing listens, so it still surfaces as an
uncaught exception without breaking the session.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QEFXDb9Pc8brL9ZCHbBJyM
…lose after unbind Follow-up to the previous commit, after an adversarial review of it. Listener exceptions: the guard covered only the pdu path, so a debug or metrics listener throwing on socket.data.in or socket.data.out still escaped the socket handler. A throw out of a 'readable' listener leaves Node's Readable without a 'readable' to come, which is a second way to go deaf that resetting _busy does not cure. A throwing failureCallback in _abortPending skipped the other pending requests and 'close', and a throwing 'response_timeout' listener skipped the failureCallback. Now debug() and emitMetric() guard themselves, and every event the session emits and every callback it calls goes through _guard(). 'error' with no listener is rethrown on the next tick instead of throwing synchronously out of the socket handler. ListenerError is renamed ListenerErrorReport and its pdu is null for events that are not about one. Parse errors: with _busy reset in a finally, the stale _command_length made the session read the rest of a broken stream as garbage pdus. A pdu that cannot be parsed now destroys the connection after 'error', as the README always described, unless the 'error' listener reconnected. send(): 'outgoing' is emitted right after socket.write() instead of before it, so a listener that closes, reconnects or sends from there cannot get between the event and the write, and the event order is the wire order. The pdu.command.out debug/metrics are back before toBuffer(), which encodes the pdu in place, so logs show the pdu as built. _addPending runs after toBuffer(), so a pdu that cannot be encoded no longer leaves a pending request and its timer behind. auto_unbind_response: close() alone is a half-close, so a peer that never closed its side after unbinding kept the session open, and a keepalive in flight ended it with a spurious enquire_link_timeout. The keepalive is stopped and the socket destroyed once the unbind_resp is flushed, so 'close' always follows. Tests: a deterministic same-chunk test (raw peer answers three requests in one write, write callbacks held back), the sequence_number captured inside the listener, synchronous emission for responses asserted from a later listener, listener_error reports checked for event name and pdu, tests for a throwing listener_error listener, failureCallback, connect and response_timeout listener, socket.data.in/out debug listeners, the rethrow asserting that the rest of the pdu was handled first, stream errors, a half-open peer after unbind, close() from an 'outgoing' listener, an unencodable pdu, and the runtime switch in both directions. The timed negative tests assert from inside the 'unbind' listener, the keepalive test no longer asserts exact counts, and the uncaughtException takeover is restored from afterEach. Docs: the README no longer claims a second unbind_resp reaches the peer, describes proxy-protocol addresses correctly, and says what the next-tick rethrow changes. The remote-port changeset is folded into the addresses one and the listener-exceptions changeset is a minor bump. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QEFXDb9Pc8brL9ZCHbBJyM
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR adds robust error handling for listener exceptions, introduces an
'outgoing'event for tracking PDUs sent to the socket, exposes session address information, and adds automaticunbindresponse handling.Key Changes
Listener Exception Handling: Exceptions thrown by event listeners, callbacks, and the
debugListeneroption no longer unwind the session's code. Instead, they are caught and emitted as'listener_error'events with a report containing the error, event name, and associated PDU. This prevents the session from silently stopping to read after an exception. A PDU that cannot be parsed now destroys the connection after'error', instead of leaving a deaf session behind.'outgoing'Event: Added a new event emitted synchronously fromsession.send()for every PDU handed to the socket (requests, responses, automaticenquire_link, etc.). This provides a reliable way to track outgoing PDUs before write callbacks are invoked, addressing timing issues with the existing'send'event.Session Address Properties: Added
session.localAddressandsession.localPortproperties that are set when the connection is established and retained after the socket closes, complementing the existingremoteAddressandremotePort.Auto Unbind Response: Added
auto_unbind_responseoption (defaultfalse) to bothconnect()andcreateServer(). When enabled, incomingunbindPDUs are automatically answered withunbind_resp(statusESME_ROK), the keepalive is stopped, the connection is closed, and the session emits the standard'pdu'and'unbind'events after the response is queued.Error Handling Infrastructure: Implemented
_guard()method to wrap all listener invocations and callbacks, catching exceptions and emitting them as'listener_error'events instead of propagating them. AddedrethrowAsync()utility to re-throw uncaught exceptions asynchronously when no listener handles them.Implementation Details
_guard()method wraps listener calls and callbacks, catching exceptions and emitting'listener_error'with aListenerErrorReportcontaining the error, event name, and PDU context.'outgoing'event is emitted right aftersocket.write()is called, still synchronously withinsend(), so it fires before any write callback and before the response can be read, and a listener that closes or sends from it cannot get between the event and the write._rememberAddresses()which reads fromrootSocket()to handle TLS and proxy-protocol sessions correctly.unbind_respis sent before'pdu'and'unbind'events are emitted, and the socket is destroyed as soon as the response is flushed (destroySoon()), so'close'follows even when the peer never closes its side.'debug'and'metrics'listeners are reported but do not prevent the session from continuing to operate.https://claude.ai/code/session_01QEFXDb9Pc8brL9ZCHbBJyM