Prove the default bound is wired, and name the option on the failure a consumer reads
Mirror / push (push) Successful in 4s
Test / lint (pull_request) Successful in 22s
Test / test (18) (pull_request) Failing after 19s
Test / test (20) (pull_request) Successful in 19s
Test / test (22) (pull_request) Successful in 24s
Test / test (24) (pull_request) Successful in 20s
Test / test (26) (pull_request) Successful in 19s

This commit is contained in:
2026-09-20 20:41:45 +02:00
parent 1416615ca3
commit a24f350b7a
7 changed files with 49 additions and 20 deletions
+1 -1
View File
@@ -1,6 +1,6 @@
# Changelog # Changelog
## Unreleased ## 0.6.0 (unreleased)
- `client()` now bounds each connect attempt at 10 seconds, the TLS handshake included, and reports - `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. one that expires as an ordinary connect failure, so `reconnect` retries it on its usual backoff.
+1 -1
View File
@@ -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. | | `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. | | `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. | | `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. | | `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`. | | `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. | | `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. |
+13 -10
View File
@@ -41,7 +41,7 @@ export type ClientOptions = {
username?: string; username?: string;
}; };
export const defaults = { const defaults = {
bindType: 'transceiver', bindType: 'transceiver',
connectTimeout: 10_000, connectTimeout: 10_000,
enquireLinkInterval: 20_000, enquireLinkInterval: 20_000,
@@ -57,19 +57,22 @@ export const defaults = {
function armConnectTimeout( function armConnectTimeout(
sock: Socket, sock: Socket,
connectTimeout: number | false, connectTimeout: number | false,
peer: string, target: { peer: string; secure: boolean },
settle: (result: Result<{ sock: Socket }>) => void, settle: (result: Result<{ sock: Socket }>) => void,
): NodeJS.Timeout | undefined { ): NodeJS.Timeout | undefined {
if (connectTimeout === false) return 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 ${target.peer}`;
let phase = `connecting to ${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(() => { return setTimeout(() => {
sock.destroy(); 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(); }, connectTimeout).unref();
} }
@@ -99,12 +102,14 @@ function openSocket(options: ClientOptions): Promise<Result<{ sock: Socket }>> {
return; 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); clearTimeout(timer);
sock.removeListener('error', onError); sock.removeListener('error', onError);
signal?.removeEventListener('abort', onAbort); signal?.removeEventListener('abort', onAbort);
resolve(result); resolve(result);
}; }
function onError(err: Error): void { function onError(err: Error): void {
settle({ err }); settle({ err });
@@ -120,8 +125,6 @@ function openSocket(options: ClientOptions): Promise<Result<{ sock: Socket }>> {
sock.once(secure ? 'secureConnect' : 'connect', () => { sock.once(secure ? 'secureConnect' : 'connect', () => {
settle({ sock }); settle({ sock });
}); });
const timer = armConnectTimeout(sock, connectTimeout, `${host}:${String(port)}`, settle);
}); });
} }
+4 -2
View File
@@ -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 { 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;
} }
+1 -1
View File
@@ -180,7 +180,7 @@ function checkConnectTimeout(connectTimeout: unknown): VoidResult {
} }
if (connectTimeout > maxTimerDelay) { 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 {}; return {};
+26 -4
View File
@@ -25,7 +25,7 @@ import { DlrMerger } from '../src/dlr-merger.ts';
import { PduRefusedError } from '../src/pdu-refusal.ts'; import { PduRefusedError } from '../src/pdu-refusal.ts';
import { objToPdu } from '../src/pdu.ts'; import { objToPdu } from '../src/pdu.ts';
import { checkSessionOptions, standsInFor } from '../src/session-options.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 { closeAfter, closeListenerAfter } from './teardown.ts';
import { concatOf } from '../src/concat.ts'; import { concatOf } from '../src/concat.ts';
import { consts } from '../src/defs/constants.ts'; import { consts } from '../src/defs/constants.ts';
@@ -1049,12 +1049,35 @@ describe('connectTimeout', () => {
assert.ok(settled.err instanceof Error); assert.ok(settled.err instanceof Error);
assert.match( assert.match(
settled.err.message, settled.err.message,
new RegExp(`Timed out completing the TLS handshake with 127\\.0\\.0\\.1:${String(port)} after 150 ms`), 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 from the operator', '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); 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 => { test('retries a connect it timed out on, like any other failed attempt', async t => {
const { accepted, port } = await stalledListener(t); const { accepted, port } = await stalledListener(t);
const controller = new AbortController(); const controller = new AbortController();
@@ -1087,7 +1110,6 @@ describe('connectTimeout', () => {
}); });
test('refuses a connect timeout that would turn itself off', async () => { test('refuses a connect timeout that would turn itself off', async () => {
assert.equal(clientDefaults.connectTimeout, 10_000, 'the number the README documents');
assert.match( assert.match(
checkSessionOptions({ connectTimeout: 0 }).err?.message ?? '', checkSessionOptions({ connectTimeout: 0 }).err?.message ?? '',
/false waits the OS out/, /false waits the OS out/,
+3 -1
View File
@@ -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` `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 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 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 - [ ] **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 `outgoing-requests.ts` runs `misuse()` and the abort check before the wait, precisely so a call