Claim an error only where the state failed, and freeze the two state lists

This commit is contained in:
2026-09-04 08:51:13 +02:00
parent 54493c12b2
commit 144567c294
5 changed files with 26 additions and 13 deletions
+8 -7
View File
@@ -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 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` 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 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 `SCHEDULED` and undefined in 3.4; a peer that writes it is read as transient rather than as saying
transient rather than as saying nothing, maintainer's call, 2026-09-03, since the codec refuses a nothing, maintainer's call, 2026-09-03, since the codec refuses a zero-length integer TLV and so an
zero-length integer TLV and so an absent one cannot land there. absent one cannot land there.
- **A transient state goes out as an intermediate delivery notification (0x20), every other state as - **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 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 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 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. 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 Rejected: 0x04 for every state, for the sake of a peer that classifies on the marker — the cost
then reads a transient report as a final one, which is goal 2's wrong answer handed to the peer accepted here is that such a peer stops recognising a transient report as a report at all and hands
instead of to the application. A 0.4.0 client is unaffected either way: its bind declares 0x00, so its application receipt text as an inbound message, where under 0x04 it would have read the state
it is sent no TLVs and refuses the receipt regardless. 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 `<base>-<n>` - **`smsIdFormat` names a notation per place, and normalisation never reaches inside a `<base>-<n>`
id.** An SMSC may answer `submit_sm_resp` in hex and write the receipt's `id:` in decimal, so one id.** An SMSC may answer `submit_sm_resp` in hex and write the receipt's `id:` in decimal, so one
+2 -2
View File
@@ -131,10 +131,10 @@ export function parseReceipt(message: string): Receipt {
type MessageType = 'intermediate' | 'other' | 'receipt' | 'unmarked'; type MessageType = 'intermediate' | 'other' | 'receipt' | 'unmarked';
/** SMPP 3.4 Appendix B lists every other receipt state as final. */ /** 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. */ /** 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.CONVERSATION_ABORT,
consts.ESM_CLASS.DELIVERY_ACKNOWLEDGEMENT, consts.ESM_CLASS.DELIVERY_ACKNOWLEDGEMENT,
consts.ESM_CLASS.USER_ACKNOWLEDGEMENT, consts.ESM_CLASS.USER_ACKNOWLEDGEMENT,
+2 -1
View File
@@ -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. */ /** 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 { function receiptText(sms: Sms, smsId: string, status: MessageState): string {
const delivered = status === 'DELIVERED'; const delivered = status === 'DELIVERED';
const failed = !delivered && !transientStates.includes(status);
return [ return [
`id:${smsId}`, `id:${smsId}`,
@@ -136,7 +137,7 @@ function receiptText(sms: Sms, smsId: string, status: MessageState): string {
`submit date:${smppDate(sms.submitTime)}`, `submit date:${smppDate(sms.submitTime)}`,
`done date:${smppDate(new Date())}`, `done date:${smppDate(new Date())}`,
`stat:${receiptCodes[status]}`, `stat:${receiptCodes[status]}`,
`err:${delivered ? '000' : '001'}`, `err:${failed ? '001' : '000'}`,
'text:', 'text:',
].join(' '); ].join(' ');
} }
+8 -3
View File
@@ -178,11 +178,11 @@ describe('merged delivery reports', () => {
assert.ok(session); assert.ok(session);
const merged = once<MessageDlr>(resolve => { session.on('messageDlr', resolve); }); const merged = once<MessageDlr>(resolve => { session.on('messageDlr', resolve); });
const perSegment: boolean[] = []; const reports: Dlr[] = [];
const markers: (number | undefined)[] = []; const markers: (number | undefined)[] = [];
session.on('dlr', (dlr, pduObj) => { session.on('dlr', (dlr, pduObj) => {
perSegment.push(dlr.intermediate); reports.push(dlr);
markers.push(paramNumber(pduObj.params.esm_class, 0)); 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.smsId, 'en-route');
assert.equal(report.statusMsg, 'DELIVERED'); assert.equal(report.statusMsg, 'DELIVERED');
assert.equal(report.segments.length, 3); assert.equal(report.segments.length, 3);
assert.deepEqual(perSegment, [true, true, true, false, false, false]);
const notification = consts.ESM_CLASS.INTERMEDIATE_DELIVERY; const notification = consts.ESM_CLASS.INTERMEDIATE_DELIVERY;
const receipt = consts.ESM_CLASS.MC_DELIVERY_RECEIPT; const receipt = consts.ESM_CLASS.MC_DELIVERY_RECEIPT;
assert.deepEqual(reports.map(one => one.intermediate), [true, true, true, false, false, false]);
assert.deepEqual( assert.deepEqual(
markers, markers,
[notification, notification, notification, receipt, receipt, receipt], [notification, notification, notification, receipt, receipt, receipt],
'a transient state goes out under the marker the spec gives it', '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 => { test('reports the worst status across the segments', async t => {
+6
View File
@@ -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, and applies it to the other is wrong. A budget type both take would close it. Raised by review,
2026-09-01. 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.** - [ ] **`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 `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 `tls.test.ts` wait forever, so an event that never fires still hangs the run the way an