diff --git a/AGENTS.md b/AGENTS.md index b5904d0..c7b6769 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -298,19 +298,20 @@ Grouped by what each one constrains. second time in `DlrMerger`, so the library cannot answer the application one way and conclude the other. Not every peer marks a transient report 0x20 — an ordinary receipt carrying `stat:ENROUTE` is common — so the state test is what the marker test cannot replace. `message_state` 0 is 5.0's - `SCHEDULED` and undefined in 3.4; a peer that writes it is read as - transient rather than as saying nothing, maintainer's call, 2026-09-03, since the codec refuses a - zero-length integer TLV and so an absent one cannot land there. + `SCHEDULED` and undefined in 3.4; a peer that writes it is read as transient rather than as saying + nothing, maintainer's call, 2026-09-03, since the codec refuses a zero-length integer TLV and so an + absent one cannot land there. - **A transient state goes out as an intermediate delivery notification (0x20), every other state as a delivery receipt (0x04).** Appendix B makes a receipt's `stat` the message's final status, so 0x04 over `ENROUTE` emits the two disagreeing spellings of finality the reading side above has to reconcile, and goal 3 has our own senders write the marker 3.4 defines. `sendDlr()` takes the list from `transientStates` in `dlr.ts`, the same one the reader uses, so the two cannot drift. - Rejected: 0x04 for every state, for the sake of a peer keyed to that marker alone — such a peer - then reads a transient report as a final one, which is goal 2's wrong answer handed to the peer - instead of to the application. A 0.4.0 client is unaffected either way: its bind declares 0x00, so - it is sent no TLVs and refuses the receipt regardless. + Rejected: 0x04 for every state, for the sake of a peer that classifies on the marker — the cost + accepted here is that such a peer stops recognising a transient report as a report at all and hands + its application receipt text as an inbound message, where under 0x04 it would have read the state + from `stat:` and been right. A transient state also carries `err:000`, since a message still on its + way has not failed. - **`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 diff --git a/src/dlr.ts b/src/dlr.ts index 1e855fe..7b6a417 100644 --- a/src/dlr.ts +++ b/src/dlr.ts @@ -131,10 +131,10 @@ export function parseReceipt(message: string): Receipt { type MessageType = 'intermediate' | 'other' | 'receipt' | 'unmarked'; /** SMPP 3.4 Appendix B lists every other receipt state as final. */ -export const transientStates: MessageState[] = ['ENROUTE', 'SCHEDULED']; +export const transientStates: readonly MessageState[] = ['ENROUTE', 'SCHEDULED']; /** Written by the far-end SME, not by the MC reporting on a message we submitted. */ -const smeMessageTypes: number[] = [ +const smeMessageTypes: readonly number[] = [ consts.ESM_CLASS.CONVERSATION_ABORT, consts.ESM_CLASS.DELIVERY_ACKNOWLEDGEMENT, consts.ESM_CLASS.USER_ACKNOWLEDGEMENT, diff --git a/src/sms.ts b/src/sms.ts index 533c23a..4f4d655 100644 --- a/src/sms.ts +++ b/src/sms.ts @@ -128,6 +128,7 @@ async function sendResp( /** The receipt as text, which is all of it a peer below SMPP 3.4 is allowed to be sent. */ function receiptText(sms: Sms, smsId: string, status: MessageState): string { const delivered = status === 'DELIVERED'; + const failed = !delivered && !transientStates.includes(status); return [ `id:${smsId}`, @@ -136,7 +137,7 @@ function receiptText(sms: Sms, smsId: string, status: MessageState): string { `submit date:${smppDate(sms.submitTime)}`, `done date:${smppDate(new Date())}`, `stat:${receiptCodes[status]}`, - `err:${delivered ? '000' : '001'}`, + `err:${failed ? '001' : '000'}`, 'text:', ].join(' '); } diff --git a/test/session-extras.test.ts b/test/session-extras.test.ts index 9499f30..5b1f322 100644 --- a/test/session-extras.test.ts +++ b/test/session-extras.test.ts @@ -178,11 +178,11 @@ describe('merged delivery reports', () => { assert.ok(session); const merged = once(resolve => { session.on('messageDlr', resolve); }); - const perSegment: boolean[] = []; + const reports: Dlr[] = []; const markers: (number | undefined)[] = []; session.on('dlr', (dlr, pduObj) => { - perSegment.push(dlr.intermediate); + reports.push(dlr); markers.push(paramNumber(pduObj.params.esm_class, 0)); }); @@ -208,15 +208,20 @@ describe('merged delivery reports', () => { 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]); const notification = consts.ESM_CLASS.INTERMEDIATE_DELIVERY; const receipt = consts.ESM_CLASS.MC_DELIVERY_RECEIPT; + assert.deepEqual(reports.map(one => one.intermediate), [true, true, true, false, false, false]); assert.deepEqual( markers, [notification, notification, notification, receipt, receipt, receipt], 'a transient state goes out under the marker the spec gives it', ); + assert.deepEqual( + reports.map(one => one.errorCode), + ['000', '000', '000', '000', '000', '000'], + 'a message still on its way has not failed', + ); }); test('reports the worst status across the segments', async t => { diff --git a/todo.md b/todo.md index 7487ce6..8ccd36a 100644 --- a/todo.md +++ b/todo.md @@ -133,6 +133,12 @@ session message is a change to every call site. and applies it to the other is wrong. A budget type both take would close it. Raised by review, 2026-09-01. +- [ ] **`err:` on a receipt for a state that neither delivered nor failed.** `receiptText()` now + writes `err:000` for `DELIVERED` and for the two transient states, and `err:001` for every + other — so `ACCEPTED`, `SKIPPED`, `UNKNOWN` and `DELETED` still announce an error code the SMSC + never had. Which of those are failures is the open half. Raised by review, 2026-09-03; needs a + decision. + - [ ] **`once()` is copied into four test files, and two copies never give up.** `session-extras.test.ts` and `readme.test.ts` reject after 5000 ms; `session.test.ts` and `tls.test.ts` wait forever, so an event that never fires still hangs the run the way an