From 42e6e97447702f25ecde22d84d363f54453a1a98 Mon Sep 17 00:00:00 2001 From: Lilleman auf Larv Date: Thu, 3 Sep 2026 19:28:08 +0200 Subject: [PATCH] Read a receipt reporting a transient state as non-final too, so the merge waits for the real one --- AGENTS.md | 10 ++++++++++ README.md | 4 ++-- src/dlr.ts | 9 +++++++-- test/dlr.test.ts | 29 ++++++++++++++++----------- test/session-extras.test.ts | 39 +++++++++++++++++++++++++++++++++++++ todo.md | 2 +- 6 files changed, 77 insertions(+), 16 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 14861b4..87a67db 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -289,6 +289,16 @@ Grouped by what each one constrains. 0x80-0xFF for MC-vendor-specific values, so an unnameable one keeps its raw `statusId` and leaves `statusMsg` to the body. +- **A report is final unless its `esm_class` or its state says otherwise, and only `ENROUTE` and + `SCHEDULED` say otherwise.** SMPP 3.4 Appendix B lists every other receipt state as final, + `UNKNOWN` and `ACCEPTED` included, so a peer writing `ACCEPTD` for a carrier-accepted step is taken + at its word. Rejected: reading `UNKNOWN` as non-final, which leaves a peer whose receipt body this + library cannot read with no `messageDlr` at all — goal 2 wants that reported as undetermined, not + withheld. Both spellings resolve into `Dlr.intermediate` at the boundary rather than being read a + second time in `DlrMerger`, so the library cannot answer the application one way and conclude the + other; this library's own `sendDlr('ENROUTE')` goes out as an ordinary receipt, which is where the + two would first disagree. + - **`smsIdFormat` names a notation per place, and normalisation never reaches inside a `-` id.** An SMSC may answer `submit_sm_resp` in hex and write the receipt's `id:` in decimal, so one transform over both sides cannot make them equal. `submitResp` covers the `receipted_message_id` diff --git a/README.md b/README.md index cde55a0..1bf5340 100644 --- a/README.md +++ b/README.md @@ -318,8 +318,8 @@ TypeScript users can import `SmppLog` to have the compiler check one. | Event | Fires when | | --- | --- | | `sms` | An SMS arrives, reassembled if it was multipart. Carries `sendResp()`, `sendDlr()` and its `smsId`. | -| `dlr` | A delivery report arrives, one per segment. `intermediate` is true where the SMSC marked the report non-final. `smsId` is undefined when the peer marked a receipt whose body carries no readable id. `statusMsg` names `statusId` unless the peer sent a `message_state` this library cannot name — then `statusId` is that raw value and `statusMsg` is whatever the body said, or `UNKNOWN`. | -| `messageDlr` | Every segment of a multipart message sent with `dlr: true` has been reported on, carrying the worst status of the segments. An intermediate report never counts towards it. Merging needs the SMSC to number its segment ids `-`, which is this library's own server's convention — an SMSC that hands out unrelated ids per segment never fires it. A base is merged once: a later message the SMSC gives the same ids is reported on through `dlr` alone, and an earlier one still collecting loses its merged report as well. | +| `dlr` | A delivery report arrives, one per segment. `intermediate` is true where the report is not final: the SMSC either marked it an intermediate notification, or reported `ENROUTE` or `SCHEDULED`. `smsId` is undefined when the peer marked a receipt whose body carries no readable id. `statusMsg` names `statusId` unless the peer sent a `message_state` this library cannot name — then `statusId` is that raw value and `statusMsg` is whatever the body said, or `UNKNOWN`. | +| `messageDlr` | Every segment of a multipart message sent with `dlr: true` has been reported on, carrying the worst status of the segments. A report carrying `intermediate` never counts towards it. Merging needs the SMSC to number its segment ids `-`, which is this library's own server's convention — an SMSC that hands out unrelated ids per segment never fires it. A base is merged once: a later message the SMSC gives the same ids is reported on through `dlr` alone, and an earlier one still collecting loses its merged report as well. | | `close` | The session is over, because nothing will bring the link back. Fires once, whether you closed it or the link failed for good. | | `disconnected` | The link dropped and the reconnect loop will retry it. Do not open a replacement client here — the session you hold comes back on its own, and `reconnected` says when. Fires again for each attempt that reconnects and then fails, so it is not one-to-one with `reconnected`. | | `reconnected` | The client re-bound after a drop. | diff --git a/src/dlr.ts b/src/dlr.ts index acd680f..ac92eac 100644 --- a/src/dlr.ts +++ b/src/dlr.ts @@ -130,6 +130,9 @@ export function parseReceipt(message: string): Receipt { type MessageType = 'intermediate' | 'other' | 'receipt' | 'unmarked'; +/** SMPP 3.4 Appendix B lists every other receipt state as final. */ +const transientStates: MessageState[] = ['ENROUTE', 'SCHEDULED']; + /** Written by the far-end SME, not by the MC reporting on a message we submitted. */ const smeMessageTypes: number[] = [ consts.ESM_CLASS.CONVERSATION_ABORT, @@ -212,13 +215,15 @@ export function dlrFromPdu(pduObj: PduObject, format: SmsIdFormat = {}): Dlr | u if (type === 'unmarked' && (smsId === undefined || statusMsg === undefined)) return undefined; + const state = statusMsg ?? 'UNKNOWN'; + return { doneDate: receiptDate(receipt?.doneDate), errorCode: receipt?.err, - intermediate: type === 'intermediate', + intermediate: type === 'intermediate' || transientStates.includes(state), receipt, smsId, statusId, - statusMsg: statusMsg ?? 'UNKNOWN', + statusMsg: state, }; } diff --git a/test/dlr.test.ts b/test/dlr.test.ts index 9c0d03d..80b6260 100644 --- a/test/dlr.test.ts +++ b/test/dlr.test.ts @@ -98,22 +98,23 @@ describe('dlrFromPdu()', () => { }); test('maps every spec status code back to its message state and id', () => { - for (const [code, expected, statusId] of [ - ['DELIVRD', 'DELIVERED', 2], - ['UNDELIV', 'UNDELIVERABLE', 5], - ['EXPIRED', 'EXPIRED', 3], - ['DELETED', 'DELETED', 4], - ['ACCEPTD', 'ACCEPTED', 6], - ['REJECTD', 'REJECTED', 8], - ['ENROUTE', 'ENROUTE', 1], - ['UNKNOWN', 'UNKNOWN', 7], - ['delivrd', 'DELIVERED', 2], + for (const [code, expected, statusId, intermediate] of [ + ['DELIVRD', 'DELIVERED', 2, false], + ['UNDELIV', 'UNDELIVERABLE', 5, false], + ['EXPIRED', 'EXPIRED', 3, false], + ['DELETED', 'DELETED', 4, false], + ['ACCEPTD', 'ACCEPTED', 6, false], + ['REJECTD', 'REJECTED', 8, false], + ['ENROUTE', 'ENROUTE', 1, true], + ['UNKNOWN', 'UNKNOWN', 7, false], + ['delivrd', 'DELIVERED', 2, false], ] as const) { const dlr = dlrFromPdu(deliverSm(`id:x stat:${code} err:0`)); assert.ok(dlr); assert.equal(dlr.statusMsg, expected); assert.equal(dlr.statusId, statusId); + assert.equal(dlr.intermediate, intermediate); } }); @@ -207,7 +208,7 @@ describe('dlrFromPdu()', () => { } }); - test('reads an intermediate delivery notification as a report, marked as one', () => { + test('reads a report the peer marked non-final, by either spelling', () => { const enroute = 'id:0195f0c7 sub:001 dlvrd:000 submit date:2508251430 done date:2508251431 stat:ENROUTE err:000 text:'; const dlr = dlrFromPdu(deliverSm(enroute, undefined, consts.ESM_CLASS.INTERMEDIATE_DELIVERY)); @@ -220,6 +221,12 @@ describe('dlrFromPdu()', () => { assert.ok(unreadable, 'the marker makes it a report whatever the body parses to'); assert.equal(unreadable.intermediate, true); + + const scheduled = dlrFromPdu(deliverSm('id:0195f0c7', { message_state: { tagValue: 0 } })); + + assert.ok(scheduled); + assert.equal(scheduled.statusMsg, 'SCHEDULED'); + assert.equal(scheduled.intermediate, true, 'an ordinary receipt reporting a transient state is not final either'); }); test('exposes the raw receipt alongside the resolved fields', () => { diff --git a/test/session-extras.test.ts b/test/session-extras.test.ts index 67322fe..b1bfb65 100644 --- a/test/session-extras.test.ts +++ b/test/session-extras.test.ts @@ -168,6 +168,45 @@ describe('merged delivery reports', () => { assert.deepEqual(perSegment, ['merge-me-1', 'merge-me-2', 'merge-me-3']); }); + test('reports once, on the final receipts, when the peer reports en route first', async t => { + const smpp = await startServer(t); + const incoming = once(resolve => { + smpp.on('session', session => session.on('sms', resolve)); + }); + const { session } = await connect(t, smpp); + + assert.ok(session); + + const merged = once(resolve => { session.on('messageDlr', resolve); }); + const perSegment: boolean[] = []; + + session.on('dlr', dlr => perSegment.push(dlr.intermediate)); + + const [sms] = await Promise.all([ + incoming.then(async received => { + await received.sendResp({ smsId: 'en-route' }); + + return received; + }), + session.sendSms({ + dlr: true, + from: '46701113311', + message: 'x'.repeat(400), + to: '46709771337', + }), + ]); + + await sms.sendDlr('ENROUTE'); + await sms.sendDlr('DELIVERED'); + + const report = await merged; + + assert.equal(report.smsId, 'en-route'); + assert.equal(report.statusMsg, 'DELIVERED'); + assert.equal(report.segments.length, 3); + assert.deepEqual(perSegment, [true, true, true, false, false, false]); + }); + test('reports the worst status across the segments', async t => { const smpp = await startServer(t); const incoming = once(resolve => { diff --git a/todo.md b/todo.md index f36977f..9e64375 100644 --- a/todo.md +++ b/todo.md @@ -64,7 +64,7 @@ Rules the API follows: | The hold released exactly when the peer was answered: a refused `sendResp()` keeps it, a listener that rejected drops it | `test/session-extras.test.ts` | | Every runnable README example | `test/readme.test.ts` | | Receipt-versus-message classification by `esm_class` | `test/dlr.test.ts`, `test/session.test.ts` | -| An intermediate delivery notification read as a report marked `intermediate`, and never counted into a merge | `test/dlr.test.ts`, `test/session.test.ts`, `test/session-extras.test.ts` | +| An intermediate delivery notification read as a report marked `intermediate`, as is a receipt reporting `ENROUTE` or `SCHEDULED`, and never counted into a merge | `test/dlr.test.ts`, `test/session.test.ts`, `test/session-extras.test.ts` | | A listener that throws, or rejects, reaching `sessionError`/`serverError` rather than the process | `test/session.test.ts`, `test/error-from.test.ts` | | Cross-checked against node-smpp both ways and over a live session | `test/interop.test.ts` | | CI on Node 18/20/22/24, Renovate, tag-triggered publish | `.github/workflows/` |