Let a signal abort a send queued behind a full send window (#90)

* Regression tests for aborting a send queued behind a full window

* Let a signal abort a send queued behind a full send window

* Log a queued send, tighten its regression tests and settle the decision record

* Take the send window's queue off an array so an abort cannot stall the loop

---------

Co-authored-by: Mikael Göransson <mikael.goransson@timewave.se>
This commit is contained in:
2026-09-06 18:17:01 +02:00
committed by GitHub
parent 0a7c491e55
commit 30eedc81ee
7 changed files with 243 additions and 17 deletions
+26 -3
View File
@@ -169,13 +169,12 @@ naming the behaviour.
| `ESME_RINVBCASTCHANIND` typo | Defined as `0x011`, three hex digits; the spec value is `0x0112` |
| Every response carries a message id | `session.js` builds `params = {'message_id': …}` for every response it sends, `deliver_sm_resp` included; SMPP 3.4 4.6.2 makes that field unused and NULL, and Jasmin closes the connection on one |
## Multipart sends and the send window
## Multipart sends
`sendSms` puts every segment of a message on the wire together instead of waiting for each response
in turn, so a long message costs one round trip rather than one per segment. Nothing on the
receiving side forces the order either way: this library answers each inbound segment as it arrives,
so a peer that dispatches one request at a time is never left waiting on us, and a message with more
segments than `maxOutstanding` goes out a slot at a time and still completes.
so a peer that dispatches one request at a time is never left waiting on us.
## GSM 7-bit is sent unpacked
@@ -639,6 +638,23 @@ Grouped by what each one constrains.
is awaited with the socket already destroyed, so an unref'd one lets a process whose only remaining
work is that send exit without settling it.
- **A send queued for a send-window slot is bounded by the caller's `signal`, and by nothing else.**
Maintainer's call, 2026-09-06, from a review of PR #71: the hold above observes the signal and the
`acquire()` on the next line did not, so a caller that aborted while the window was full waited for
a slot it no longer wanted — at `responseTimeout: 0` for as long as the peer stayed quiet, which is
the deadline the README sends the caller to that signal for. Goal 4 is not re-opened by an
unbounded wait here: the queue is the application's own backlog, unbounded in depth as well as in
time because capping it would refuse a send the application asked for, and nothing in it keeps the
peer waiting — which is what separates it from the inbound stores capped on constants. Rejected:
having `release()` skip a waiter whose signal already fired, which leaves the departed waiter in
the queue where `unfinished()` still counts it and the drain waits on it; the waiter leaves as it
settles instead. Rejected: bounding this wait by `responseTimeout` as the hold is bounded — a full
window is this end's own concurrency draining as the peer answers rather than a link going nowhere,
and that bound would fail a message with more segments than `maxOutstanding` partway through
against a slow peer. The failure is a plain `Error` rather than `UnansweredError`, the same answer
an abort at the gate already gives. The drain half needs nothing: `close({ signal })` already hands the signal to
`window.idle()`, and `unbind()` taking none is the shape README states.
- **The gate decides whether a link can carry a request, and a bind is what makes it one.**
Maintainer's call, 2026-09-01: `attach()` clears `closed` the moment a socket is handed over, one
round trip before the bind is answered, so gating on `closed` let a send arriving in that window go
@@ -666,6 +682,13 @@ Grouped by what each one constrains.
handlers normalise through `errorFrom()` rather than inline — a route out of the handler would land
on a bare `process.nextTick` with nothing to catch it.
- **The four-line abort dance is copied across `LinkGate`, `IdleWaiters`, `PendingRequests` and
`SendWindow` rather than extracted.** Architecture review, 2026-09-06: pre-check `aborted`, attach
`{ once: true }`, detach on settle, leave the registry. What differs at each site is the registry
and what settling means — a FIFO handing over a slot, a set released together, a map keyed by
sequence number, a count recomputed at settle — so a shared `Waiters<T>` fits two of the four and
is a shallower module than the copies. Extract it once a fifth appears.
- **`SmppLog` is a five-method contract this library declares, not a dependency.** `debug`, `error`,
`info`, `verbose` and `warn` are what the code actually calls, so an application can satisfy it
with an object literal. `@larvit/log` implements it structurally and stays a devDependency, where