Fold the session's closed and ended flags into one lifecycle state #41

Merged
lilleman merged 3 commits from session-owns-bind into main 2026-09-28 02:52:33 +02:00
3 changed files with 50 additions and 18 deletions
Showing only changes of commit 20aa2828c2 - Show all commits
+2 -2
View File
@@ -87,8 +87,8 @@ src/
```
Imports point one way: `defs` knows nothing above it but `result.ts`, `pdu` uses `defs`, `session`
uses `pdu`, and `client`/`server` use `session`. The one way back up is the `Session` handed to
`createSms()`, imported as a type only.
uses `pdu`, and `client`/`server` use `session`. The ways back up are the `Session` handed to
`createSms()` and to the hooks `session-options.ts` types, both imported as a type only.
**Parameter order is wire order.** The key order inside `cmds.*.params` is the order the fields are
written to and read from the buffer. Never sort those alphabetically — the alphabetical-ordering
+13 -13
View File
@@ -72,8 +72,8 @@ export class Session extends EventEmitter<SessionEvents> {
private readonly timers: LinkTimers;
private readonly transport: PduTransport;
private closed = false;
private ended = false;
/** `torn-down` goes back to `attached` when the reconnect loop brings a link up. */
private link: 'attached' | 'ended' | 'torn-down' = 'attached';
/** A listener that throws is the application's bug; it must not become ours. Hard rule 1. */
override emit<K extends keyof SessionEvents>(
@@ -186,7 +186,7 @@ export class Session extends EventEmitter<SessionEvents> {
const sent = built.err ? { err: built.err } : this.transport.write(built.buffer);
// A peer that unbinds and drops the link takes our response with it; that is not a failure.
if (sent.err && !this.closed) {
if (sent.err && this.link === 'attached') {
this.log.warn('session - could not answer a request', {
cmdName,
message: sent.err.message,
@@ -221,12 +221,12 @@ export class Session extends EventEmitter<SessionEvents> {
*/
async unbind(): Promise<VoidResult> {
const drained = await this.drain(undefined);
const wasOpen = !this.closed;
const wasOpen = this.link === 'attached';
// now(), not send(): a drain refuses a send, and the unbind goes out either way.
const sent = wasOpen
? await this.outgoing.now({ cmdName: 'unbind' })
: { err: new Error('Session is closed') };
const closedOnUnbind = wasOpen && this.closed;
const closedOnUnbind = wasOpen && this.link !== 'attached';
this.end();
@@ -325,7 +325,7 @@ export class Session extends EventEmitter<SessionEvents> {
private attach(sock: Socket): void {
this.transport.attach(sock);
this.closed = false;
this.link = 'attached';
}
/** Stops new sends and waits out the messages we hold and the requests already issued. */
@@ -371,17 +371,17 @@ export class Session extends EventEmitter<SessionEvents> {
}
private emitClose(): void {
if (this.ended) return;
if (this.link === 'ended') return;
this.ended = true;
this.link = 'ended';
this.outgoing.linkLost(false);
this.emit('close');
}
private teardown(): void {
if (this.closed) return;
if (this.link !== 'attached') return;
this.closed = true;
this.link = 'torn-down';
// Read once: clear() reports lost segments, and a listener could stop the loop between reads.
const retrying = this.retrying();
@@ -445,15 +445,15 @@ export class Session extends EventEmitter<SessionEvents> {
}
private resetTimers(): void {
if (this.closed) return;
if (this.link !== 'attached') return;
this.timers.reset();
}
private onClose(): void {
if (this.reconnectLoop && !this.reconnectLoop.isStopped()) {
if (this.retrying()) {
this.teardown();
this.reconnectLoop.schedule();
this.reconnectLoop?.schedule();
return;
}
+35 -3
View File
@@ -199,9 +199,24 @@ next work ([decision](docs/decisions.md#internals-and-tests)).
### Locality — next, ahead of everything below; 5–6 today, and the gate is 7
- [ ] **Lift the held-message and shutdown code to Locality 7, and confirm it with a scoring run.**
The 2026-09-27 run capped every seat there; a run reading 7.0 or above also retires the #30
decision.
A second four-seat run on 2026-09-28, after #35–#40, read 6, 6, 7 and 6 again, Locality 5, 5, 6
and 5. Every seat ranked the session's lifecycle hardest and least wanted to modify it.
- [ ] **Lift Locality to 7, and confirm it with a scoring run.** A run reading 7.0 or above also
retires the #30 decision. The sub-items are what the 2026-09-28 run named, most seats first.
- [ ] **Let `Session` own its bind state.** `client.ts` and `server.ts` write `boundAs`, `loggedIn`
and `peerInterfaceVersion` onto the session from outside, and `loggedIn` duplicates
`boundAs !== undefined`. A method such as `session.bound(bindType, declaredVersion)` with the
three fields read-only changes the public surface a hand-wired SMSC uses, so it needs the
maintainer's call first. All four seats.
- [ ] **Name what `request()`, `carry()` and `now()` each skip.** Three ways onto the wire differ
only in which of the drain, the gate and the window they bypass, and none of the names says
which. Three seats.
- [ ] **Give the held-message flow one place a reader can follow it.** Whether a drain still waits
on a message is spread over `emitSms()`, `MessageHold`, the session's rejection route and
`Sms.isHeld()`. Four seats.
- [ ] **Shrink the `IncomingDeps` closure bag.** 16 lambdas, six of them repeated in
`SmsHandlers`, which makes every inbound call path indirect. Two seats.
### Correctness
@@ -244,6 +259,23 @@ next work ([decision](docs/decisions.md#internals-and-tests)).
README.md or in MIGRATION.md — write it in the same change as the rule, so it is worded once.
From the stability and product-owner reviews of #18.
- [ ] **Build every error from a thrown value through `errorFrom()`.** `reconnect-loop.ts` (in
`run()`'s catch and in `bringUp()`) and `client.ts`'s connect still call `String(thrown)`,
which throws on a null-prototype object; in `run()` that lands as an unhandled rejection from
a `void`ed promise, against hard rule 1. From the 2026-09-28 scoring run.
- [ ] **Leave `sms.smsId` alone when `sendResp()` fails.** `sms.ts` assigns `options.smsId` before
the lost-link check and before the write, so a failed answer still renames the message and a
later `sendDlr()` names an id the peer was never given. From the 2026-09-28 scoring run.
- [ ] **Refuse a timeout past 2³¹−1 ms, as `connectTimeout` already is.** `checkLimits()` bounds
`responseTimeout`, `idleTimeout`, `shutdownTimeout` and `reassemblyTimeout` from below only,
and Node fires a larger delay after 1 ms. From the 2026-09-28 scoring run.
- [ ] **Keep a bare ESC out of GSM detection.** `gsmRegex` in `defs/encodings.ts` admits `\x1B`,
so `"\x1B("` is detected as GSM, goes out as 0x1B 0x28 and arrives as `{`. From the
2026-09-28 scoring run.
### Throughput — goal 6, and the default window is where we are slowest
- [ ] **Close the gap to jsmpp at `maxOutstanding: 10`.** Measured 2026-09-20 against the same sink,