Give up a connect after connectTimeout instead of waiting the OS out
Mirror / push (push) Successful in 4s
Test / lint (pull_request) Successful in 21s
Test / test (18) (pull_request) Successful in 22s
Test / test (20) (pull_request) Successful in 18s
Test / test (22) (pull_request) Successful in 18s
Test / test (24) (pull_request) Successful in 19s
Test / test (26) (pull_request) Successful in 27s

This commit is contained in:
2026-09-20 00:42:09 +02:00
parent d874448273
commit 17447bbe3a
8 changed files with 175 additions and 33 deletions
+4
View File
@@ -214,6 +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.
- **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
@@ -299,6 +301,8 @@ the file.
- Coming up is not proof a link works, so only one that outlasted `maxDelay` resets the backoff.
- `reconnect: { fromStart: true }` puts the first connect and bind through that same loop, and
`client()` then resolves only once it is bound.
- `connectTimeout` is absent by default, bounds the whole connect including the TLS handshake, and
`0` is refused.
- A stream this library cannot frame is a dead link; one PDU it cannot parse is not.
- A deliberate shutdown drains; an unusable link and an abort do not.
- Every segment of a concatenated message is answered as it arrives, so `sendResp()` on one is the
+9
View File
@@ -0,0 +1,9 @@
# Changelog
What changed for someone using `@larvit/smpp`, newest first.
## Unreleased
- `client()` takes `connectTimeout`, which gives up on a connect the SMSC never completes — the TLS
handshake included — and reports it as an ordinary connect failure, so `reconnect` retries it on
its usual backoff. Leave it out and the wait is the operating system's, as before.
+1
View File
@@ -227,6 +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` | — | Give up on a connect the SMSC never completes, the TLS handshake included, and report it as an ordinary connect failure, which `reconnect` then retries. Absent, the wait is the operating system's, around 130 s on Linux. |
| `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. |
+13
View File
@@ -524,6 +524,19 @@ rule and an index of the titles below.
`LinkGate`'s hold is not — it is awaited with no other handle, so a process whose only work is
`client()` would exit unbound.
- **`connectTimeout` is absent by default, bounds the whole connect including the TLS handshake, and
`0` is refused.** Maintainer's call, 2026-09-20, serving goal 6: a connect that never returns is
one `reconnect` cannot retry, because the operating system holds the attempt for around 130 s at
Linux's default `tcp_syn_retries` and nothing above it is counting. Absent is how that wait stays
the operating system's, so a call passing no options is unchanged, and `0` would be a second
spelling for absent — refused at the call, naming the spelling that turns it off. What expires is
reported as the ordinary connect failure, so the loop retries it like any other. It settles on
`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.
- **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
16 or above `maxPduLength` leaves nothing that can say where the next PDU starts, so it tears the
+20
View File
@@ -22,6 +22,7 @@ export type ClientOptions = {
addrNpi?: number;
addrTon?: number;
bindType?: BindType;
connectTimeout?: number;
enquireLinkInterval?: number;
host?: string;
idleTimeout?: number;
@@ -52,7 +53,22 @@ const defaults = {
username: 'user',
} as const;
function armConnectTimeout(
sock: Socket,
connectTimeout: number | undefined,
settle: (result: Result<{ sock: Socket }>) => void,
): NodeJS.Timeout | undefined {
if (connectTimeout === undefined) return undefined;
// The connecting socket is what holds the process, so this wait never has to.
return setTimeout(() => {
sock.destroy();
settle({ err: new Error(`Timed out connecting after ${String(connectTimeout)} ms`) });
}, connectTimeout).unref();
}
function openSocket(options: ClientOptions): Promise<Result<{ sock: Socket }>> {
const connectTimeout = options.connectTimeout;
const host = options.host ?? defaults.host;
const port = options.port ?? defaults.port;
const secure = options.tls !== undefined && options.tls !== false;
@@ -78,6 +94,8 @@ function openSocket(options: ClientOptions): Promise<Result<{ sock: Socket }>> {
}
const settle = (result: Result<{ sock: Socket }>): void => {
if (timer) clearTimeout(timer);
sock.removeListener('error', onError);
signal?.removeEventListener('abort', onAbort);
resolve(result);
@@ -97,6 +115,8 @@ function openSocket(options: ClientOptions): Promise<Result<{ sock: Socket }>> {
sock.once(secure ? 'secureConnect' : 'connect', () => {
settle({ sock });
});
const timer = armConnectTimeout(sock, connectTimeout, settle);
});
}
+28 -8
View File
@@ -144,14 +144,11 @@ export function checkSessionOptions(options: CheckableOptions): VoidResult {
return { err: new Error('fromStart is part of the reconnect policy, spell it reconnect: { fromStart: true }') };
}
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],
]);
const connect = checkConnectTimeout(options.connectTimeout);
if (connect.err) return connect;
const checked = checkLimits(limitsOf(options));
if (checked.err) return checked;
@@ -160,6 +157,28 @@ export function checkSessionOptions(options: CheckableOptions): VoidResult {
return backoff.err ? backoff : checkSmsIdFormat(options.smsIdFormat);
}
function limitsOf(options: CheckableOptions): [string, number, number][] {
return [
['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],
];
}
/** Leaving it out is how the wait stays the OS's, so 0 would be a second spelling for that. */
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`) };
}
function checkLimits(limits: [string, number, number][]): VoidResult {
for (const [name, value, min] of limits) {
if (!Number.isInteger(value) || value < min) {
@@ -238,6 +257,7 @@ function checkSmsIdFormat(smsIdFormat: unknown): VoidResult {
/** What the checker reads, as it arrives: a caller without types can put anything in it. */
export type CheckableOptions = {
connectTimeout?: number | undefined;
/** Not an option: the one spelling is inside reconnect, and this is where the other is refused. */
fromStart?: unknown;
idleTimeout?: number | undefined;
+95 -15
View File
@@ -84,6 +84,21 @@ function within<T>(ms: number, promise: Promise<T>): Promise<T | undefined> {
return Promise.race([promise, delay(ms).then((): undefined => undefined)]);
}
/** A client still retrying holds a socket and a timer nothing else releases. */
function abortAfter(
t: TestContext,
controller: AbortController,
connecting: ReturnType<typeof client>,
): void {
t.after(async () => {
controller.abort();
const { session } = await connecting;
await session?.close({ signal: AbortSignal.abort() });
});
}
function submitPdu(seqNr: number, cmdStatus: ErrorName = 'ESME_ROK'): PduObject {
return {
cmdId: 0x00000004,
@@ -798,21 +813,6 @@ describe('reconnect from the first bind', () => {
return port;
}
/** A client still retrying holds a socket and a timer nothing else releases. */
function abortAfter(
t: TestContext,
controller: AbortController,
connecting: ReturnType<typeof client>,
): void {
t.after(async () => {
controller.abort();
const { session } = await connecting;
await session?.close({ signal: AbortSignal.abort() });
});
}
test('gives up on the first attempt where reconnect alone is asked for', async () => {
const port = await closedPort();
const spy = logSpy();
@@ -1018,6 +1018,86 @@ describe('reconnect from the first bind', () => {
});
});
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); });
closeListenerAfter(t, listener, accepted);
await new Promise<void>(resolve => { listener.listen(0, '127.0.0.1', resolve); });
const address = listener.address();
return { accepted, port: typeof address === 'object' && address !== null ? address.port : 0 };
}
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',
port,
reconnect: false,
tls: true,
}));
assert.ok(settled, 'a handshake nothing answers is what the OS wait would swallow for minutes');
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 => {
const { accepted, port } = await stalledListener(t);
const controller = new AbortController();
const connecting = client({
connectTimeout: 60,
host: '127.0.0.1',
port,
reconnect: { fromStart: true, maxDelay: 40, minDelay: 10 },
signal: controller.signal,
tls: true,
});
abortAfter(t, controller, connecting);
await delay(400);
assert.ok(accepted.length >= 3, `the loop retried what timed out, got ${String(accepted.length)} attempts`);
});
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 });
assert.ok(session);
await delay(150);
const probe = await session.send({ cmdName: 'enquire_link' });
assert.equal(probe.err, undefined);
assert.equal(session.sock.destroyed, false);
});
test('refuses a connect timeout that would turn itself off', async () => {
assert.match(
checkSessionOptions({ connectTimeout: 0 }).err?.message ?? '',
/omit it/,
'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.equal(checkSessionOptions({ connectTimeout: 1000 }).err, undefined);
const refused = await client({ connectTimeout: 0, port: 1 });
assert.ok(refused.err instanceof Error);
assert.equal(refused.session, undefined);
});
});
describe('sends across a reconnect', () => {
/** Answers every message after the first, which is left to hold the send window open. */
function answerAfterTheFirst(smpp: SmppServer, arrived: string[]): Latch {
+5 -10
View File
@@ -179,16 +179,11 @@ the rewrite, for a dependency added later. Maintainer's call, 2026-09-14.
## Worth doing, not blocking
- [ ] **Give up a connect after `connectTimeout` instead of waiting the OS out.** `openSocket()` in
`client.ts` settles only on `connect`/`secureConnect`, a socket `error`, or the caller's
`signal`, and `reconnect-loop.ts` times nothing but its backoff — so a host that drops SYNs
stalls every attempt for the OS TCP timeout, around 130 s on Linux at the default
`tcp_syn_retries`, and `reconnect.fromStart` cannot retry what never returns. The option aborts
the socket and returns the ordinary connect failure; absent, the wait stays exactly 0.5.0's, so
a call passing no options is unchanged (goal 6). Whether a later major makes it a default is
the open half. Found comparing 0.5.0 with the seven `larvitsmpp` forks, 2026-09-20 —
[SwiftHero/larvitsmpp](https://github.com/SwiftHero/larvitsmpp) added a 10 s connect timeout in
2017, and it is the one fork change with no equivalent here.
- [ ] **Decide whether a later major gives `connectTimeout` a default.** The option ships absent, so
a call passing none waits the OS out as 0.5.0 did — the half the option deliberately left open.
A default is a breaking change for anyone relying on that wait, and the minor is the breaking
unit until 1.0.0. Needs a number an operator would recognise, where SwiftHero's fork picked
10 s.
- [ ] **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