Give up a connect after connectTimeout instead of waiting the OS out #12
@@ -302,8 +302,8 @@ the file.
|
|||||||
- Coming up is not proof a link works, so only one that outlasted `maxDelay` resets the backoff.
|
- 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
|
- `reconnect: { fromStart: true }` puts the first connect and bind through that same loop, and
|
||||||
`client()` then resolves only once it is bound.
|
`client()` then resolves only once it is bound.
|
||||||
- `connectTimeout` is absent by default, bounds the whole connect including the TLS handshake, and
|
- `connectTimeout` defaults to 10 s, bounds the whole connect including the TLS handshake, and
|
||||||
`0` is refused.
|
`false` is the one way to turn it off.
|
||||||
- A stream this library cannot frame is a dead link; one PDU it cannot parse is not.
|
- 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.
|
- 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
|
- Every segment of a concatenated message is answered as it arrives, so `sendResp()` on one is the
|
||||||
|
|||||||
+4
-3
@@ -2,9 +2,10 @@
|
|||||||
|
|
||||||
## Unreleased
|
## Unreleased
|
||||||
|
|
||||||
- `client()` takes `connectTimeout`, which gives up on a connect attempt the SMSC never completes —
|
- `client()` now bounds each connect attempt at 10 seconds, the TLS handshake included, and reports
|
||||||
the TLS handshake included — and reports it as an ordinary connect failure, so `reconnect` retries
|
one that expires as an ordinary connect failure, so `reconnect` retries it on its usual backoff.
|
||||||
it on its usual backoff. Leave it out and each attempt waits the operating system out, as before.
|
A connect previously waited the operating system out, around 130 s on Linux against a host that
|
||||||
|
drops SYNs. `connectTimeout` retunes the bound, and `connectTimeout: false` restores the old wait.
|
||||||
|
|
||||||
## 0.5.0
|
## 0.5.0
|
||||||
|
|
||||||
|
|||||||
@@ -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` | — | 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. Absent, each attempt waits the operating system out, 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 one attempt, never the `client()` call. `false` waits the operating system out instead, around 130 s on Linux. |
|
||||||
| `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. |
|
||||||
|
|||||||
+18
-12
@@ -524,19 +524,25 @@ 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
|
`LinkGate`'s hold is not — it is awaited with no other handle, so a process whose only work is
|
||||||
`client()` would exit unbound.
|
`client()` would exit unbound.
|
||||||
|
|
||||||
- **`connectTimeout` is absent by default, bounds the whole connect including the TLS handshake, and
|
- **`connectTimeout` defaults to 10 s, 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
|
`false` is the one way to turn it off.** Maintainer's call, 2026-09-20, serving goal 5: a connect
|
||||||
one `reconnect` cannot retry, because the operating system holds the attempt for around 130 s at
|
that never returns is one `reconnect` cannot retry, because the operating system holds the attempt
|
||||||
Linux's default `tcp_syn_retries` and nothing above it is counting. Absent is how that wait stays
|
for around 130 s at Linux's default `tcp_syn_retries` and nothing above it is counting — and no
|
||||||
the operating system's, so a call passing no options is unchanged, and `0` would be a second
|
application chooses that, so it is a default rather than an option that switches on what the caller
|
||||||
spelling for absent — refused at the call, naming the spelling that turns it off. What expires is
|
obviously wanted. Accepted: the far end sees roughly four times the SYNs against a dead host, one
|
||||||
reported as the ordinary connect failure, so the loop retries it like any other. It settles on
|
per ~40 s rather than one per ~160 s, which goal 4 tolerates because the backoff still caps the
|
||||||
|
rate. `false` spells the operating system's wait, as it does for `reconnect`, and `0` is refused
|
||||||
|
naming it, so one spelling reaches each result. What expires is reported as the ordinary connect
|
||||||
|
failure, so the loop retries it like any other, and the message names the peer and whether the TCP
|
||||||
|
connect or the TLS handshake stalled — different faults, different answers. It settles on
|
||||||
`secureConnect` for a TLS socket, so a peer that accepts and then says nothing is bounded the same
|
`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
|
way a black-holed SYN is. Rejected: shipping the option with no default, which left goal 5's "an
|
||||||
once the link is up. Rejected: bounding it with `responseTimeout`, which names the wait for an
|
option does not switch on the thing the caller obviously wanted" unmet, and would have cost a
|
||||||
answer on a link that already exists and would retune both at once. `server()` shares the checker
|
second breaking minor plus a reversal of the `0` spelling to correct later. Rejected:
|
||||||
and ignores the option, as it already ignores `reconnect` — nothing at that end connects out. Open
|
`socket.setTimeout()`, an idle timeout that goes on arming once the link is up. Rejected: bounding
|
||||||
while it has no default: whether a later major gives it one.
|
it with `responseTimeout`, which names the wait for an 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.
|
||||||
|
|
||||||
- **A stream this library cannot frame is a dead link; one PDU it cannot parse is not.**
|
- **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
|
Maintainer's call, 2026-08-31, narrowed 2026-09-05 via the interop plan: a `command_length` below
|
||||||
|
|||||||
+6
-5
@@ -22,7 +22,7 @@ export type ClientOptions = {
|
|||||||
addrNpi?: number;
|
addrNpi?: number;
|
||||||
addrTon?: number;
|
addrTon?: number;
|
||||||
bindType?: BindType;
|
bindType?: BindType;
|
||||||
connectTimeout?: number;
|
connectTimeout?: number | false;
|
||||||
enquireLinkInterval?: number;
|
enquireLinkInterval?: number;
|
||||||
host?: string;
|
host?: string;
|
||||||
idleTimeout?: number;
|
idleTimeout?: number;
|
||||||
@@ -41,8 +41,9 @@ export type ClientOptions = {
|
|||||||
username?: string;
|
username?: string;
|
||||||
};
|
};
|
||||||
|
|
||||||
const defaults = {
|
export const defaults = {
|
||||||
bindType: 'transceiver',
|
bindType: 'transceiver',
|
||||||
|
connectTimeout: 10_000,
|
||||||
enquireLinkInterval: 20_000,
|
enquireLinkInterval: 20_000,
|
||||||
host: 'localhost',
|
host: 'localhost',
|
||||||
/** The idle timeout is what notices a dead link, so it has to outlast one silent probe. */
|
/** The idle timeout is what notices a dead link, so it has to outlast one silent probe. */
|
||||||
@@ -55,11 +56,11 @@ const defaults = {
|
|||||||
|
|
||||||
function armConnectTimeout(
|
function armConnectTimeout(
|
||||||
sock: Socket,
|
sock: Socket,
|
||||||
connectTimeout: number | undefined,
|
connectTimeout: number | false,
|
||||||
peer: string,
|
peer: string,
|
||||||
settle: (result: Result<{ sock: Socket }>) => void,
|
settle: (result: Result<{ sock: Socket }>) => void,
|
||||||
): NodeJS.Timeout | undefined {
|
): NodeJS.Timeout | undefined {
|
||||||
if (connectTimeout === undefined) return undefined;
|
if (connectTimeout === false) return undefined;
|
||||||
|
|
||||||
// A firewall and a stalled handshake need different answers, and only the phase tells them apart.
|
// 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 ${peer}`;
|
||||||
@@ -73,7 +74,7 @@ function armConnectTimeout(
|
|||||||
}
|
}
|
||||||
|
|
||||||
function openSocket(options: ClientOptions): Promise<Result<{ sock: Socket }>> {
|
function openSocket(options: ClientOptions): Promise<Result<{ sock: Socket }>> {
|
||||||
const connectTimeout = options.connectTimeout;
|
const connectTimeout = options.connectTimeout ?? defaults.connectTimeout;
|
||||||
const host = options.host ?? defaults.host;
|
const host = options.host ?? defaults.host;
|
||||||
const port = options.port ?? defaults.port;
|
const port = options.port ?? defaults.port;
|
||||||
const secure = options.tls !== undefined && options.tls !== false;
|
const secure = options.tls !== undefined && options.tls !== false;
|
||||||
|
|||||||
@@ -171,12 +171,12 @@ function limitsOf(options: CheckableOptions): [string, number, number][] {
|
|||||||
const maxTimerDelay = 2_147_483_647;
|
const maxTimerDelay = 2_147_483_647;
|
||||||
|
|
||||||
function checkConnectTimeout(connectTimeout: unknown): VoidResult {
|
function checkConnectTimeout(connectTimeout: unknown): VoidResult {
|
||||||
if (connectTimeout === undefined) return {};
|
if (connectTimeout === undefined || connectTimeout === false) return {};
|
||||||
|
|
||||||
const got = typeof connectTimeout === 'string' ? `"${connectTimeout}"` : namedValue(connectTimeout);
|
const got = typeof connectTimeout === 'string' ? `"${connectTimeout}"` : namedValue(connectTimeout);
|
||||||
|
|
||||||
if (typeof connectTimeout !== 'number' || !Number.isInteger(connectTimeout) || connectTimeout < 1) {
|
if (typeof connectTimeout !== 'number' || !Number.isInteger(connectTimeout) || connectTimeout < 1) {
|
||||||
return { err: new Error(`connectTimeout must be a whole number of milliseconds, 1 or more, got ${got}; omit it or pass undefined to wait the OS out`) };
|
return { err: new Error(`connectTimeout must be a whole number of milliseconds, 1 or more, got ${got}; false waits the OS out instead`) };
|
||||||
}
|
}
|
||||||
|
|
||||||
if (connectTimeout > maxTimerDelay) {
|
if (connectTimeout > maxTimerDelay) {
|
||||||
|
|||||||
@@ -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 } from '../src/client.ts';
|
import { client, defaults as clientDefaults } 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';
|
||||||
@@ -1087,11 +1087,13 @@ 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 ?? '',
|
||||||
/omit it/,
|
/false waits the OS out/,
|
||||||
'off is spelled by leaving it out, so 0 may not stand in for it',
|
'off is spelled false, so 0 may not stand in for it',
|
||||||
);
|
);
|
||||||
|
assert.equal(checkSessionOptions({ connectTimeout: false }).err, undefined);
|
||||||
assert.match(checkSessionOptions({ connectTimeout: -1 }).err?.message ?? '', /connectTimeout/);
|
assert.match(checkSessionOptions({ connectTimeout: -1 }).err?.message ?? '', /connectTimeout/);
|
||||||
assert.match(checkSessionOptions({ connectTimeout: 1.5 }).err?.message ?? '', /whole number/);
|
assert.match(checkSessionOptions({ connectTimeout: 1.5 }).err?.message ?? '', /whole number/);
|
||||||
assert.match(
|
assert.match(
|
||||||
@@ -1110,8 +1112,13 @@ describe('connectTimeout', () => {
|
|||||||
const refused = await client({ connectTimeout: 0, port: 1 });
|
const refused = await client({ connectTimeout: 0, port: 1 });
|
||||||
|
|
||||||
assert.ok(refused.err instanceof Error);
|
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.match(refused.err.message, /false waits/, 'the socket may not be opened before the option is refused');
|
||||||
assert.equal(refused.session, undefined);
|
assert.equal(refused.session, undefined);
|
||||||
|
|
||||||
|
const off = await client({ connectTimeout: false, port: 1 });
|
||||||
|
|
||||||
|
assert.ok(off.err instanceof Error);
|
||||||
|
assert.match(off.err.message, /ECONNREFUSED/, 'false opts out of the bound without breaking the connect');
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|||||||
@@ -179,12 +179,6 @@ the rewrite, for a dependency added later. Maintainer's call, 2026-09-14.
|
|||||||
|
|
||||||
## Worth doing, not blocking
|
## Worth doing, not blocking
|
||||||
|
|
||||||
- [ ] **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.
|
|
||||||
|
|
||||||
- [ ] **Cut the three teardown sentences `test/teardown.ts` already says.** Under AGENTS.md's
|
- [ ] **Cut the three teardown sentences `test/teardown.ts` already says.** Under AGENTS.md's
|
||||||
Conventions, "`test/teardown.ts` covers a session, a server and a listener" restates its two
|
Conventions, "`test/teardown.ts` covers a session, a server and a listener" restates its two
|
||||||
exported names, "Its close aborts rather than drains" restates `closeAfter`'s own doc comment,
|
exported names, "Its close aborts rather than drains" restates `closeAfter`'s own doc comment,
|
||||||
|
|||||||
Reference in New Issue
Block a user