Split a retried drop out of close, and range-check the backoff

`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.
This commit is contained in:
2026-08-31 21:11:19 +02:00
parent 18177053bc
commit 880454b4f4
6 changed files with 111 additions and 8 deletions
+7
View File
@@ -288,6 +288,13 @@ exactly 140.
and fail on every developer machine, and a committed key leaks in a public repository. Valid while and fail on every developer machine, and a committed key leaks in a public repository. Valid while
the dev image has no openssl. 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: - **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 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 application that never read the options table got none of it. `reconnect` takes
+3 -2
View File
@@ -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. | | `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`. | | `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 `<base>-<n>`, 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. | | `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 `<base>-<n>`, 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. | | `close` | The session is over: you closed it, or the link dropped with `reconnect: false`. Nothing brings it back. |
| `reconnected` | The client re-bound after a drop. Never fires with `reconnect: false`. | | `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. | | `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. | | `data` | Raw bytes arrived on the socket. |
| `incomingPdu` | A complete PDU arrived, as a buffer. | | `incomingPdu` | A complete PDU arrived, as a buffer. |
+34 -3
View File
@@ -12,6 +12,7 @@ import { isSmsIdNotation, smsIdNotations, smsIdPlaces } from './sms-id.ts';
export type SessionEvents = { export type SessionEvents = {
close: []; close: [];
data: [Buffer]; data: [Buffer];
disconnected: [];
dlr: [Dlr, PduObject]; dlr: [Dlr, PduObject];
incomingPdu: [Buffer]; incomingPdu: [Buffer];
incomingPduObj: [PduObject]; incomingPduObj: [PduObject];
@@ -116,28 +117,57 @@ export const defaults = {
* queued behind a slot that is never freed, so the call never settles at all. * queued behind a slot that is never freed, so the call never settles at all.
*/ */
export function checkSessionOptions(options: CheckableOptions): VoidResult { export function checkSessionOptions(options: CheckableOptions): VoidResult {
const limits: [string, number, number][] = [ const checked = checkLimits([
['idleTimeout', options.idleTimeout ?? 0, 0], ['idleTimeout', options.idleTimeout ?? 0, 0],
['maxOutstanding', options.maxOutstanding ?? defaults.maxOutstanding, 1], ['maxOutstanding', options.maxOutstanding ?? defaults.maxOutstanding, 1],
['maxReassembly', options.maxReassembly ?? defaults.maxReassembly, 1], ['maxReassembly', options.maxReassembly ?? defaults.maxReassembly, 1],
['reassemblyTimeout', options.reassemblyTimeout ?? defaults.reassemblyTimeout, 0], ['reassemblyTimeout', options.reassemblyTimeout ?? defaults.reassemblyTimeout, 0],
['responseTimeout', options.responseTimeout ?? defaults.responseTimeout, 0], ['responseTimeout', options.responseTimeout ?? defaults.responseTimeout, 0],
['shutdownTimeout', options.shutdownTimeout ?? defaults.shutdownTimeout, 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) { for (const [name, value, min] of limits) {
if (!Number.isInteger(value) || value < min) { if (!Number.isInteger(value) || value < min) {
return { err: new Error(`${name} must be ${String(min)} or more, got ${String(value)}`) }; 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<string, unknown> { function isRecord(value: unknown): value is Record<string, unknown> {
return typeof value === 'object' && value !== null && !Array.isArray(value); 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 { function checkSmsIdFormat(smsIdFormat: unknown): VoidResult {
if (smsIdFormat === undefined) return {}; if (smsIdFormat === undefined) return {};
@@ -167,6 +197,7 @@ export type CheckableOptions = {
maxOutstanding?: number | undefined; maxOutstanding?: number | undefined;
maxReassembly?: number | undefined; maxReassembly?: number | undefined;
reassemblyTimeout?: number | undefined; reassemblyTimeout?: number | undefined;
reconnect?: unknown;
responseTimeout?: number | undefined; responseTimeout?: number | undefined;
shutdownTimeout?: number | undefined; shutdownTimeout?: number | undefined;
smsIdFormat?: unknown; smsIdFormat?: unknown;
+6 -1
View File
@@ -376,7 +376,12 @@ export class Session extends EventEmitter<SessionEvents> {
this.pending.settleAll(new Error('Session closed before a response arrived')); this.pending.settleAll(new Error('Session closed before a response arrived'));
this.incoming.clear(); this.incoming.clear();
this.sock.destroy(); 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 { private nextConcatReference(): number {
+56 -1
View File
@@ -334,10 +334,65 @@ describe('reconnect', () => {
const reconnected = once<true>(resolve => { session.on('reconnected', () => { resolve(true); }); }); const reconnected = once<true>(resolve => { session.on('reconnected', () => { resolve(true); }); });
const dropped = session.sock;
await peerOf(smpp).close(); await peerOf(smpp).close();
await reconnected; 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<true>(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<true>(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 => { test('schedules nothing after a drop when reconnect is false', async t => {
+5 -1
View File
@@ -1408,7 +1408,11 @@ describe('application hooks that throw or reject', () => {
describe('link timers', () => { describe('link timers', () => {
test('closes a client link the peer has stopped answering', async t => { test('closes a client link the peer has stopped answering', async t => {
const peer = await bindOnlyPeer(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.equal(err, undefined);
assert.ok(session); assert.ok(session);