Cut the prose the sweep of #48 found restated or false
Mirror / push (push) Successful in 8s
Test / lint (pull_request) Successful in 24s
Test / test (18) (pull_request) Successful in 32s
Test / test (20) (pull_request) Successful in 31s
Test / test (22) (pull_request) Successful in 33s
Test / test (24) (pull_request) Successful in 32s
Test / test (26) (pull_request) Successful in 31s

This commit is contained in:
2026-09-28 21:44:54 +02:00
parent 242eb30ab3
commit de572803a0
7 changed files with 32 additions and 42 deletions
+2 -5
View File
@@ -13,9 +13,7 @@ not for structure or style.
## Goals ## Goals
The goals, in priority order, live in The goals, in priority order, live in
[README.md](https://gitea.larvit.se/larvit/smpp-js/src/branch/main/README.md#goals) — they say where this library is heading, which an outside [README.md](https://gitea.larvit.se/larvit/smpp-js/src/branch/main/README.md#goals). The README states the audience alongside them.
reader judges it by. The README states the audience alongside them.
## Hard rules ## Hard rules
@@ -180,7 +178,6 @@ decision under [The wire](docs/decisions.md#the-wire).
collaborator the type system already keeps in step: `recordingDeps()` in `messaging-mode.test.ts`, collaborator the type system already keeps in step: `recordingDeps()` in `messaging-mode.test.ts`,
`message-class.test.ts` and `unsendable.test.ts` is one `SendSmsDeps.send` that answers nothing, `message-class.test.ts` and `unsendable.test.ts` is one `SendSmsDeps.send` that answers nothing,
and a field added to that type fails to compile in every copy at once. and a field added to that type fails to compile in every copy at once.
- `message_id` values the library generates are UUID v7.
- A socket a test opens and never reads must be `resume()`d, and a `data` listener counts. An unread - A socket a test opens and never reads must be `resume()`d, and a `data` listener counts. An unread
socket never processes the peer's FIN, so `server.close()` hangs forever — that is a test bug, not socket never processes the peer's FIN, so `server.close()` hangs forever — that is a test bug, not
a library one. a library one.
@@ -314,7 +311,7 @@ the file.
- A message id base is merged at most once. - A message id base is merged at most once.
- A send that never reached the socket waits for the next link; one that did is counted, not resent. - A send that never reached the socket waits for the next link; one that did is counted, not resent.
- A send queued for a send-window slot is bounded by the caller's `signal`, and by nothing else. - A send queued for a send-window slot is bounded by the caller's `signal`, and by nothing else.
- `LinkLife` decides whether a link can carry a request, and a bind is what makes it one. - One owner decides whether a link can carry a request, and a bind is what makes it one.
### [Internals and tests](docs/decisions.md#internals-and-tests) ### [Internals and tests](docs/decisions.md#internals-and-tests)
+1 -4
View File
@@ -746,10 +746,7 @@ Who depends on this library, and what they may rely on.
spooling, scheduling, retry policy and billing belong to whatever this is the edge of. State shared spooling, scheduling, retry policy and billing belong to whatever this is the edge of. State shared
between instances is for goal 9's store, which has not shipped: today every session keeps its own, between instances is for goal 9's store, which has not shipped: today every session keeps its own,
in memory. in memory.
- **Pre-1.0, so the minor is the breaking unit** and a patch never breaks. What a 0.4.0 consumer has - **Pre-1.0, so the minor is the breaking unit** and a patch never breaks.
to change is in [MIGRATION.md](https://gitea.larvit.se/larvit/smpp-js/src/branch/main/MIGRATION.md);
what each later minor changes is in
[CHANGELOG.md](https://gitea.larvit.se/larvit/smpp-js/src/branch/main/CHANGELOG.md).
Personas this README serves, in order: Personas this README serves, in order:
+11 -23
View File
@@ -508,10 +508,7 @@ rule and an index of the titles below.
- **`close` means the session is over, and a drop the loop will retry is `disconnected`.** - **`close` means the session is over, and a drop the loop will retry is `disconnected`.**
Maintainer's call, 2026-08-31: without the split, an application that opens a replacement client on Maintainer's call, 2026-08-31: without the split, an application that opens a replacement client on
`close` ends up holding two binds on one account. `teardown()` picks the event by `close` ends up holding two binds on one account, which goal 4 forbids.
`LinkLife.retrying()`, and `end()` stops the session before tearing down, so every deliberate
shutdown emits `close`. A retry that opens a socket and then loses it is attached again through
`attach()`, which is why a second drop emits again.
- **An answer belongs to the link the message arrived on; a receipt does not.** Maintainer's call, - **An answer belongs to the link the message arrived on; a receipt does not.** Maintainer's call,
2026-09-01. Rejected: answering on the new link, which succeeds and reports `{}` for a response 2026-09-01. Rejected: answering on the new link, which succeeds and reports `{}` for a response
@@ -541,9 +538,7 @@ rule and an index of the titles below.
gives up on the operator whose provisioning lands a minute later; the backoff is what bounds the gives up on the operator whose provisioning lands a minute later; the backoff is what bounds the
rate goal 4 cares about. The attempts before the first link report nothing, because the session rate goal 4 cares about. The attempts before the first link report nothing, because the session
running one has not reached the application: `disconnected` would have no listener and `close` running one has not reached the application: `disconnected` would have no listener and `close`
would be a lie. Its wait is the one retry timer that is not `unref()`'d, for the reason would be a lie.
`LinkLife`'s hold is not — it is awaited with no other handle, so a process whose only work is
`client()` would exit unbound.
- **`connectTimeout` defaults to 10 s, bounds the whole connect including the TLS handshake, and - **`connectTimeout` defaults to 10 s, bounds the whole connect including the TLS handshake, and
`false` is the one way to turn it off.** Maintainer's call, 2026-09-20, serving goal 5: a connect `false` is the one way to turn it off.** Maintainer's call, 2026-09-20, serving goal 5: a connect
@@ -739,9 +734,7 @@ rule and an index of the titles below.
the one spelling on the public surface. The hold is bounded by `responseTimeout` rather than an the one spelling on the public surface. The hold is bounded by `responseTimeout` rather than an
option of its own — that is already the answer to how long one request may wait — and its clock option of its own — that is already the answer to how long one request may wait — and its clock
starts when the send is issued rather than when it first finds the link down, so one budget covers starts when the send is issued rather than when it first finds the link down, so one budget covers
every hold a single call makes. That timer is the one here that is not `unref()`'d: a held request every hold a single call makes.
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.** - **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 Maintainer's call, 2026-09-06, from a review of PR #71: the hold above observes the signal and the
@@ -760,19 +753,14 @@ rule and an index of the titles below.
an abort while held for a link already gives. The drain half needs nothing: `close({ signal })` already hands the signal to an abort while held for a link 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. `window.idle()`, and `unbind()` taking none is the shape README states.
- **`LinkLife` decides whether a link can carry a request, and a bind is what makes it one.** - **One owner decides whether a link can carry a request, and a bind is what makes it one.**
Maintainer's call, 2026-09-01: `attach()` marks the session attached the moment a socket is Maintainer's call, 2026-09-01, extended 2026-09-28; goal 1, since a send on a link not yet bound
handed over, one round trip before the bind is answered, so gating on that let a send arriving in comes back `ESME_RINVBNDSTS`. `LinkLife` is told what happened and never reads back into the
that window go out unbound and come back `ESME_RINVBNDSTS`. `LinkLife` is told what happened and session; every other collaborator reads it and keeps no copy. Rejected: gating on the socket being
never reads back into the session: a collaborator that has to ask does not own its decision, which attached, which admits a send one round trip before the bind is answered, and collaborators that
is how the first cut ended up answering the same question two different ways at admit and at ask the session, which answered the same question two ways at admit and at release.
release. Every other collaborator reads whether the link lives from it and keeps no copy: five `ReconnectLoop.halted` is the one other flag, because `client()` also runs a loop with no session
copies held in step by statement order were what the 2026-09-28 comprehension runs ranked hardest. behind it for `fromStart`; a session's loop is stopped by `Session.stop()` alone.
`ReconnectLoop.halted` is the loop's own, for its timer, because `client()` also runs a loop with
no session behind it for `fromStart`; a session's loop is stopped by `Session.stop()` alone.
The retry in `requestPastDrain()` asks `link.awaitsNextLink()` rather than `canCarry()`, which also
reads the socket: a loop condition the link does not gate on spins against a link that admits it
straight back.
## Internals and tests ## Internals and tests
+3 -3
View File
@@ -10,7 +10,7 @@ export type LinkLifeOptions = {
timeout: number; timeout: number;
}; };
/** `binding`: a socket is attached and its bind is not answered yet, so it carries no request. */ /** `binding`: a socket is attached and its bind is not answered yet, so it carries nothing but that bind. */
type Phase = 'binding' | 'down' | 'ended' | 'up'; type Phase = 'binding' | 'down' | 'ended' | 'up';
type Waiter = (result: VoidResult) => void; type Waiter = (result: VoidResult) => void;
@@ -69,7 +69,7 @@ export class LinkLife {
return this.reconnects && !this.stopped; return this.reconnects && !this.stopped;
} }
/** Down, with another link on its way. */ /** Not up and not over, with a link to come. */
awaitsNextLink(): boolean { awaitsNextLink(): boolean {
return !this.isUp() && !this.isOver() && this.retrying(); return !this.isUp() && !this.isOver() && this.retrying();
} }
@@ -84,7 +84,7 @@ export class LinkLife {
return this.isUp() || this.awaitsNextLink() ? undefined : over(); return this.isUp() || this.awaitsNextLink() ? undefined : over();
} }
/** One budget for a request, however many links it waits through. 0 never gives up. */ /** One budget for a request, however many links it waits through. */
hold(signal: AbortSignal | undefined): () => Promise<VoidResult> { hold(signal: AbortSignal | undefined): () => Promise<VoidResult> {
const deadline = this.timeout > 0 ? this.now() + this.timeout : 0; const deadline = this.timeout > 0 ? this.now() + this.timeout : 0;
+1 -2
View File
@@ -69,7 +69,6 @@ export class OutgoingRequests {
this.pending.settle(seqNr, { err }); this.pending.settle(seqNr, { err });
} }
/** Sends a request and resolves with the peer's response. */
request(input: PduObjectInput, options: SendOptions): Promise<Result<{ pduObj: PduObject }>> { request(input: PduObjectInput, options: SendOptions): Promise<Result<{ pduObj: PduObject }>> {
// Ahead of the drain, so a misuse is named as one rather than blamed on the shutdown. // Ahead of the drain, so a misuse is named as one rather than blamed on the shutdown.
const wrong = misuse(input); const wrong = misuse(input);
@@ -136,7 +135,7 @@ export class OutgoingRequests {
return { err: new Error(`Shut down with ${String(unfinished)} request(s) unfinished`) }; return { err: new Error(`Shut down with ${String(unfinished)} request(s) unfinished`) };
} }
/** Nothing reached the socket, so the next link carries it instead of the caller resending. */ /** Nothing reached the socket, so the next link carries it. */
private retriesOnNextLink(attempt: Attempt): boolean { private retriesOnNextLink(attempt: Attempt): boolean {
// Until the link is dropped it admits the retry straight back onto the dead socket, and the loop spins. // Until the link is dropped it admits the retry straight back onto the dead socket, and the loop spins.
return attempt.retryOnNextLink && this.link.awaitsNextLink(); return attempt.retryOnNextLink && this.link.awaitsNextLink();
+4 -4
View File
@@ -247,8 +247,8 @@ export class Session extends EventEmitter<SessionEvents> {
} }
/** /**
* Closes for good: refuses new sends, waits out the requests already on the wire up to * Closes for good: refuses new sends, waits up to `shutdownTimeout` for the requests already sent
* `shutdownTimeout`, then tears down whatever is left. A session closed this way never reconnects. * and the messages not yet answered, then tears down whatever is left. A session closed this way never reconnects.
*/ */
async close(options: CloseOptions = {}): Promise<VoidResult> { async close(options: CloseOptions = {}): Promise<VoidResult> {
const drained = await this.drain(options.signal); const drained = await this.drain(options.signal);
@@ -324,7 +324,7 @@ export class Session extends EventEmitter<SessionEvents> {
private async drain(signal: AbortSignal | undefined): Promise<VoidResult> { private async drain(signal: AbortSignal | undefined): Promise<VoidResult> {
this.stop(); this.stop();
// No link, so nothing is on the wire to wait out. // No bound link, so nothing is on the wire to wait out.
if (!this.outgoing.canCarry()) return {}; if (!this.outgoing.canCarry()) return {};
const timeout = this.options.shutdownTimeout ?? defaults.shutdownTimeout; const timeout = this.options.shutdownTimeout ?? defaults.shutdownTimeout;
@@ -385,7 +385,7 @@ export class Session extends EventEmitter<SessionEvents> {
this.incoming.clear(); this.incoming.clear();
this.sock.destroy(); this.sock.destroy();
// Not re-read: clear() reports lost segments, and a listener can stop the session in between. // `lost` is read before clear(): a listener it reaches may close() the session, and the drop still reports as disconnected.
if (lost === 'disconnected') this.emit('disconnected'); if (lost === 'disconnected') this.emit('disconnected');
else this.emitClose(); else this.emitClose();
} }
+10 -1
View File
@@ -378,7 +378,16 @@ hardest, and the held-message flow across `incoming-requests.ts`, `held-messages
`error`-event reason (hard rule 3 owns it) and the Audience bullets restating goal 8 and `error`-event reason (hard rule 3 owns it) and the Audience bullets restating goal 8 and
Install; the `'use strict'` clause in both MIGRATION.md and CHANGELOG.md; the node-smpp Install; the `'use strict'` clause in both MIGRATION.md and CHANGELOG.md; the node-smpp
cross-check in MIGRATION.md; the planned work in `interop-tests/AGENTS.md` (an expected cross-check in MIGRATION.md; the planned work in `interop-tests/AGENTS.md` (an expected
malformed count per peer) and `benchmarks/README.md`. malformed count per peer) and `benchmarks/README.md`. The prose sweep of #48 adds: the smppload
note in both `benchmarks/README.md` and `interop-tests/README.md`; the summary after the
`AGENTS.md` link in `interop-tests/README.md`; the `run.py` foreground rule tacked onto rule 5 in
`interop-tests/AGENTS.md`, which wants its own number.
- [ ] **Give this library one figure at window 50 in `benchmarks/README.md`.** Its "same sink, same
host" table reads 37,125/s where the table below it reads 38,675/s; re-measure or cite one run.
- [ ] **Name the goal and the premise of every `docs/decisions.md` entry.** The prose sweep of #48
counted 30 of 58 entries naming no goal and 52 with no "valid while" premise.
- [ ] **Make `LinkLife` start unbound, or its decision's title true.** A link attached but not - [ ] **Make `LinkLife` start unbound, or its decision's title true.** A link attached but not
yet bound cannot carry a request, while `phase` starts `up`, so the first link and a server yet bound cannot carry a request, while `phase` starts `up`, so the first link and a server