From f0327044b161b636bdc42ffbd188cfdc3da3ec26 Mon Sep 17 00:00:00 2001 From: Lilleman auf Larv Date: Tue, 1 Sep 2026 18:13:35 +0200 Subject: [PATCH] Cover the receipt refusal and the sweep that wakes a drain --- AGENTS.md | 7 ++-- README.md | 3 +- src/held-messages.ts | 7 ++-- src/sms.ts | 2 +- test/session-extras.test.ts | 70 +++++++++++++++++++++++++------------ 5 files changed, 58 insertions(+), 31 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 816a46f..d791e9d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -407,10 +407,9 @@ Grouped by what each one constrains. up answering the same question two different ways at admit and at release. For the same reason the retry in `pastDrain()` asks `gate.isUp()` rather than `linkDown()`, which also reads the socket — a condition that loops on something the gate does not gate on spins against a gate that admits it - straight back. - `LinkGate.returning` is a copy of `retrying()` taken at teardown, and stays true only because - nothing stops the reconnect loop without `emitClose()` following it: `drain()` and `end()` are the - only callers of `stop()`. A third caller has to shut the gate itself. + straight back. `LinkGate.returning` is a copy of `retrying()` taken at teardown, and stays true + only because nothing stops the reconnect loop without `emitClose()` following it: `drain()` and + `end()` are the only callers of `stop()`. A third caller has to shut the gate itself. ### Internals and tests diff --git a/README.md b/README.md index b036159..51acdc6 100644 --- a/README.md +++ b/README.md @@ -352,8 +352,7 @@ A send issued while the link is down waits for the reconnect instead of failing, the new link is bound — up to `responseTimeout`, after which it gives up having sent nothing. A request already on the wire is the other case: the SMSC may have taken it and lost only the response, so it fails, and `sendSms()` and `sms.sendDlr()` count it in `unanswered`, whether the link dropped -under it, the peer -never answered in time, or you aborted it after it went out. Neither applies with `reconnect: false`, +under it, the peer never answered in time, or you aborted it after it went out. Neither applies with `reconnect: false`, where a drop ends the session and every send after it is refused. `responseTimeout` bounds the wait for a link and the wait for an answer separately, and a send also diff --git a/src/held-messages.ts b/src/held-messages.ts index 57ddfde..0a2a665 100644 --- a/src/held-messages.ts +++ b/src/held-messages.ts @@ -49,7 +49,7 @@ export class HeldMessages { this.sweep(); if (this.held.get(key)) { - this.log.warn('heldMessages - replacing a message on a re-used sequence number', { seqNr: key }); + this.log.warn('heldMessages - replacing a message on a re-used sequence number', { seqNr: Number(key) }); } else if (this.held.full) { this.dropOldest(); } @@ -92,7 +92,10 @@ export class HeldMessages { const [seqNr] = oldest; - this.log.warn('heldMessages - buffer full, dropping the oldest message', { max: this.max, seqNr }); + this.log.warn('heldMessages - buffer full, dropping the oldest message', { + max: this.max, + seqNr: Number(seqNr), + }); } /** Drops every message past its deadline. Runs before each hold and on its own timer. */ diff --git a/src/sms.ts b/src/sms.ts index 5b741b4..a93cf6f 100644 --- a/src/sms.ts +++ b/src/sms.ts @@ -9,7 +9,7 @@ import { receiptCodes } from './dlr.ts'; import { smppDate } from './message.ts'; import { uuidv7 } from './uuid.ts'; -/** Both fields hold what the peer took, so a partial failure names what is already receipted. */ +/** `pduObjs` holds what the peer took, so a partial failure names what is already receipted. */ export type SendDlrResult = { err?: Error; pduObjs: PduObject[]; diff --git a/test/session-extras.test.ts b/test/session-extras.test.ts index f011bbc..91c41be 100644 --- a/test/session-extras.test.ts +++ b/test/session-extras.test.ts @@ -73,6 +73,19 @@ function delay(ms: number): Promise { return new Promise(resolve => { setTimeout(resolve, ms); }); } +function submitPdu(seqNr: number, cmdStatus: ErrorName = 'ESME_ROK'): PduObject { + return { + cmdId: 0x00000004, + cmdLength: 0, + cmdName: 'submit_sm', + cmdStatus, + cmdStatusId: 0, + params: { destination_addr: '46709771337', short_message: 'held', source_addr: '46701113311' }, + seqNr, + tlvs: {}, + }; +} + /** A cmd_length below the 16-octet header: a stream no framing can recover from. */ const unreadablePdu = Buffer.from([0, 0, 0, 4, 0, 0, 0, 4, 0, 0, 0, 0, 0, 0, 0, 1]); @@ -862,21 +875,8 @@ describe('LinkGate', () => { // Goal 4: an application that answers nothing must not grow this for the life of the link. describe('held message bounds', () => { - function heldPdu(seqNr: number): PduObject { - return { - cmdId: 0x00000004, - cmdLength: 0, - cmdName: 'submit_sm', - cmdStatus: 'ESME_ROK', - cmdStatusId: 0, - params: { destination_addr: '46709771337', short_message: 'held', source_addr: '46701113311' }, - seqNr, - tlvs: {}, - }; - } - function message(seqNr: number): PduObject[] { - return [heldPdu(seqNr)]; + return [submitPdu(seqNr)]; } test('drops the message held longest rather than holding every one', () => { @@ -885,6 +885,11 @@ describe('held message bounds', () => { held.hold(oldest); held.hold(message(2)); + held.hold(message(2)); + + assert.equal(held.size, 2, 'a re-used sequence number replaces rather than evicting'); + assert.equal(held.has(oldest), true); + held.hold(message(3)); assert.equal(held.size, 2); @@ -908,9 +913,25 @@ describe('held message bounds', () => { held.clear(); }); - // A receipt cannot be resent wholesale without duplicating the segments that landed, so sendDlr() - // names what the peer took and what it may have, the way sendSms() does. - test('a partial receipt names the segments the peer took and the ones it may have', async t => { + // Without this the drain sits out its whole budget before returning what a sweep already settled. + test('wakes a waiting drain when the last message expires', async () => { + let now = 0; + const held = new HeldMessages({ log: silentLog, max: 10, now: () => now, timeout: 60 }); + + held.hold(message(1)); + + const waiting = held.idle(1000, undefined); + + now = 61; + held.sweep(); + + assert.equal(await waiting, 0); + }); +}); + +describe('sendDlr()', () => { + // A receipt cannot be resent wholesale without duplicating the segments that landed. + test('names the segments the peer took, refused, and may have taken', async t => { const session = new Session({ sock: new net.Socket() }); closeAfter(t, session); @@ -919,7 +940,7 @@ describe('held message bounds', () => { const sms = createSms({ from: '46701113311', message: 'three segments', - pduObjs: [heldPdu(1), heldPdu(2), heldPdu(3)], + pduObjs: [submitPdu(1), submitPdu(2), submitPdu(3)], session, to: '46709771337', }, { @@ -927,15 +948,20 @@ describe('held message bounds', () => { send: () => { call++; - return Promise.resolve(call === 2 - ? { err: new UnansweredError(new Error('nothing came back')) } - : { pduObj: heldPdu(call) }); + if (call === 1) return Promise.resolve({ pduObj: submitPdu(1, 'ESME_RX_T_APPN') }); + + if (call === 2) { + return Promise.resolve({ err: new UnansweredError(new Error('nothing came back')) }); + } + + return Promise.resolve({ pduObj: submitPdu(3) }); }, }); const report = await sms.sendDlr('DELIVERED'); assert.ok(report.err instanceof Error); - assert.equal(report.pduObjs.length, 2); + assert.match(report.err.message, /deliver_sm refused by the peer/); + assert.equal(report.pduObjs.length, 1); assert.equal(report.unanswered, 1); }); });