diff --git a/CHANGELOG.md b/CHANGELOG.md index 0eee28d..6dc91e2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ # Changelog -## Unreleased +## 0.6.0 (unreleased) - `client()` now bounds each connect attempt at 10 seconds, the TLS handshake included, and reports one that expires as an ordinary connect failure, so `reconnect` retries it on its usual backoff. diff --git a/README.md b/README.md index eb66a1d..3cf199d 100644 --- a/README.md +++ b/README.md @@ -227,7 +227,7 @@ All optional. Timeouts and delays are milliseconds. | `interfaceVersion` | `0x34` | The SMPP version declared at bind. `0x50` for an SMSC that requires SMPP 5.0. | | `systemType`, `addressRange`, `addrTon`, `addrNpi` | `''`, `''`, `0`, `0` | The remaining bind fields, for operators that require them. | | `tls` | `false` | `true` for defaults, or a `tls.ConnectionOptions` object for a private CA or a client certificate. | -| `connectTimeout` | `10000` | Give up on **each connect attempt** the SMSC never completes, the TLS handshake included, and report it as an ordinary connect failure, which `reconnect` then retries. It bounds one attempt, never the `client()` call. `false` waits the operating system out instead, around 130 s on Linux. | +| `connectTimeout` | `10000` | Give up on **each connect attempt** the SMSC never completes, the TLS handshake included, and report it as an ordinary connect failure, which `reconnect` then retries. It bounds the socket and the handshake — never the `client()` call, and never the wait for the bind response, which is `responseTimeout`. `false` waits the operating system out instead, around 130 s on Linux; `0` is refused. | | `enquireLinkInterval` | `20000` | Interval between `enquire_link` on a quiet link. | | `idleTimeout` | `2 × enquireLinkInterval` | Give up on a link the peer has stopped answering, and re-bind unless `reconnect` is `false`. | | `responseTimeout` | `30000` | How long to wait for a response, and how long a send with no link waits for the next one. `0` waits forever. | diff --git a/src/client.ts b/src/client.ts index ab61ff9..f20e831 100644 --- a/src/client.ts +++ b/src/client.ts @@ -41,7 +41,7 @@ export type ClientOptions = { username?: string; }; -export const defaults = { +const defaults = { bindType: 'transceiver', connectTimeout: 10_000, enquireLinkInterval: 20_000, @@ -57,19 +57,22 @@ export const defaults = { function armConnectTimeout( sock: Socket, connectTimeout: number | false, - peer: string, + target: { peer: string; secure: boolean }, settle: (result: Result<{ sock: Socket }>) => void, ): NodeJS.Timeout | undefined { if (connectTimeout === false) return undefined; - // A firewall and a stalled handshake need different answers, and only the phase tells them apart. - let phase = `connecting to ${peer}`; + let phase = `connecting to ${target.peer}`; - sock.once('connect', () => { phase = `completing the TLS handshake with ${peer}`; }); + if (target.secure) { + sock.once('connect', () => { phase = `completing the TLS handshake with ${target.peer}`; }); + } return setTimeout(() => { sock.destroy(); - settle({ err: new Error(`Timed out ${phase} after ${String(connectTimeout)} ms`) }); + settle({ + err: new Error(`Timed out ${phase} after ${String(connectTimeout)} ms; raise connectTimeout or set it to false`), + }); }, connectTimeout).unref(); } @@ -99,12 +102,14 @@ function openSocket(options: ClientOptions): Promise> { return; } - const settle = (result: Result<{ sock: Socket }>): void => { + const timer = armConnectTimeout(sock, connectTimeout, { peer: `${host}:${String(port)}`, secure }, settle); + + function settle(result: Result<{ sock: Socket }>): void { clearTimeout(timer); sock.removeListener('error', onError); signal?.removeEventListener('abort', onAbort); resolve(result); - }; + } function onError(err: Error): void { settle({ err }); @@ -120,8 +125,6 @@ function openSocket(options: ClientOptions): Promise> { sock.once(secure ? 'secureConnect' : 'connect', () => { settle({ sock }); }); - - const timer = armConnectTimeout(sock, connectTimeout, `${host}:${String(port)}`, settle); }); } diff --git a/src/error-from.ts b/src/error-from.ts index b2ccbd4..1ae1f4b 100644 --- a/src/error-from.ts +++ b/src/error-from.ts @@ -9,7 +9,9 @@ export function errorFrom(reason: unknown): Error { } } -/** String() throws on a null-prototype object or a symbol, so only a string or number is printed. */ +const printable: readonly string[] = ['boolean', 'number', 'string']; + +/** String() throws on a null-prototype object or a symbol, so those are named by type instead. */ export function namedValue(value: unknown): string { - return typeof value === 'string' || typeof value === 'number' ? String(value) : typeof value; + return printable.includes(typeof value) ? String(value) : typeof value; } diff --git a/src/session-options.ts b/src/session-options.ts index 01b7372..fd53842 100644 --- a/src/session-options.ts +++ b/src/session-options.ts @@ -180,7 +180,7 @@ function checkConnectTimeout(connectTimeout: unknown): VoidResult { } if (connectTimeout > maxTimerDelay) { - return { err: new Error(`connectTimeout must be ${String(maxTimerDelay)} ms or less (about 24 days), got ${got}`) }; + return { err: new Error(`connectTimeout must be ${String(maxTimerDelay)} ms or less (about 24 days), got ${got}; false waits the OS out instead`) }; } return {}; diff --git a/test/session-extras.test.ts b/test/session-extras.test.ts index 2f54f77..d9e33ee 100644 --- a/test/session-extras.test.ts +++ b/test/session-extras.test.ts @@ -25,7 +25,7 @@ import { DlrMerger } from '../src/dlr-merger.ts'; import { PduRefusedError } from '../src/pdu-refusal.ts'; import { objToPdu } from '../src/pdu.ts'; import { checkSessionOptions, standsInFor } from '../src/session-options.ts'; -import { client, defaults as clientDefaults } from '../src/client.ts'; +import { client } from '../src/client.ts'; import { closeAfter, closeListenerAfter } from './teardown.ts'; import { concatOf } from '../src/concat.ts'; import { consts } from '../src/defs/constants.ts'; @@ -1049,12 +1049,35 @@ describe('connectTimeout', () => { assert.ok(settled.err instanceof Error); assert.match( settled.err.message, - new RegExp(`Timed out completing the TLS handshake with 127\\.0\\.0\\.1:${String(port)} after 150 ms`), - 'a firewall and a peer that accepts then stalls need different answers from the operator', + new RegExp(`Timed out completing the TLS handshake with 127\\.0\\.0\\.1:${String(port)} after 150 ms; raise connectTimeout`), + 'a firewall and a peer that accepts then stalls need different answers, and whoever reads this has never heard of the option', ); assert.equal(settled.session, undefined); }); + // The fallback is what every call that names no timeout gets, and localhost settles too fast to see it. + test('bounds a connect nobody asked to bound, at the default', async t => { + const { accepted, port } = await stalledListener(t); + + t.mock.timers.enable({ apis: ['setTimeout'] }); + + const connecting = client({ host: '127.0.0.1', port, reconnect: false, tls: true }); + + while (accepted.length === 0) { + await new Promise(resolve => setImmediate(resolve)); + } + + await new Promise(resolve => setImmediate(resolve)); + t.mock.timers.tick(10_000); + t.mock.timers.reset(); + + const settled = await within(2000, connecting); + + assert.ok(settled, 'no bound was armed, so nothing ever settled this connect'); + assert.ok(settled.err instanceof Error); + assert.match(settled.err.message, /after 10000 ms/); + }); + test('retries a connect it timed out on, like any other failed attempt', async t => { const { accepted, port } = await stalledListener(t); const controller = new AbortController(); @@ -1087,7 +1110,6 @@ describe('connectTimeout', () => { }); test('refuses a connect timeout that would turn itself off', async () => { - assert.equal(clientDefaults.connectTimeout, 10_000, 'the number the README documents'); assert.match( checkSessionOptions({ connectTimeout: 0 }).err?.message ?? '', /false waits the OS out/, diff --git a/todo.md b/todo.md index 3de7bbf..5c119c8 100644 --- a/todo.md +++ b/todo.md @@ -190,7 +190,9 @@ the rewrite, for a dependency added later. Maintainer's call, 2026-09-14. `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. + already, which is the asymmetry to close. The same four print an untyped value bare, so + `idleTimeout: '5000'` is refused with `got 5000` — a value the reader reads as correct — where + `connectTimeout` quotes it. 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