diff --git a/AGENTS.md b/AGENTS.md index 25783ae..1f0829e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -214,8 +214,8 @@ Each file answers one question, and a fact belongs to the file whose question it - **README.md — what you can rely on, and where this is heading.** Observable behaviour, for someone using the package, plus the goals and the audience. It carries a reason only where the reason changes how you would call the thing. -- **CHANGELOG.md — what changed for a consumer, per release.** Written for whoever takes the package - off npm, never for the next agent, and a line lands there as the work ships rather than at release. +- **CHANGELOG.md — what changed for a consumer, per release.** Written for the public, never for the + next agent, and a line lands there as the work ships rather than at release. - **MIGRATION.md — what a 0.4.0 consumer has to change.** Renamed and removed surface, and the behaviour that changed on the wire. - **AGENTS.md — what may not change, and why.** Hard rules, architecture, conventions, and an index diff --git a/docs/decisions.md b/docs/decisions.md index 9596801..b2bfec7 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -534,8 +534,9 @@ rule and an index of the titles below. `secureConnect` for a TLS socket, so a peer that accepts and then says nothing is bounded the same way a black-holed SYN is. Rejected: `socket.setTimeout()`, an idle timeout that goes on arming once the link is up. Rejected: bounding it with `responseTimeout`, which names the wait for an - answer on a link that already exists and would retune both at once. Open while it has no default: - whether a later major gives it one. + answer on a link that already exists and would retune both at once. `server()` shares the checker + and ignores the option, as it already ignores `reconnect` — nothing at that end connects out. Open + while it has no default: whether a later major gives it one. - **A stream this library cannot frame is a dead link; one PDU it cannot parse is not.** Maintainer's call, 2026-08-31, narrowed 2026-09-05 via the interop plan: a `command_length` below diff --git a/src/client.ts b/src/client.ts index 1f52457..0894270 100644 --- a/src/client.ts +++ b/src/client.ts @@ -94,8 +94,7 @@ function openSocket(options: ClientOptions): Promise> { } const settle = (result: Result<{ sock: Socket }>): void => { - if (timer) clearTimeout(timer); - + clearTimeout(timer); sock.removeListener('error', onError); signal?.removeEventListener('abort', onAbort); resolve(result); diff --git a/src/session-options.ts b/src/session-options.ts index 23aed0f..46592c4 100644 --- a/src/session-options.ts +++ b/src/session-options.ts @@ -168,15 +168,23 @@ function limitsOf(options: CheckableOptions): [string, number, number][] { ]; } -/** Leaving it out is how the wait stays the OS's, so 0 would be a second spelling for that. */ +/** Node's timers are 32-bit, and a delay above this one fires after 1 ms instead of never. */ +const maxTimerDelay = 2_147_483_647; + function checkConnectTimeout(connectTimeout: number | undefined): VoidResult { if (connectTimeout === undefined) return {}; - if (Number.isInteger(connectTimeout) && connectTimeout >= 1) return {}; - const got = String(connectTimeout); - return { err: new Error(`connectTimeout must be 1 or more, got ${got}; omit it to wait the OS out`) }; + if (!Number.isInteger(connectTimeout) || connectTimeout < 1) { + return { err: new Error(`connectTimeout must be a whole number of milliseconds, 1 or more, got ${got}; omit it to wait the OS out`) }; + } + + if (connectTimeout > maxTimerDelay) { + return { err: new Error(`connectTimeout must be ${String(maxTimerDelay)} or less, got ${got}`) }; + } + + return {}; } function checkLimits(limits: [string, number, number][]): VoidResult { diff --git a/test/session-extras.test.ts b/test/session-extras.test.ts index eedd257..00cc60f 100644 --- a/test/session-extras.test.ts +++ b/test/session-extras.test.ts @@ -1022,7 +1022,10 @@ describe('connectTimeout', () => { /** Accepts and then says nothing, so a TLS handshake started on it never completes. */ async function stalledListener(t: TestContext): Promise<{ accepted: net.Socket[]; port: number }> { const accepted: net.Socket[] = []; - const listener = net.createServer(sock => { accepted.push(sock); }); + const listener = net.createServer(sock => { + accepted.push(sock); + sock.resume(); + }); closeListenerAfter(t, listener, accepted); await new Promise(resolve => { listener.listen(0, '127.0.0.1', resolve); }); @@ -1034,7 +1037,6 @@ describe('connectTimeout', () => { test('gives up on a connect the peer never completes', async t => { const { port } = await stalledListener(t); - const started = Date.now(); const settled = await within(2000, client({ connectTimeout: 150, host: '127.0.0.1', @@ -1047,7 +1049,6 @@ describe('connectTimeout', () => { assert.ok(settled.err instanceof Error); assert.match(settled.err.message, /Timed out connecting/); assert.equal(settled.session, undefined); - assert.ok(Date.now() - started >= 150, 'the timeout is what settles it, not a socket error'); }); test('retries a connect it timed out on, like any other failed attempt', async t => { @@ -1070,10 +1071,10 @@ describe('connectTimeout', () => { test('disarms on the connect that completed, rather than on the socket that follows it', async t => { const smpp = await startServer(t); - const { session } = await connect(t, smpp, { connectTimeout: 50 }); + const { session } = await connect(t, smpp, { connectTimeout: 200 }); assert.ok(session); - await delay(150); + await delay(300); const probe = await session.send({ cmdName: 'enquire_link' }); @@ -1088,12 +1089,19 @@ describe('connectTimeout', () => { 'off is spelled by leaving it out, so 0 may not stand in for it', ); assert.match(checkSessionOptions({ connectTimeout: -1 }).err?.message ?? '', /connectTimeout/); - assert.match(checkSessionOptions({ connectTimeout: 1.5 }).err?.message ?? '', /connectTimeout/); + assert.match(checkSessionOptions({ connectTimeout: 1.5 }).err?.message ?? '', /whole number/); + assert.match( + checkSessionOptions({ connectTimeout: 2_147_483_648 }).err?.message ?? '', + /2147483647 or less/, + 'a delay Node cannot hold in 32 bits fires after 1 ms, the inverse of what it asked for', + ); + assert.equal(checkSessionOptions({ connectTimeout: 2_147_483_647 }).err, undefined); assert.equal(checkSessionOptions({ connectTimeout: 1000 }).err, undefined); const refused = await client({ connectTimeout: 0, port: 1 }); assert.ok(refused.err instanceof Error); + assert.match(refused.err.message, /omit it/, 'the socket may not be opened before the option is refused'); assert.equal(refused.session, undefined); }); }); diff --git a/todo.md b/todo.md index fabfb18..00875e1 100644 --- a/todo.md +++ b/todo.md @@ -185,6 +185,12 @@ the rewrite, for a dependency added later. Maintainer's call, 2026-09-14. unit until 1.0.0. Needs a number an operator would recognise, where SwiftHero's fork picked 10 s. +- [ ] **Refuse a delay Node's timers cannot hold, in `checkLimits`.** `idleTimeout`, + `reassemblyTimeout`, `responseTimeout` and `shutdownTimeout` take any integer, and `setTimeout` + fires after 1 ms for anything above 2147483647 — so a value in the wrong unit gets the inverse + of what it asked for, explained only by a warning on stderr. `connectTimeout` refuses one + already, which is the asymmetry to close. Raised by review, 2026-09-20. + - [ ] **A send the codec will refuse waits for a link and a window slot first.** `refuse()` in `outgoing-requests.ts` runs `misuse()` and the abort check before the wait, precisely so a call that can never go out does not queue for what it will never use; a body `objToPdu()` refuses on