diff --git a/AGENTS.md b/AGENTS.md index 2536e28..e224719 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -195,6 +195,10 @@ exactly 140. - Comments are the exception, not the default — see the root `CLAUDE.md` rules. Do not write file preambles or restate what the code says. - Test data uses real randomised UUID v7 values, never `aaaa-0000` placeholders. +- Fixtures that encode the wire are shared so no two files can drift on it: `test/raw-pdus.ts` builds + the octets a test writes straight to a socket, the PDUs `objToPdu()` refuses to build included. The + waiting helpers each file carries are copies, tolerated because a wrong one fails that file's own + tests and nothing else. - `message_id` values the library generates are UUID v7. - A test that needs a dummy peer must `resume()` its sockets. An unread socket never processes the peer's FIN, so `server.close()` hangs forever — that is a test bug, not a library one. @@ -265,6 +269,26 @@ Grouped by what each one constrains. cannot be re-declared the same way — Node types it invariantly enough that widening `void` to `unknown` is `TS2416`. Re-probed 2026-09-01; `sendResp()` stays the signal. +- **`PduRefusedError` is exported, and `sessionError` names it in the event's type.** Maintainer's + call, 2026-09-05, from a product review: one event carries both a PDU the peer malformed and the + session's own failure, and `instanceof` is the only way to separate them that hard rule 4 allows — + without the class as a value an application is left string-matching `err.message`. Goal 6 is paid by + exporting the discriminant and the struct it carries and nothing else: `PduHeader` is named because + an application that logs or forwards a header wants a name for it, `PduRefusalReason` is not + because `reason` is compared against string literals, and an accessor + (`PduRefusedError['header']`) names either one where a signature wants it. The payload union + enforces nothing — a subclass narrows out of `Error` either way — and is there so the event's own + type names what to narrow to, which is also what makes it a half-truth if a second `Error` subclass + ever reaches this event without joining it. Rejected: a `SessionError` alias for that union, a + third name for a type that is structurally `Error`. Rejected: a separate `pduRefused` event, which + splits the failure channel so an application that wants every failure listens twice and an existing + listener silently stops seeing refusals. Rejected: coalescing or rate-limiting them, which re-opens + the standing decision that `sessionError` carries every failure, never coalesced or suppressed — + the filtering belongs where the application is, since only it knows which peer is routinely sloppy. + Rejected: an error code on a plain `Error`, which reads back off an `unknown` property only through + a cast and types nothing it carries. Accepted: a second copy of the package installed alongside + this one defeats `instanceof`, where `err.name` still reads `PduRefusedError`. + ### The wire - **The declared interface version is an option on both `client()` and `server()`, and is not the diff --git a/README.md b/README.md index 6af455e..ddbc68e 100644 --- a/README.md +++ b/README.md @@ -265,6 +265,41 @@ Runtime failures on a live connection arrive as `sessionError` and `serverError` deliberately not called `error`: Node turns an unhandled `error` event into a thrown exception, which is exactly what this library promises not to do. +`sessionError` carries two kinds of failure, and `PduRefusedError` is what separates them: + +- **A PDU the peer sent that the codec could not read with the stream still in sync.** The link is + healthy and only that one PDU is lost, so this is the kind to count rather than alert on. +- **Everything else**: the session or the socket failing, and a hook or listener that threw or, if it + was `async`, rejected. + +```javascript +import { PduRefusedError } from '@larvit/smpp'; + +session.on('sessionError', err => { + if (err instanceof PduRefusedError) { + log.warn('the peer sent a PDU that could not be read', { + cmdName: err.header.cmdName ?? err.header.cmdId, + reason: err.reason, + }); + + return; + } + + log.error('the session failed', { message: err.message }); +}); +``` + +`reason` is `command`, `body` or `tlvs`, naming the part the codec stopped at, and `header` is the 16 +octets that did parse: `cmdId`, `cmdLength`, `cmdName`, `cmdStatusId` and `seqNr`. `cmdName` is +undefined where the command id names no command this library knows — `PduHeader` is its type, for a +TypeScript consumer passing it on. + +A refused inbound `deliver_sm` is lost traffic: a message or a receipt that never arrives as `sms` or +`dlr`, and this event is where that loss shows up. A refused *response* is reported twice where a +call is still waiting on it, once as the `err` that `sendSms()` or `send()` returns and once here. +That is deliberate: the call answers what became of that one send, and the event is what shows a peer +answering unreadably at all. + ## Logging `log` takes any object with `debug`, `error`, `info`, `verbose` and `warn` methods, each @@ -306,7 +341,7 @@ TypeScript users can import `SmppLog` to have the compiler check one. | `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. | -| `sessionError` | Something failed on a live session, including a hook or listener that threw or, if it was `async`, rejected. Fires for each PDU the codec refused, and the link carries on: a refused request is answered with the status SMPP names, and a refused response is answered with nothing and settles the request it named as `unanswered`. | +| `sessionError` | Something failed on a live session, including a hook or listener that threw or, if it was `async`, rejected. Fires for each PDU the codec refused as well, carrying a `PduRefusedError` while the link carries on: a refused request is answered with the status SMPP names, and a refused response is answered with nothing and settles the request it named as `unanswered`. [Errors](#errors) tells the two kinds apart. | | `data` | Raw bytes arrived on the socket. | | `incomingPdu` | A complete PDU arrived, as a buffer. | | `incomingPduObj` | The same PDU, parsed into an object. | diff --git a/src/index.ts b/src/index.ts index a2a3ade..4812caf 100644 --- a/src/index.ts +++ b/src/index.ts @@ -18,7 +18,7 @@ export { pduToObj, } from './pdu.ts'; -export { maxPduLength } from './pdu-refusal.ts'; +export { maxPduLength, PduRefusedError } from './pdu-refusal.ts'; export { bitCount, @@ -62,6 +62,7 @@ export type { ConstGroup, MessageState } from './defs/constants.ts'; export type { Encoding, EncodingName } from './defs/encodings.ts'; export type { ErrorName } from './defs/errors.ts'; export type { PduObject, PduObjectInput, TlvInput } from './pdu.ts'; +export type { PduHeader } from './pdu-refusal.ts'; export type { SplitOptions } from './message.ts'; export type { Tlv, TlvDefinition, TlvName } from './defs/tlvs.ts'; export type { DestAddress, ParamValue, UnsuccessSme, WireType } from './defs/types.ts'; diff --git a/src/session-options.ts b/src/session-options.ts index e8f613a..f0743ff 100644 --- a/src/session-options.ts +++ b/src/session-options.ts @@ -1,6 +1,7 @@ import type { Dlr } from './dlr.ts'; import type { MessageDlr } from './dlr-merger.ts'; import type { PduObject } from './pdu.ts'; +import type { PduRefusedError } from './pdu-refusal.ts'; import type { Result, VoidResult } from './result.ts'; import type { Session } from './session.ts'; import type { SmppLog } from './log.ts'; @@ -19,7 +20,7 @@ export type SessionEvents = { incomingPduObj: [PduObject]; messageDlr: [MessageDlr]; reconnected: []; - sessionError: [Error]; + sessionError: [Error | PduRefusedError]; sms: [Sms]; }; diff --git a/test/raw-pdus.ts b/test/raw-pdus.ts new file mode 100644 index 0000000..90b316f --- /dev/null +++ b/test/raw-pdus.ts @@ -0,0 +1,40 @@ +import assert from 'node:assert/strict'; +import type { PduObjectInput } from '../src/pdu.ts'; +import { objToPdu } from '../src/pdu.ts'; + +/** The octets a test writes straight to a socket, which objToPdu builds for every valid PDU. */ +export function pduBytes(input: PduObjectInput): Buffer { + const { buffer } = objToPdu(input); + + assert.ok(buffer); + + return buffer; +} + +/** The same, wearing a command id the command table defines nothing for. */ +export function withUnknownCmdId(input: PduObjectInput): Buffer { + const buffer = pduBytes(input); + + buffer.writeUInt32BE(0x00010001, 4); + + return buffer; +} + +/** command_length honoured, so the stream stays in sync, with the declared body cut short. */ +export function shortened(input: PduObjectInput, octets: number): Buffer { + const buffer = pduBytes(input); + const cut = buffer.subarray(0, buffer.length - octets); + + cut.writeUInt32BE(cut.length, 0); + + return cut; +} + +/** The same, with a message_state TLV declaring four octets of value and carrying one. */ +export function truncatedTlv(input: PduObjectInput): Buffer { + const appended = Buffer.concat([pduBytes(input), Buffer.from('0427000401', 'hex')]); + + appended.writeUInt32BE(appended.length, 0); + + return appended; +} diff --git a/test/readme.test.ts b/test/readme.test.ts index d68ce28..0895180 100644 --- a/test/readme.test.ts +++ b/test/readme.test.ts @@ -6,6 +6,8 @@ import type { Sms } from '../src/sms.ts'; import type { SmppLog } from '../src/log.ts'; import type { SmppServer } from '../src/server.ts'; import type { TestContext } from 'node:test'; +import { PduRefusedError } from '../src/pdu-refusal.ts'; +import { objToPdu } from '../src/pdu.ts'; import { client } from '../src/client.ts'; import { closeAfter } from './teardown.ts'; import { server } from '../src/server.ts'; @@ -258,4 +260,52 @@ describe('README: Errors', () => { assert.ok(err instanceof Error); assert.equal(session, undefined); }); + + test('telling a refused PDU from a session that failed', async t => { + const smpp = await answeringServer(t); + const bound = once(resolve => { smpp.on('session', resolve); }); + const { err, session } = await client(); + if (err) throw err; + + closeAfter(t, session); + + const warned: Record[] = []; + const log: SmppLog = { + debug: () => undefined, + error: () => undefined, + info: () => undefined, + verbose: () => undefined, + warn: (msg, metadata) => { warned.push({ msg, ...metadata }); }, + }; + + session.on('sessionError', err => { + if (err instanceof PduRefusedError) { + log.warn('the peer sent a PDU that could not be read', { + cmdName: err.header.cmdName ?? err.header.cmdId, + reason: err.reason, + }); + + return; + } + + log.error('the session failed', { message: err.message }); + }); + + const reported = once(resolve => { session.on('sessionError', resolve); }); + const peer = await bound; + const { buffer } = objToPdu({ cmdName: 'enquire_link', seqNr: 5 }); + + assert.ok(buffer); + // A command id no command is defined for: refused on its own, with the link untouched. + buffer.writeUInt32BE(0x00010001, 4); + peer.sock.write(buffer); + + await reported; + + assert.deepEqual(warned, [{ + cmdName: 0x00010001, + msg: 'the peer sent a PDU that could not be read', + reason: 'command', + }]); + }); }); diff --git a/test/session-error.test.ts b/test/session-error.test.ts new file mode 100644 index 0000000..7496f17 --- /dev/null +++ b/test/session-error.test.ts @@ -0,0 +1,122 @@ +import assert from 'node:assert/strict'; +import test, { describe } from 'node:test'; +import type { PduHeader, Session } from '../src/index.ts'; +import type { TestContext } from 'node:test'; +import { PduRefusedError, client, server } from '../src/index.ts'; +import { closeAfter } from './teardown.ts'; +import { shortened, truncatedTlv, withUnknownCmdId } from './raw-pdus.ts'; + +const receipt = { + destination_addr: '46709771337', + short_message: 'id:01a07342-a1d0-7479-88d6-c80082e19aec stat:DELIVRD err:000 text:', + source_addr: '46701113311', +}; + +function once(register: (resolve: (value: T) => void) => void): Promise { + return new Promise(resolve => { register(resolve); }); +} + +function delay(ms: number): Promise { + return new Promise(resolve => { setTimeout(resolve, ms); }); +} + +function raceWithin(ms: number, promise: Promise): Promise { + return Promise.race([promise, delay(ms).then((): false => false)]); +} + +/** Polls until the condition holds; false means it never did within the budget. */ +async function waitFor(condition: () => boolean, budget = 2000): Promise { + const deadline = Date.now() + budget; + + while (!condition()) { + if (Date.now() > deadline) return false; + + await delay(5); + } + + return true; +} + +/** A bound client, and the server session at the far end whose socket the tests write from. */ +async function linked(t: TestContext, options: Parameters[0] = {}) { + const { err, server: smpp } = await server({ port: 0 }); + + assert.equal(err, undefined); + assert.ok(smpp); + closeAfter(t, smpp); + + const accepted = once(resolve => { smpp.on('session', resolve); }); + const { session } = await client({ port: smpp.port, ...options }); + + assert.ok(session); + closeAfter(t, session); + + const peer = await raceWithin(2000, accepted); + + assert.ok(peer, 'the server never accepted a session'); + + return { peer, session }; +} + +describe('telling a refused PDU from a failed session', () => { + test('narrows a refusal to its class, command and reason, from the entry point alone', async t => { + const { peer, session } = await linked(t); + const failed = once(resolve => { session.on('sessionError', resolve); }); + + peer.sock.write(truncatedTlv({ cmdName: 'deliver_sm', params: receipt, seqNr: 5 })); + + const reported = await raceWithin(2000, failed); + + assert.ok(reported instanceof PduRefusedError, 'a refusal narrows without a cast'); + + const header: PduHeader = reported.header; + + assert.equal(header.cmdName, 'deliver_sm'); + assert.equal(header.seqNr, 5); + assert.equal(reported.reason, 'tlvs'); + }); + + test('names which part of the PDU it could not read, one refusal at a time', async t => { + const { peer, session } = await linked(t); + const seen: Error[] = []; + + session.on('sessionError', err => { seen.push(err); }); + + peer.sock.write(withUnknownCmdId({ cmdName: 'enquire_link', seqNr: 9 })); + peer.sock.write(shortened({ cmdName: 'deliver_sm', params: receipt, seqNr: 6 }, 3)); + peer.sock.write(truncatedTlv({ cmdName: 'deliver_sm', params: receipt, seqNr: 7 })); + + assert.ok(await waitFor(() => seen.length === 3), 'one sessionError per refused PDU'); + assert.deepEqual( + seen.map(err => (err instanceof PduRefusedError ? err.reason : err.message)), + ['command', 'body', 'tlvs'], + ); + }); + + test('reports a listener that threw as an error that is no refusal', async t => { + const { peer, session } = await linked(t, { responseTimeout: 200 }); + const failed = once(resolve => { peer.on('sessionError', resolve); }); + + peer.on('sms', () => { throw new Error('listener exploded'); }); + + await session.sendSms({ from: '46701113311', message: 'blows the listener up', to: '46709771337' }); + + const reported = await raceWithin(2000, failed); + + assert.ok(reported, 'the listener that threw never reached the session'); + assert.ok(!(reported instanceof PduRefusedError), 'a session failure is not a refused PDU'); + assert.equal(reported.message, 'listener exploded'); + }); + + test('reports a socket the peer reset as an error that is no refusal', async t => { + const { peer, session } = await linked(t, { reconnect: false }); + const failed = once(resolve => { session.on('sessionError', resolve); }); + + peer.sock.resetAndDestroy(); + + const reported = await raceWithin(2000, failed); + + assert.ok(reported, 'a reset socket never reached the session'); + assert.ok(!(reported instanceof PduRefusedError), 'a dead socket is not a refused PDU'); + }); +}); diff --git a/test/session-extras.test.ts b/test/session-extras.test.ts index dedbff0..810eb7b 100644 --- a/test/session-extras.test.ts +++ b/test/session-extras.test.ts @@ -20,12 +20,13 @@ import { LinkGate } from '../src/link-gate.ts'; import { Reassembler, decodeSegments } from '../src/reassembly.ts'; import { Session } from '../src/session.ts'; import { DlrMerger } from '../src/dlr-merger.ts'; +import { PduRefusedError } from '../src/pdu-refusal.ts'; +import { objToPdu } from '../src/pdu.ts'; import { checkSessionOptions } from '../src/session-options.ts'; import { client } from '../src/client.ts'; import { closeAfter, closeListenerAfter } from './teardown.ts'; import { consts } from '../src/defs/constants.ts'; import { errors } from '../src/defs/errors.ts'; -import { objToPdu } from '../src/pdu.ts'; import { paramNumber, paramText } from '../src/defs/types.ts'; import { server } from '../src/server.ts'; import { silentLog } from '../src/log.ts'; @@ -499,14 +500,16 @@ describe('reconnect', () => { const events: string[] = []; session.on('close', () => { events.push('close'); }); - session.on('sessionError', () => { events.push('sessionError'); }); + session.on('sessionError', err => { + events.push(err instanceof PduRefusedError ? 'refused' : 'sessionError'); + }); const reconnected = once(resolve => { session.on('reconnected', () => { resolve(true); }); }); peerOf(smpp).sock.write(unreadablePdu); await reconnected; - assert.deepEqual(events, ['sessionError']); + assert.deepEqual(events, ['sessionError'], 'an unframeable stream is the link dying, not one PDU refused'); }); test('refuses a backoff that would retry without pausing', () => { diff --git a/test/session.test.ts b/test/session.test.ts index 04f3061..17923c7 100644 --- a/test/session.test.ts +++ b/test/session.test.ts @@ -15,9 +15,11 @@ import { Session, bindCommands } from '../src/session.ts'; import { client } from '../src/client.ts'; import { closeAfter, closeListenerAfter } from './teardown.ts'; import { consts } from '../src/defs/constants.ts'; +import { PduRefusedError } from '../src/pdu-refusal.ts'; import { isCommand, objToPdu, pduReturn, pduToObj } from '../src/pdu.ts'; import { paramText } from '../src/defs/types.ts'; import { server } from '../src/server.ts'; +import { pduBytes, shortened, truncatedTlv, withUnknownCmdId } from './raw-pdus.ts'; import { silentLog } from '../src/log.ts'; import { splitMessage } from '../src/message.ts'; @@ -1241,42 +1243,6 @@ describe('a PDU the codec cannot read', () => { source_addr: '46701113311', }; - /** The octets objToPdu built, wearing a sequence number it would refuse to write itself. */ - function withSeqNr(input: PduObjectInput, seqNr: number): Buffer { - const { buffer } = objToPdu(input); - - assert.ok(buffer); - buffer.writeUInt32BE(seqNr, 12); - - return buffer; - } - - /** command_length honoured, so the stream stays in sync, with the declared body cut short. */ - function shortened(input: PduObjectInput, octets: number): Buffer { - const { buffer } = objToPdu(input); - - assert.ok(buffer); - - const cut = buffer.subarray(0, buffer.length - octets); - - cut.writeUInt32BE(cut.length, 0); - - return cut; - } - - /** The same, with a message_state TLV declaring four octets of value and carrying one. */ - function truncatedTlv(input: PduObjectInput): Buffer { - const { buffer } = objToPdu(input); - - assert.ok(buffer); - - const appended = Buffer.concat([buffer, Buffer.from('0427000401', 'hex')]); - - appended.writeUInt32BE(appended.length, 0); - - return appended; - } - /** The peer's next PDU within a budget: a dropped link must fail the test, not hang it. */ async function answerTo(peer: Peer): Promise { const pduObj = await raceWithin(2000, peer.next()); @@ -1302,7 +1268,7 @@ describe('a PDU the codec cannot read', () => { const { peer, session } = await bound(t); const reported = once(resolve => { session.on('dlr', resolve); }); - peer.writeRaw(withSeqNr({ cmdName: 'deliver_sm', params: receipt, seqNr: 1 }, 0x80000001)); + peer.writeRaw(pduBytes({ cmdName: 'deliver_sm', params: receipt, seqNr: 0x80000001 })); const answered = await answerTo(peer); @@ -1315,7 +1281,7 @@ describe('a PDU the codec cannot read', () => { assert.ok(dlr, 'the receipt is a report, not a reason to drop the link'); assert.equal(dlr.statusMsg, 'DELIVERED'); - peer.writeRaw(withSeqNr({ cmdName: 'enquire_link', seqNr: 1 }, 0xFFFFFFFF)); + peer.writeRaw(pduBytes({ cmdName: 'enquire_link', seqNr: 0xFFFFFFFF })); const pinged = await answerTo(peer); @@ -1326,10 +1292,8 @@ describe('a PDU the codec cannot read', () => { test('answers an unknown command id with generic_nack ESME_RINVCMDID', async t => { const { peer, session } = await bound(t); const failed = once(resolve => { session.on('sessionError', resolve); }); - const vendorSpecific = withSeqNr({ cmdName: 'enquire_link', seqNr: 1 }, 9); - vendorSpecific.writeUInt32BE(0x00010001, 4); - peer.writeRaw(vendorSpecific); + peer.writeRaw(withUnknownCmdId({ cmdName: 'enquire_link', seqNr: 9 })); const answered = await answerTo(peer); @@ -1354,7 +1318,7 @@ describe('a PDU the codec cannot read', () => { assert.equal(reports, 0, 'a refused PDU is not a report'); // The regression this fixes: the link, and the stream's sync, outlive the refused PDU. - peer.writeRaw(withSeqNr({ cmdName: 'enquire_link', seqNr: 1 }, 78)); + peer.writeRaw(pduBytes({ cmdName: 'enquire_link', seqNr: 78 })); assert.equal((await answerTo(peer)).cmdName, 'enquire_link_resp'); }); @@ -1370,8 +1334,9 @@ describe('a PDU the codec cannot read', () => { assert.equal(answered.seqNr, 6); }); - test('settles the request a response it could not read was answering', async t => { + test('settles the request a response it could not read was answering, and reports it', async t => { const { peer, session } = await bound(t, { responseTimeout: 60000 }); + const failed = once(resolve => { session.on('sessionError', resolve); }); const sending = session.sendSms({ from: '46701113311', message: 'hi', to: '46709771337' }); const submitted = await answerTo(peer); @@ -1388,8 +1353,13 @@ describe('a PDU the codec cannot read', () => { assert.ok(sent.err instanceof Error); assert.equal(sent.unanswered, 1); + const reported = await raceWithin(2000, failed); + + assert.ok(reported instanceof PduRefusedError); + assert.equal(reported.header.cmdName, 'submit_sm_resp'); + // Nothing goes back: a response carries a sequence number of ours, not one of the peer's. - peer.writeRaw(withSeqNr({ cmdName: 'enquire_link', seqNr: 1 }, 77)); + peer.writeRaw(pduBytes({ cmdName: 'enquire_link', seqNr: 77 })); assert.equal((await answerTo(peer)).cmdName, 'enquire_link_resp'); });