Drop a request whose link went while onRequest ran, and pin the wire in the test

This commit is contained in:
2026-09-01 20:40:17 +02:00
parent c8d265d5fa
commit e6250ef44e
5 changed files with 42 additions and 13 deletions
+3 -6
View File
@@ -308,12 +308,9 @@ Grouped by what each one constrains.
`attach()`, which is why a second drop emits again.
- **An answer belongs to the link the message arrived on; a receipt does not.** Maintainer's call,
2026-09-01: `sendResp()` carries the peer's own sequence number, which the next link knows nothing
about, so on a message whose link went it writes nothing and returns an `err`. Writing it to the
new socket instead succeeds and reports `{}` for a response that correlates with nothing, which is
goal 2's wrong answer. `sendDlr()` still goes out, because a receipt is a request of its own
correlated by `receipted_message_id`; the accepted cost is that one sent after a refused response
names an id the peer has no record of.
2026-09-01. Rejected: answering on the new link, which succeeds and reports `{}` for a response
that correlates with nothing — goal 2's wrong answer. Accepted: a receipt sent after a refused
response names an id the peer has no record of.
- **`reconnect` takes `{ minDelay, maxDelay }` to retune and `false` to turn off**, so absent means
on and there is one spelling for each. Only `client()` reconnects — a `server()` session is a
+1 -1
View File
@@ -316,7 +316,7 @@ 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 the `smsId` it was answered with. |
| `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. |
| `close` | The session is over, because nothing will bring the link back. Fires once, whether you closed it or the link failed for good. |
+5 -1
View File
@@ -39,7 +39,6 @@ export class IncomingRequests {
private readonly session: Session;
private readonly smsIdFormat: SmsIdFormat;
private readonly systemId: string;
/** Bumped when a link goes, so a message from an earlier one knows its answer cannot correlate. */
private linkGeneration = 0;
constructor(options: IncomingRequestsOptions) {
@@ -64,8 +63,13 @@ export class IncomingRequests {
}
async handle(pduObj: PduObject): Promise<void> {
const generation = this.linkGeneration;
if (this.onRequest && await this.onRequest(this.session, pduObj)) return;
// The link it arrived on went while the hook ran, so nothing we answer now correlates.
if (this.linkGeneration !== generation) return;
if (!this.session.bindAllows(pduObj.cmdName)) {
this.log.info('session - command the peer\'s bind direction does not carry', {
bindType: this.session.boundAs ?? '',
+1 -1
View File
@@ -38,7 +38,7 @@ export type Sms = {
/** Answers every segment. Part of the protocol, not optional. Defaults to ESME_ROK. */
sendResp: (options?: SendRespOptions) => Promise<VoidResult>;
session: Session;
/** The id answered to the peer: what sendResp() was given, or a generated UUID v7. */
/** The id `sendResp()` was given, or a generated UUID v7. */
readonly smsId: string;
submitTime: Date;
to: string;
+32 -4
View File
@@ -13,6 +13,7 @@ import type { Sms } from '../src/sms.ts';
import type { SmppServer } from '../src/server.ts';
import type { TestContext } from 'node:test';
import { HeldMessages } from '../src/held-messages.ts';
import { IncomingRequests } from '../src/incoming-requests.ts';
import { UnansweredError } from '../src/unanswered-error.ts';
import { createSms } from '../src/sms.ts';
import { LinkGate } from '../src/link-gate.ts';
@@ -589,6 +590,7 @@ describe('reconnect', () => {
assert.ok(answered);
assert.ok(held);
assert.equal(answered.message, 'answered before the drop');
assert.equal((await answered.sendResp()).err, undefined);
const reconnected = once<true>(resolve => { session.on('reconnected', () => { resolve(true); }); });
@@ -596,17 +598,43 @@ describe('reconnect', () => {
await peerOf(smpp).close({ signal: AbortSignal.abort() });
await reconnected;
const taken: string[] = [];
let taken = 0;
peerOf(smpp).on('incomingPduObj', pduObj => { taken.push(pduObj.cmdName); });
// A response is dispatched before `incomingPduObj`, so only the raw event sees one arrive.
peerOf(smpp).on('incomingPdu', () => { taken++; });
// The answered one is out of the hold and the held one is not, so neither may reach the answer.
assert.match((await answered.sendResp()).err?.message ?? '', /link this message arrived on is gone/);
assert.match((await held.sendResp()).err?.message ?? '', /link this message arrived on is gone/);
// A receipt is a request of its own, correlated by its id, so the new link carries it.
assert.equal((await held.sendDlr('DELIVERED')).err, undefined);
assert.deepEqual(taken, ['deliver_sm'], 'a response reached the new link');
assert.equal(taken, 1, 'a refused response reached the new link');
});
test('drops a message whose link went while onRequest was still running', async t => {
const session = new Session({ sock: new net.Socket() });
closeAfter(t, session);
session.boundAs = 'transceiver';
const incoming = new IncomingRequests({
dlrMerger: new DlrMerger({ log: silentLog, max: 10, timeout: 10_000 }),
log: silentLog,
onRequest: async () => { await delay(10); return false; },
sendPastDrain: () => Promise.resolve({ err: new Error('never sent') }),
session,
});
let messages = 0;
session.on('sms', () => { messages++; });
const handled = incoming.handle(submitPdu(1));
incoming.clear();
await handled;
assert.equal(messages, 0);
});
test('does not reconnect after an explicit close', async t => {