Read an intermediate delivery notification as a report, and keep it out of the merge

This commit is contained in:
2026-09-03 17:30:09 +02:00
parent 94184a9c63
commit 50679ec2fe
8 changed files with 93 additions and 31 deletions
+16 -10
View File
@@ -18,16 +18,19 @@ one wins. They do not override the hard rules below.
1. **Correct on the wire.** SMPP 3.4 as SMSCs actually run it. Every other goal yields to this one;
the defect table below is what the alternative costs.
2. **Never give the application a wrong answer about what happened.** An outcome we cannot determine
is reported as undetermined rather than guessed; a request the peer may already have taken is
never re-sent on the library's own initiative; work the peer has no reason to send again is not
dropped.
is reported as undetermined rather than guessed; a report the peer marked as not final settles
nothing, so nothing the library concludes may rest on one; a request the peer may already have
taken is never re-sent on the library's own initiative; work the peer has no reason to send again
is not dropped.
3. **Strict in what we send, generous in what we read.** The library's own senders follow 3.4, and
the codec parses whatever arrives. Where the letter of the spec would discard traffic a real SMSC
sends, keep the traffic.
4. **A peer an operator never has to complain about.** No bind flooding, nothing a bind direction
forbids, no optional parameters to a peer that declared none, nothing held without a bound.
5. **The session layer is in here, and its defaults are what most applications should run.**
Keepalive, reconnect, the send window, reassembly and receipt correlation. An option retunes a
Keepalive, reconnect, the send window, reassembly and receipt correlation. What the network says
about a message the application sent reaches it as a report rather than as an inbound message, and
says whether it is final, so nothing has to read the PDU to tell those apart. An option retunes a
default or opts out of it; an option does not switch on the thing the caller obviously wanted.
6. **A small, stable public surface over reshapeable internals.** Only what `src/index.ts` exports is
published. A new option has to beat "the application can do this itself", and has to keep a
@@ -273,12 +276,15 @@ Grouped by what each one constrains.
and sent no optional parameters, which is how the spec reads an absent `sc_interface_version`.
- **`esm_class` decides what a `deliver_sm` is, and the body is read only when it names nothing.**
`MC_DELIVERY_RECEIPT` (0x04) makes it a receipt whatever the body parses to, so a receipt in a
format `dlrFromPdu()` cannot read reaches `dlr` with `smsId` undefined instead of arriving as an
inbound SMS. Any other named type is not a receipt and its body is not scraped; a message type of 0
or one of the ten reserved keeps the scrape, and a non-empty `receipted_message_id` TLV marks a
receipt on the same footing. A receipt this library recognises never reaches the reassembler, so an
SMSC that splits one across segments gets a `dlr` per segment rather than one merged report. The
The two types the MC writes about a message we submitted — `MC_DELIVERY_RECEIPT` (0x04) and
`INTERMEDIATE_DELIVERY` (0x20) — are reports whatever the body parses to, so one in a format
`dlrFromPdu()` cannot read reaches `dlr` with `smsId` undefined instead of arriving as an inbound
SMS. The three the far-end SME writes (0x08, 0x10, 0x18) are messages and their bodies are not
scraped: Kannel reads 0x08 as report-bearing and this does not, because a delivery acknowledgement
is the handset's word about a message, not the network's. A message type of 0 or one of the ten
reserved keeps the scrape, and a non-empty `receipted_message_id` TLV marks a report on the same
footing. A report this library recognises never reaches the reassembler, so an SMSC that splits one
across segments gets a `dlr` per segment rather than one merged report. The
`message_state` TLV is authoritative only where it names a state in the table — SMPP reserves
0x80-0xFF for MC-vendor-specific values, so an unnameable one keeps its raw `statusId` and leaves
`statusMsg` to the body.
+5 -3
View File
@@ -163,7 +163,9 @@ session.on('sms', async sms => {
Delivery receipts travel on the same SMPP command but reach you as `dlr`, so nothing you write has
to tell the two apart. `esm_class` is what tells them apart; where it names no message type a
`receipted_message_id` TLV does, and failing both the message body is read for the standard
`id:` and `stat:` receipt fields.
`id:` and `stat:` receipt fields. An intermediate delivery notification is the SMSC reporting too,
and arrives with `dlr.intermediate` true: nothing about the message is settled, and a later receipt
says how it ended.
Matching a receipt to a send means comparing `dlr.smsId` against the `smsIds` that `sendSms()`
returned. Some SMSCs write the two in different notations — a hex `message_id` on the
@@ -317,8 +319,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. `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. Merging needs the SMSC to number its segment ids `<base>-<n>`, 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 SMSC marked it a report it will follow with a final receipt. `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 `<base>-<n>`, 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. |
+3 -2
View File
@@ -41,7 +41,8 @@ const severity: Record<MessageState, number> = {
* Merges the per-segment receipts of a multipart message into one report, but only when the peer
* numbered its ids `<base>-<n>` off one base — the convention this library's own server follows. An
* SMSC that hands out unrelated ids per segment cannot be merged, so nothing is reported for it.
* A base is merged at most once: a reused id cannot be told apart from a straggler.
* A base is merged at most once: a reused id cannot be told apart from a straggler, and a report the
* peer marked intermediate is never counted — it would fill a slot before the real receipt arrives.
*/
export class DlrMerger {
private readonly groups: ExpiringGroups<Group>;
@@ -95,7 +96,7 @@ export class DlrMerger {
collect(dlr: Dlr): MessageDlr | undefined {
this.sweep();
if (dlr.smsId === undefined) return undefined;
if (dlr.intermediate || dlr.smsId === undefined) return undefined;
const match = numbered.exec(dlr.smsId);
const base = match?.[1];
+7 -5
View File
@@ -50,6 +50,7 @@ export type Receipt = {
export type Dlr = {
doneDate: Date | undefined;
errorCode: string | undefined;
intermediate: boolean;
receipt: Receipt | undefined;
smsId: string | undefined;
statusId: number;
@@ -127,13 +128,12 @@ export function parseReceipt(message: string): Receipt {
};
}
type MessageType = 'other' | 'receipt' | 'unmarked';
type MessageType = 'intermediate' | 'other' | 'receipt' | 'unmarked';
/** The message types the spec names that are not receipts. It reserves the remaining ten. */
const notReceiptTypes: number[] = [
/** The types the far-end SME writes, rather than the MC reporting. The spec reserves the remaining ten. */
const smeMessageTypes: number[] = [
consts.ESM_CLASS.CONVERSATION_ABORT,
consts.ESM_CLASS.DELIVERY_ACKNOWLEDGEMENT,
consts.ESM_CLASS.INTERMEDIATE_DELIVERY,
consts.ESM_CLASS.USER_ACKNOWLEDGEMENT,
];
@@ -145,7 +145,8 @@ function messageType(pduObj: PduObject): MessageType {
const type = messageTypeOf(paramNumber(pduObj.params.esm_class, 0));
if (type === consts.ESM_CLASS.MC_DELIVERY_RECEIPT) return 'receipt';
if (notReceiptTypes.includes(type)) return 'other';
if (type === consts.ESM_CLASS.INTERMEDIATE_DELIVERY) return 'intermediate';
if (smeMessageTypes.includes(type)) return 'other';
return nonEmptyText(pduObj.tlvs.receipted_message_id?.tagValue) === undefined ? 'unmarked' : 'receipt';
}
@@ -214,6 +215,7 @@ export function dlrFromPdu(pduObj: PduObject, format: SmsIdFormat = {}): Dlr | u
return {
doneDate: receiptDate(receipt?.doneDate),
errorCode: receipt?.err,
intermediate: type === 'intermediate',
receipt,
smsId,
statusId,
+17 -2
View File
@@ -145,6 +145,7 @@ describe('dlrFromPdu()', () => {
assert.equal(dlr.smsId, undefined);
assert.equal(dlr.statusMsg, 'UNKNOWN');
assert.equal(dlr.statusId, 7);
assert.equal(dlr.intermediate, false);
});
// pduToObj leaves a UDH-carrying short_message a buffer, so the body needs decoding before it
@@ -196,17 +197,31 @@ describe('dlrFromPdu()', () => {
assert.equal(dlr.statusMsg, 'DELIVERED');
});
test('leaves a message the peer marked as another type to arrive as an SMS', () => {
test('leaves a message the far-end SME marked as another type to arrive as an SMS', () => {
for (const esmClass of [
consts.ESM_CLASS.CONVERSATION_ABORT,
consts.ESM_CLASS.DELIVERY_ACKNOWLEDGEMENT,
consts.ESM_CLASS.INTERMEDIATE_DELIVERY,
consts.ESM_CLASS.USER_ACKNOWLEDGEMENT,
]) {
assert.equal(dlrFromPdu(deliverSm(receiptText, undefined, esmClass)), undefined);
}
});
test('reads an intermediate delivery notification as a report, marked as one', () => {
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));
assert.ok(dlr);
assert.equal(dlr.smsId, '0195f0c7');
assert.equal(dlr.statusMsg, 'ENROUTE');
assert.equal(dlr.intermediate, true);
const unreadable = dlrFromPdu(deliverSm('no fields here', undefined, consts.ESM_CLASS.INTERMEDIATE_DELIVERY));
assert.ok(unreadable, 'the marker makes it a report whatever the body parses to');
assert.equal(unreadable.intermediate, true);
});
test('exposes the raw receipt alongside the resolved fields', () => {
const dlr = dlrFromPdu(deliverSm(receiptText));
+20 -1
View File
@@ -203,10 +203,11 @@ describe('merged delivery reports', () => {
});
describe('merging segment statuses', () => {
function receipt(smsId: string, statusMsg: MessageState): Dlr {
function receipt(smsId: string, statusMsg: MessageState, intermediate = false): Dlr {
return {
doneDate: undefined,
errorCode: undefined,
intermediate,
receipt: undefined,
smsId,
statusId: consts.MESSAGE_STATE[statusMsg],
@@ -214,6 +215,24 @@ describe('merging segment statuses', () => {
};
}
// Filling every slot with a non-final report merges early and spends the base, so the receipts
// that say what actually happened would report nothing.
test('never counts an intermediate report toward a merge', () => {
const merger = new DlrMerger({ log: silentLog, max: 10, now: () => 0, timeout: 60_000 });
merger.expect(['msg-1', 'msg-2']);
assert.equal(merger.collect(receipt('msg-1', 'ENROUTE', true)), undefined);
assert.equal(merger.collect(receipt('msg-2', 'ENROUTE', true)), undefined);
assert.equal(merger.collect(receipt('msg-1', 'DELIVERED')), undefined);
const merged = merger.collect(receipt('msg-2', 'DELIVERED'));
assert.ok(merged);
assert.equal(merged.statusMsg, 'DELIVERED');
assert.equal(merged.segments.length, 2);
});
// MESSAGE_STATE is a flat enum: ACCEPTED is 6 where UNDELIVERABLE is 5, so reducing on the
// wire value called a part-failed message delivered.
test('reports the worse of two states the wire numbers the other way round', () => {
+24 -1
View File
@@ -685,7 +685,7 @@ describe('receiving', () => {
assert.equal(answered.pduObj.params.message_id, 'inbound-id');
});
test('hands a client a receipt it cannot read as a dlr rather than as an sms', async t => {
test('hands a client a report as a dlr rather than as an sms', async t => {
const { peer, session } = await inbound(t);
const reported = once<Dlr>(resolve => { session.on('dlr', resolve); });
let messages = 0;
@@ -711,6 +711,28 @@ describe('receiving', () => {
assert.ok(answered.pduObj);
assert.equal(answered.pduObj.cmdName, 'deliver_sm_resp');
const notified = once<Dlr>(resolve => { session.on('dlr', resolve); });
const notification = peer.send({
cmdName: 'deliver_sm',
params: {
destination_addr: '46709771337',
esm_class: consts.ESM_CLASS.INTERMEDIATE_DELIVERY,
short_message: 'id:0195f0c7 stat:ENROUTE err:000 text:',
source_addr: '46701113311',
},
});
const report = await raceWithin(2000, notified);
assert.ok(report, 'an intermediate notification is the MC reporting on our send, not an inbound SMS');
assert.equal(report.intermediate, true);
assert.equal(report.statusMsg, 'ENROUTE');
assert.equal(messages, 0);
const answeredNotification = await notification;
assert.ok(answeredNotification.pduObj);
assert.equal(answeredNotification.pduObj.cmdName, 'deliver_sm_resp');
});
test('reassembles a multipart inbound SMS before the sms event', async t => {
@@ -1494,6 +1516,7 @@ describe('merged delivery report bounds', () => {
return {
doneDate: undefined,
errorCode: undefined,
intermediate: false,
receipt: undefined,
smsId,
statusId: 2,
+1 -7
View File
@@ -64,6 +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` |
| 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/` |
@@ -131,13 +132,6 @@ 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.
- [ ] **Does an intermediate delivery notification deserve to be a `dlr`?** `esm_class` message type
`INTERMEDIATE_DELIVERY` (0x20) is classified as a message today, so a peer that reports
non-final states with it hands the application a raw `id:… stat:ENROUTE` text as an inbound
SMS. Kannel treats 0x04, 0x08 and 0x20 alike as report-bearing. Against it: a non-final report
would take a segment's slot in `DlrMerger` and complete the group early. Raised by review,
2026-08-27; 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