Back off from a link that dies as soon as it comes up
A bind that returns is not proof the link works: an unreadable stream is only found afterwards, so resetting the delay there handed a link that died on arrival a fresh minDelay every cycle — one connect and bind per second forever, which is how an account gets blocked for bind flooding. The loop resets only once a link has outlasted maxDelay. Also covers the unreadable stream that has no loop to retry it.
This commit is contained in:
@@ -296,19 +296,24 @@ exactly 140.
|
|||||||
that loop before tearing down, so every deliberate shutdown emits `close`. Without the split an
|
that loop before tearing down, so every deliberate shutdown emits `close`. Without the split an
|
||||||
application that opens a replacement client on `close` ends up holding two binds on one account.
|
application that opens a replacement client on `close` ends up holding two binds on one account.
|
||||||
|
|
||||||
|
- **Only a link that outlasted `maxDelay` resets the backoff.** Coming up is not proof it works: an
|
||||||
|
unreadable stream is found after the bind returns, so resetting there gave a link that died on
|
||||||
|
arrival a fresh `minDelay` every cycle — one TCP connect and bind per second, forever, which is how
|
||||||
|
an account gets blocked for bind flooding. `ReconnectLoop` records when it brought the owner up and
|
||||||
|
resets only if the link then lasted longer than the longest wait it would ever schedule. A drop
|
||||||
|
after a healthy link still retries at `minDelay`.
|
||||||
|
|
||||||
- **A stream this library cannot read is a dead link, not a dead session.** Maintainer's call,
|
- **A stream this library cannot read is a dead link, not a dead session.** Maintainer's call,
|
||||||
2026-08-31: a framing or codec error tears the link down through `teardown()`, so the reconnect
|
2026-08-31: a framing or codec error tears the link down through `teardown()`, so the reconnect
|
||||||
loop retries it on a fresh socket with a fresh framer — which is what a desynced stream needs, and
|
loop retries it on a fresh socket with a fresh framer — which is what a desynced stream needs, and
|
||||||
the common cause. `sessionError` still carries every failure, so a peer that only ever sends
|
the common cause. `sessionError` still carries every failure, so a peer that only ever sends
|
||||||
garbage is visible in the log rather than silent, and the backoff caps the retries at one per
|
garbage is visible in the log rather than silent, and the backoff grows to one attempt per
|
||||||
`maxDelay`. Ending the session outright was inherited from when reconnect was opt-in, where the
|
`maxDelay`.
|
||||||
distinction could not arise.
|
|
||||||
|
|
||||||
- **`disconnected` counts failed links, not outages.** A retry that opens a socket and then loses its
|
- **`disconnected` counts failed links, not outages.** A retry that opens a socket and then loses it
|
||||||
bind re-enters `attach()` and so emits again, which makes it deliberately not one-to-one with
|
clears `closed` through `attach()`, so the next `teardown()` emits again: each emission is a link
|
||||||
`reconnected`: each emission is a link that went down, and suppressing the later ones would leave a
|
that went down, which makes the event deliberately not one-to-one with `reconnected`. Suppressing
|
||||||
failed rebind with nothing but a log line. The README says so, because the pairing is what a reader
|
the later ones would leave a failed rebind with nothing but a log line.
|
||||||
would otherwise assume.
|
|
||||||
|
|
||||||
- **`session.sock` is a getter over `PduTransport`, and stays public.** Maintainer's call, 2026-08-31:
|
- **`session.sock` is a getter over `PduTransport`, and stays public.** Maintainer's call, 2026-08-31:
|
||||||
`session.ts` had reached its line cap, so the socket-to-PDU seam todo.md named was opened —
|
`session.ts` had reached its line cap, so the socket-to-PDU seam todo.md named was opened —
|
||||||
|
|||||||
@@ -104,7 +104,7 @@ Every one is optional.
|
|||||||
| `shutdownTimeout` | `5000` | How long `close()` and `unbind()` wait for the requests this end already sent; `0` waits forever. |
|
| `shutdownTimeout` | `5000` | How long `close()` and `unbind()` wait for the requests this end already sent; `0` waits forever. |
|
||||||
| `maxOutstanding` | `10` | Requests allowed on the wire at once; further sends queue. |
|
| `maxOutstanding` | `10` | Requests allowed on the wire at once; further sends queue. |
|
||||||
| `smsIdFormat` | — | The notation the SMSC writes message ids in, per place it writes them: `{ receipt: 'decimal', submitResp: 'hex' }`. Only needed where the two disagree. |
|
| `smsIdFormat` | — | The notation the SMSC writes message ids in, per place it writes them: `{ receipt: 'decimal', submitResp: 'hex' }`. Only needed where the two disagree. |
|
||||||
| `reconnect` | on | Re-binds after a drop or an idle timeout, backing off from `minDelay` 1 s to `maxDelay` 30 s. `{ minDelay, maxDelay }` retunes it; `false` turns it off, so a drop ends the session. |
|
| `reconnect` | on | Re-binds after a drop, an idle timeout, or a stream the library cannot read, backing off from `minDelay` 1 s to `maxDelay` 30 s. `{ minDelay, maxDelay }` retunes it; `false` turns it off, so a drop ends the session. |
|
||||||
| `log` | silent | Any object with `debug`, `error`, `info`, `verbose` and `warn` methods — see [Logging](#logging). |
|
| `log` | silent | Any object with `debug`, `error`, `info`, `verbose` and `warn` methods — see [Logging](#logging). |
|
||||||
| `signal` | — | An `AbortSignal` that cancels connecting and tears the session down. |
|
| `signal` | — | An `AbortSignal` that cancels connecting and tears the session down. |
|
||||||
|
|
||||||
|
|||||||
@@ -14,7 +14,7 @@ export type PduTransportOptions = {
|
|||||||
/** A complete PDU, before it is parsed. */
|
/** A complete PDU, before it is parsed. */
|
||||||
onFramed: (pdu: Buffer) => void;
|
onFramed: (pdu: Buffer) => void;
|
||||||
onPdu: (pduObj: PduObject) => void;
|
onPdu: (pduObj: PduObject) => void;
|
||||||
/** Nothing further can be read off this stream; the socket has to go. */
|
/** Nothing further can be read off this stream, whatever the socket does next. */
|
||||||
onUnreadable: (err: Error) => void;
|
onUnreadable: (err: Error) => void;
|
||||||
};
|
};
|
||||||
|
|
||||||
|
|||||||
+12
-1
@@ -7,19 +7,23 @@ export type ReconnectLoopOptions = {
|
|||||||
log: SmppLog;
|
log: SmppLog;
|
||||||
maxDelay: number;
|
maxDelay: number;
|
||||||
minDelay: number;
|
minDelay: number;
|
||||||
|
now?: (() => number) | undefined;
|
||||||
/** Brings the owner back up on a freshly opened socket. An err means try again. */
|
/** Brings the owner back up on a freshly opened socket. An err means try again. */
|
||||||
onConnected: (sock: Socket) => Promise<VoidResult>;
|
onConnected: (sock: Socket) => Promise<VoidResult>;
|
||||||
};
|
};
|
||||||
|
|
||||||
/** Reopens a dropped connection, backing off between attempts until it is told to stop. */
|
/** Reopens a dropped connection, backing off between attempts until it is told to stop. */
|
||||||
export class ReconnectLoop {
|
export class ReconnectLoop {
|
||||||
|
private readonly now: () => number;
|
||||||
private readonly options: ReconnectLoopOptions;
|
private readonly options: ReconnectLoopOptions;
|
||||||
private attempting = false;
|
private attempting = false;
|
||||||
private delay: number;
|
private delay: number;
|
||||||
private halted = false;
|
private halted = false;
|
||||||
private timer: NodeJS.Timeout | undefined;
|
private timer: NodeJS.Timeout | undefined;
|
||||||
|
private upAt: number | undefined;
|
||||||
|
|
||||||
constructor(options: ReconnectLoopOptions) {
|
constructor(options: ReconnectLoopOptions) {
|
||||||
|
this.now = options.now ?? Date.now;
|
||||||
this.options = options;
|
this.options = options;
|
||||||
this.delay = options.minDelay;
|
this.delay = options.minDelay;
|
||||||
}
|
}
|
||||||
@@ -32,6 +36,13 @@ export class ReconnectLoop {
|
|||||||
schedule(): void {
|
schedule(): void {
|
||||||
if (this.timer || this.attempting || this.isStopped()) return;
|
if (this.timer || this.attempting || this.isStopped()) return;
|
||||||
|
|
||||||
|
// Coming up is not proof: a stream we cannot read is only found once the link is bound.
|
||||||
|
if (this.upAt !== undefined && this.now() - this.upAt >= this.options.maxDelay) {
|
||||||
|
this.delay = this.options.minDelay;
|
||||||
|
}
|
||||||
|
|
||||||
|
this.upAt = undefined;
|
||||||
|
|
||||||
const delay = this.delay;
|
const delay = this.delay;
|
||||||
|
|
||||||
this.options.log.info('reconnect - retrying after a drop', { delay });
|
this.options.log.info('reconnect - retrying after a drop', { delay });
|
||||||
@@ -98,7 +109,7 @@ export class ReconnectLoop {
|
|||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
this.delay = this.options.minDelay;
|
this.upAt = this.now();
|
||||||
|
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -344,7 +344,7 @@ describe('reconnect', () => {
|
|||||||
|
|
||||||
test('reports a drop it will retry as disconnected, keeping close for the end', async t => {
|
test('reports a drop it will retry as disconnected, keeping close for the end', async t => {
|
||||||
const smpp = await startServer(t);
|
const smpp = await startServer(t);
|
||||||
const { session } = await connect(t, smpp);
|
const { session } = await connect(t, smpp, { reconnect: { maxDelay: 100, minDelay: 20 } });
|
||||||
|
|
||||||
assert.ok(session);
|
assert.ok(session);
|
||||||
|
|
||||||
@@ -391,7 +391,7 @@ describe('reconnect', () => {
|
|||||||
|
|
||||||
test('re-binds after a stream it cannot read, rather than ending the session', async t => {
|
test('re-binds after a stream it cannot read, rather than ending the session', async t => {
|
||||||
const smpp = await startServer(t);
|
const smpp = await startServer(t);
|
||||||
const { session } = await connect(t, smpp);
|
const { session } = await connect(t, smpp, { reconnect: { maxDelay: 100, minDelay: 20 } });
|
||||||
|
|
||||||
assert.ok(session);
|
assert.ok(session);
|
||||||
|
|
||||||
@@ -448,7 +448,8 @@ describe('reconnect', () => {
|
|||||||
|
|
||||||
const closed = once<true>(resolve => { session.on('close', () => { resolve(true); }); });
|
const closed = once<true>(resolve => { session.on('close', () => { resolve(true); }); });
|
||||||
|
|
||||||
await peerOf(smpp).close();
|
// A cmd_length below the 16-octet header: unreadable, and nothing left to retry it.
|
||||||
|
peerOf(smpp).sock.write(Buffer.from([0, 0, 0, 4, 0, 0, 0, 4, 0, 0, 0, 0, 0, 0, 0, 1]));
|
||||||
await closed;
|
await closed;
|
||||||
|
|
||||||
assert.equal(disconnects, 0);
|
assert.equal(disconnects, 0);
|
||||||
|
|||||||
@@ -4,6 +4,7 @@ import test, { describe } from 'node:test';
|
|||||||
import type { Dlr } from '../src/dlr.ts';
|
import type { Dlr } from '../src/dlr.ts';
|
||||||
import type { PduObject, PduObjectInput } from '../src/pdu.ts';
|
import type { PduObject, PduObjectInput } from '../src/pdu.ts';
|
||||||
import type { Sms } from '../src/sms.ts';
|
import type { Sms } from '../src/sms.ts';
|
||||||
|
import type { SmppLog } from '../src/log.ts';
|
||||||
import type { SmppServer } from '../src/server.ts';
|
import type { SmppServer } from '../src/server.ts';
|
||||||
import type { TestContext } from 'node:test';
|
import type { TestContext } from 'node:test';
|
||||||
import type { VoidResult } from '../src/result.ts';
|
import type { VoidResult } from '../src/result.ts';
|
||||||
@@ -1372,6 +1373,52 @@ describe('application hooks that throw or reject', () => {
|
|||||||
assert.ok(retried, 'a throwing connect should be retried, not left for the process to die on');
|
assert.ok(retried, 'a throwing connect should be retried, not left for the process to die on');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test('keeps backing off when every link dies as soon as it comes up', async t => {
|
||||||
|
// A stream we cannot read is found after the bind, so a bind alone must not prove the link.
|
||||||
|
const clock = { now: 0 };
|
||||||
|
const delays: number[] = [];
|
||||||
|
const noop = (): void => undefined;
|
||||||
|
const log: SmppLog = {
|
||||||
|
debug: noop,
|
||||||
|
error: noop,
|
||||||
|
info: (msg, metadata) => {
|
||||||
|
if (msg === 'reconnect - retrying after a drop') delays.push(Number(metadata?.delay));
|
||||||
|
},
|
||||||
|
verbose: noop,
|
||||||
|
warn: noop,
|
||||||
|
};
|
||||||
|
let up = 0;
|
||||||
|
const loop = new ReconnectLoop({
|
||||||
|
connect: () => Promise.resolve({ sock: new net.Socket() }),
|
||||||
|
log,
|
||||||
|
maxDelay: 80,
|
||||||
|
minDelay: 10,
|
||||||
|
now: () => clock.now,
|
||||||
|
onConnected: () => {
|
||||||
|
up++;
|
||||||
|
|
||||||
|
return Promise.resolve({});
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
t.after(() => { loop.stop(); });
|
||||||
|
|
||||||
|
for (let died = 0; died < 4; died++) {
|
||||||
|
loop.schedule();
|
||||||
|
await waitFor(() => up === died + 1);
|
||||||
|
await delay(5);
|
||||||
|
}
|
||||||
|
|
||||||
|
assert.deepEqual(delays, [10, 20, 40, 80]);
|
||||||
|
|
||||||
|
// A link that outlasted the longest wait earned a fresh start.
|
||||||
|
clock.now += 80;
|
||||||
|
loop.schedule();
|
||||||
|
await waitFor(() => delays.length === 5);
|
||||||
|
|
||||||
|
assert.deepEqual(delays, [10, 20, 40, 80, 10]);
|
||||||
|
});
|
||||||
|
|
||||||
test('starts only one reconnect attempt at a time', async t => {
|
test('starts only one reconnect attempt at a time', async t => {
|
||||||
let attempts = 0;
|
let attempts = 0;
|
||||||
let finish: (() => void) | undefined;
|
let finish: (() => void) | undefined;
|
||||||
|
|||||||
Reference in New Issue
Block a user