Read a receipt reporting a transient state as non-final too, so the merge waits for the real one
This commit is contained in:
@@ -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
|
0x80-0xFF for MC-vendor-specific values, so an unnameable one keeps its raw `statusId` and leaves
|
||||||
`statusMsg` to the body.
|
`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 `<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
|
||||||
transform over both sides cannot make them equal. `submitResp` covers the `receipted_message_id`
|
transform over both sides cannot make them equal. `submitResp` covers the `receipted_message_id`
|
||||||
|
|||||||
@@ -318,8 +318,8 @@ TypeScript users can import `SmppLog` to have the compiler check one.
|
|||||||
| Event | Fires when |
|
| Event | Fires when |
|
||||||
| --- | --- |
|
| --- | --- |
|
||||||
| `sms` | An SMS arrives, reassembled if it was multipart. Carries `sendResp()`, `sendDlr()` and its `smsId`. |
|
| `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`. |
|
| `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. 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. |
|
| `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 `<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. |
|
| `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`. |
|
| `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. |
|
| `reconnected` | The client re-bound after a drop. |
|
||||||
|
|||||||
+7
-2
@@ -130,6 +130,9 @@ 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. */
|
||||||
|
const transientStates: 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: number[] = [
|
||||||
consts.ESM_CLASS.CONVERSATION_ABORT,
|
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;
|
if (type === 'unmarked' && (smsId === undefined || statusMsg === undefined)) return undefined;
|
||||||
|
|
||||||
|
const state = statusMsg ?? 'UNKNOWN';
|
||||||
|
|
||||||
return {
|
return {
|
||||||
doneDate: receiptDate(receipt?.doneDate),
|
doneDate: receiptDate(receipt?.doneDate),
|
||||||
errorCode: receipt?.err,
|
errorCode: receipt?.err,
|
||||||
intermediate: type === 'intermediate',
|
intermediate: type === 'intermediate' || transientStates.includes(state),
|
||||||
receipt,
|
receipt,
|
||||||
smsId,
|
smsId,
|
||||||
statusId,
|
statusId,
|
||||||
statusMsg: statusMsg ?? 'UNKNOWN',
|
statusMsg: state,
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|||||||
+18
-11
@@ -98,22 +98,23 @@ describe('dlrFromPdu()', () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
test('maps every spec status code back to its message state and id', () => {
|
test('maps every spec status code back to its message state and id', () => {
|
||||||
for (const [code, expected, statusId] of [
|
for (const [code, expected, statusId, intermediate] of [
|
||||||
['DELIVRD', 'DELIVERED', 2],
|
['DELIVRD', 'DELIVERED', 2, false],
|
||||||
['UNDELIV', 'UNDELIVERABLE', 5],
|
['UNDELIV', 'UNDELIVERABLE', 5, false],
|
||||||
['EXPIRED', 'EXPIRED', 3],
|
['EXPIRED', 'EXPIRED', 3, false],
|
||||||
['DELETED', 'DELETED', 4],
|
['DELETED', 'DELETED', 4, false],
|
||||||
['ACCEPTD', 'ACCEPTED', 6],
|
['ACCEPTD', 'ACCEPTED', 6, false],
|
||||||
['REJECTD', 'REJECTED', 8],
|
['REJECTD', 'REJECTED', 8, false],
|
||||||
['ENROUTE', 'ENROUTE', 1],
|
['ENROUTE', 'ENROUTE', 1, true],
|
||||||
['UNKNOWN', 'UNKNOWN', 7],
|
['UNKNOWN', 'UNKNOWN', 7, false],
|
||||||
['delivrd', 'DELIVERED', 2],
|
['delivrd', 'DELIVERED', 2, false],
|
||||||
] as const) {
|
] as const) {
|
||||||
const dlr = dlrFromPdu(deliverSm(`id:x stat:${code} err:0`));
|
const dlr = dlrFromPdu(deliverSm(`id:x stat:${code} err:0`));
|
||||||
|
|
||||||
assert.ok(dlr);
|
assert.ok(dlr);
|
||||||
assert.equal(dlr.statusMsg, expected);
|
assert.equal(dlr.statusMsg, expected);
|
||||||
assert.equal(dlr.statusId, statusId);
|
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 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));
|
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.ok(unreadable, 'the marker makes it a report whatever the body parses to');
|
||||||
assert.equal(unreadable.intermediate, true);
|
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', () => {
|
test('exposes the raw receipt alongside the resolved fields', () => {
|
||||||
|
|||||||
@@ -168,6 +168,45 @@ describe('merged delivery reports', () => {
|
|||||||
assert.deepEqual(perSegment, ['merge-me-1', 'merge-me-2', 'merge-me-3']);
|
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<Sms>(resolve => {
|
||||||
|
smpp.on('session', session => session.on('sms', resolve));
|
||||||
|
});
|
||||||
|
const { session } = await connect(t, smpp);
|
||||||
|
|
||||||
|
assert.ok(session);
|
||||||
|
|
||||||
|
const merged = once<MessageDlr>(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 => {
|
test('reports the worst status across the segments', async t => {
|
||||||
const smpp = await startServer(t);
|
const smpp = await startServer(t);
|
||||||
const incoming = once<Sms>(resolve => {
|
const incoming = once<Sms>(resolve => {
|
||||||
|
|||||||
@@ -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` |
|
| 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` |
|
| Every runnable README example | `test/readme.test.ts` |
|
||||||
| Receipt-versus-message classification by `esm_class` | `test/dlr.test.ts`, `test/session.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` |
|
| 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` |
|
| 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/` |
|
| CI on Node 18/20/22/24, Renovate, tag-triggered publish | `.github/workflows/` |
|
||||||
|
|||||||
Reference in New Issue
Block a user