From 880454b4f4c5fc38d7a3024344f2e871ec14105d Mon Sep 17 00:00:00 2001 From: Lilleman auf Larv Date: Mon, 31 Aug 2026 21:11:19 +0200 Subject: [PATCH] Split a retried drop out of close, and range-check the backoff MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `close` fires only where nothing will bring the session back; a drop the reconnect loop will retry is `disconnected`, pairing with `reconnected`. `minDelay: 0` never doubles, so the backoff never started — both delays are range-checked now, and only `false` spells reconnect off. --- AGENTS.md | 7 +++++ README.md | 5 ++-- src/session-options.ts | 37 ++++++++++++++++++++++-- src/session.ts | 7 ++++- test/session-extras.test.ts | 57 ++++++++++++++++++++++++++++++++++++- test/session.test.ts | 6 +++- 6 files changed, 111 insertions(+), 8 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index af3fa53..d837b4f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -288,6 +288,13 @@ exactly 140. and fail on every developer machine, and a committed key leaks in a public repository. Valid while the dev image has no openssl. +- **`close` means the session is over, and a drop the loop will retry is `disconnected`.** Maintainer's + call, 2026-08-31: with reconnect on by default a `close` on every transient drop left an application + unable to tell a retry from the end, and no second one follows because `teardown()` is a no-op once + `closed`. `teardown()` picks the event by whether the reconnect loop is still live, and `end()` stops + that loop before tearing down, so every deliberate shutdown emits `close`. Without the split an + application that opens a replacement client on `close` ends up holding two binds on one account. + - **A client re-binds after a drop unless it is told not to.** Maintainer's call, 2026-08-31: surviving a dropped link is most of what the session layer is for, and behind an opt-in an application that never read the options table got none of it. `reconnect` takes diff --git a/README.md b/README.md index 49a3d15..52e2148 100644 --- a/README.md +++ b/README.md @@ -316,8 +316,9 @@ TypeScript users can import `SmppLog` to have the compiler check one. | `sms` | An SMS arrives, reassembled if it was multipart. Carries `sendResp()`, `sendDlr()` and the `smsId` it was answered with. | | `dlr` | A delivery report arrives, one per segment. `smsId` is undefined when the peer marked a receipt whose body carries no readable id. `statusMsg` names `statusId` unless the peer sent a `message_state` this library cannot name — then `statusId` is that raw value and `statusMsg` is whatever the body said, or `UNKNOWN`. | | `messageDlr` | Every segment of a multipart message sent with `dlr: true` has been reported on, carrying the worst status of the segments. Merging needs the SMSC to number its segment ids `-`, which is this library's own server's convention — an SMSC that hands out unrelated ids per segment never fires it. A base is merged once: a later message the SMSC gives the same ids is reported on through `dlr` alone, and an earlier one still collecting loses its merged report as well. | -| `close` | The connection closed. | -| `reconnected` | The client re-bound after a drop. Never fires with `reconnect: false`. | +| `close` | The session is over: you closed it, or the link dropped with `reconnect: false`. Nothing brings it back. | +| `disconnected` | The link dropped and the reconnect loop will retry it. Do not open a replacement client here — the session you hold comes back on its own, and `reconnected` says when. | +| `reconnected` | The client re-bound after a drop. | | `sessionError` | Something failed on a live session, including a hook or listener that threw or, if it was `async`, rejected. | | `data` | Raw bytes arrived on the socket. | | `incomingPdu` | A complete PDU arrived, as a buffer. | diff --git a/src/session-options.ts b/src/session-options.ts index 9905f14..9c47b9b 100644 --- a/src/session-options.ts +++ b/src/session-options.ts @@ -12,6 +12,7 @@ import { isSmsIdNotation, smsIdNotations, smsIdPlaces } from './sms-id.ts'; export type SessionEvents = { close: []; data: [Buffer]; + disconnected: []; dlr: [Dlr, PduObject]; incomingPdu: [Buffer]; incomingPduObj: [PduObject]; @@ -116,28 +117,57 @@ export const defaults = { * queued behind a slot that is never freed, so the call never settles at all. */ export function checkSessionOptions(options: CheckableOptions): VoidResult { - const limits: [string, number, number][] = [ + const checked = checkLimits([ ['idleTimeout', options.idleTimeout ?? 0, 0], ['maxOutstanding', options.maxOutstanding ?? defaults.maxOutstanding, 1], ['maxReassembly', options.maxReassembly ?? defaults.maxReassembly, 1], ['reassemblyTimeout', options.reassemblyTimeout ?? defaults.reassemblyTimeout, 0], ['responseTimeout', options.responseTimeout ?? defaults.responseTimeout, 0], ['shutdownTimeout', options.shutdownTimeout ?? defaults.shutdownTimeout, 0], - ]; + ]); + if (checked.err) return checked; + + const backoff = checkReconnect(options.reconnect); + + return backoff.err ? backoff : checkSmsIdFormat(options.smsIdFormat); +} + +function checkLimits(limits: [string, number, number][]): VoidResult { for (const [name, value, min] of limits) { if (!Number.isInteger(value) || value < min) { return { err: new Error(`${name} must be ${String(min)} or more, got ${String(value)}`) }; } } - return checkSmsIdFormat(options.smsIdFormat); + return {}; +} + +function checkReconnect(reconnect: unknown): VoidResult { + if (reconnect === undefined || reconnect === false) return {}; + + if (!isRecord(reconnect)) { + return { err: new Error('reconnect takes { maxDelay, minDelay }, or false to turn it off') }; + } + + // A minDelay of 0 never doubles, so the backoff never starts and every retry lands at once. + return checkLimits([ + ['maxDelay', delayOr(reconnect.maxDelay, defaults.maxDelay), 1], + ['minDelay', delayOr(reconnect.minDelay, defaults.minDelay), 1], + ]); } function isRecord(value: unknown): value is Record { return typeof value === 'object' && value !== null && !Array.isArray(value); } +/** A tuning value that is not a number lands on NaN, which the range check refuses by name. */ +function delayOr(value: unknown, fallback: number): number { + if (value === undefined) return fallback; + + return typeof value === 'number' ? value : NaN; +} + function checkSmsIdFormat(smsIdFormat: unknown): VoidResult { if (smsIdFormat === undefined) return {}; @@ -167,6 +197,7 @@ export type CheckableOptions = { maxOutstanding?: number | undefined; maxReassembly?: number | undefined; reassemblyTimeout?: number | undefined; + reconnect?: unknown; responseTimeout?: number | undefined; shutdownTimeout?: number | undefined; smsIdFormat?: unknown; diff --git a/src/session.ts b/src/session.ts index e18fd53..1dbb84a 100644 --- a/src/session.ts +++ b/src/session.ts @@ -376,7 +376,12 @@ export class Session extends EventEmitter { this.pending.settleAll(new Error('Session closed before a response arrived')); this.incoming.clear(); this.sock.destroy(); - this.emit('close'); + this.emit(this.retrying() ? 'disconnected' : 'close'); + } + + /** A drop the loop will bring the session back from is a disconnect, not the end of it. */ + private retrying(): boolean { + return this.reconnectLoop !== undefined && !this.reconnectLoop.isStopped(); } private nextConcatReference(): number { diff --git a/test/session-extras.test.ts b/test/session-extras.test.ts index 0aa1f9f..611024e 100644 --- a/test/session-extras.test.ts +++ b/test/session-extras.test.ts @@ -334,10 +334,65 @@ describe('reconnect', () => { const reconnected = once(resolve => { session.on('reconnected', () => { resolve(true); }); }); + const dropped = session.sock; + await peerOf(smpp).close(); await reconnected; - assert.ok(session.loggedIn); + assert.notEqual(session.sock, dropped, 'the session should be live on a fresh socket'); + }); + + test('reports a drop it will retry as disconnected, keeping close for the end', async t => { + const smpp = await startServer(t); + const { session } = await connect(t, smpp); + + assert.ok(session); + + const events: string[] = []; + + session.on('close', () => { events.push('close'); }); + session.on('disconnected', () => { events.push('disconnected'); }); + + const reconnected = once(resolve => { session.on('reconnected', () => { resolve(true); }); }); + + await peerOf(smpp).close(); + await reconnected; + + assert.deepEqual(events, ['disconnected'], 'a link the loop brings back is not the end'); + + await session.close(); + + assert.deepEqual(events, ['disconnected', 'close']); + }); + + test('reports a drop as close when nothing will retry it', async t => { + const smpp = await startServer(t); + const { session } = await connect(t, smpp, { reconnect: false }); + + assert.ok(session); + + const events: string[] = []; + + session.on('disconnected', () => { events.push('disconnected'); }); + + const closed = once(resolve => { session.on('close', () => { resolve(true); }); }); + + await peerOf(smpp).close(); + await closed; + + assert.deepEqual(events, []); + }); + + test('refuses a backoff that would retry without pausing', () => { + assert.match(checkSessionOptions({ reconnect: { minDelay: 0 } }).err?.message ?? '', /minDelay/); + assert.match(checkSessionOptions({ reconnect: { maxDelay: -1 } }).err?.message ?? '', /maxDelay/); + assert.match( + checkSessionOptions({ reconnect: null }).err?.message ?? '', + /false/, + 'off is spelled false, so nothing else may stand in for it', + ); + assert.equal(checkSessionOptions({ reconnect: false }).err, undefined); + assert.equal(checkSessionOptions({ reconnect: { maxDelay: 60_000, minDelay: 500 } }).err, undefined); }); test('schedules nothing after a drop when reconnect is false', async t => { diff --git a/test/session.test.ts b/test/session.test.ts index 5565ef5..5165d1e 100644 --- a/test/session.test.ts +++ b/test/session.test.ts @@ -1408,7 +1408,11 @@ describe('application hooks that throw or reject', () => { describe('link timers', () => { test('closes a client link the peer has stopped answering', async t => { const peer = await bindOnlyPeer(t); - const { err, session } = await client({ enquireLinkInterval: 50, port: peer.port }); + const { err, session } = await client({ + enquireLinkInterval: 50, + port: peer.port, + reconnect: false, + }); assert.equal(err, undefined); assert.ok(session);