Route a rejecting async listener to sessionError instead of the process
This commit is contained in:
@@ -213,6 +213,14 @@ exactly 140.
|
|||||||
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 listener that rejects is routed by Node's `captureRejections`, not by hand-dispatching.** Both
|
||||||
|
emitters construct with `captureRejections: true` and implement
|
||||||
|
`[EventEmitter.captureRejectionSymbol]`, which lands a rejected `async` listener on `sessionError`
|
||||||
|
or `serverError` beside the synchronous guard in `emit()`. Dispatching `rawListeners()` from
|
||||||
|
`emit()` instead needs a cast to call them with the event's argument tuple, which hard rule 4
|
||||||
|
forbids. `Session.on()` still types its listeners as void-returning, because widening that needs
|
||||||
|
the same cast — which is why `test/*.test.ts` turns `no-misused-promises` off.
|
||||||
|
|
||||||
- **The TLS tests build their own self-signed certificate in DER** (`test/tls.test.ts`) instead of
|
- **The TLS tests build their own self-signed certificate in DER** (`test/tls.test.ts`) instead of
|
||||||
adding a devDependency or shelling out to openssl. Maintainer's call, 2026-08-26: the dev image
|
adding a devDependency or shelling out to openssl. Maintainer's call, 2026-08-26: the dev image
|
||||||
`node:24.18.0-bookworm-slim` ships no openssl binary, so a shelled-out fixture would pass in CI
|
`node:24.18.0-bookworm-slim` ships no openssl binary, so a shelled-out fixture would pass in CI
|
||||||
|
|||||||
@@ -300,7 +300,7 @@ TypeScript users can import `SmppLog` to have the compiler check one.
|
|||||||
| `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. |
|
| `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. |
|
||||||
| `close` | The connection closed. |
|
| `close` | The connection closed. |
|
||||||
| `reconnected` | The client re-bound after a drop (only with `reconnect` configured). |
|
| `reconnected` | The client re-bound after a drop (only with `reconnect` configured). |
|
||||||
| `sessionError` | Something failed on a live session, including a hook or listener that threw. |
|
| `sessionError` | Something failed on a live session, including a hook or listener that threw or, if it was `async`, rejected. |
|
||||||
| `data` | Raw bytes arrived on the socket. |
|
| `data` | Raw bytes arrived on the socket. |
|
||||||
| `incomingPdu` | A complete PDU arrived, as a buffer. |
|
| `incomingPdu` | A complete PDU arrived, as a buffer. |
|
||||||
| `incomingPduObj` | The same PDU, parsed into an object. |
|
| `incomingPduObj` | The same PDU, parsed into an object. |
|
||||||
|
|||||||
+2
-2
@@ -49,8 +49,8 @@ export default tseslint.config(
|
|||||||
rules: { 'no-control-regex': 'off' },
|
rules: { 'no-control-regex': 'off' },
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
// Mirrors the README's examples as written; an async event listener is part of what they show.
|
// An async event listener is a documented, supported shape; EventEmitter types listeners void.
|
||||||
files: ['test/readme.test.ts'],
|
files: ['test/*.test.ts'],
|
||||||
rules: { '@typescript-eslint/no-misused-promises': 'off' },
|
rules: { '@typescript-eslint/no-misused-promises': 'off' },
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
|
|||||||
+13
-1
@@ -58,7 +58,7 @@ export class SmppServer extends EventEmitter<ServerEvents> {
|
|||||||
private readonly server: NetServer;
|
private readonly server: NetServer;
|
||||||
|
|
||||||
constructor(server: NetServer, log: SmppLog) {
|
constructor(server: NetServer, log: SmppLog) {
|
||||||
super();
|
super({ captureRejections: true });
|
||||||
this.log = log;
|
this.log = log;
|
||||||
this.server = server;
|
this.server = server;
|
||||||
}
|
}
|
||||||
@@ -89,6 +89,18 @@ export class SmppServer extends EventEmitter<ServerEvents> {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/** The same guard for a listener that rejects rather than throws; captureRejections routes here. */
|
||||||
|
override [EventEmitter.captureRejectionSymbol](
|
||||||
|
error: Error,
|
||||||
|
...args: [event: keyof ServerEvents, ...rest: unknown[]]
|
||||||
|
): void {
|
||||||
|
const [event] = args;
|
||||||
|
|
||||||
|
this.log.error('server - a listener rejected', { event, message: error.message });
|
||||||
|
|
||||||
|
if (event !== 'serverError') this.emit('serverError', error);
|
||||||
|
}
|
||||||
|
|
||||||
/** Stops listening and closes every live session. */
|
/** Stops listening and closes every live session. */
|
||||||
close(): Promise<void> {
|
close(): Promise<void> {
|
||||||
return new Promise(resolve => {
|
return new Promise(resolve => {
|
||||||
|
|||||||
+13
-1
@@ -76,8 +76,20 @@ export class Session extends EventEmitter<SessionEvents> {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/** The same guard for a listener that rejects rather than throws; captureRejections routes here. */
|
||||||
|
override [EventEmitter.captureRejectionSymbol](
|
||||||
|
error: Error,
|
||||||
|
...args: [event: keyof SessionEvents, ...rest: unknown[]]
|
||||||
|
): void {
|
||||||
|
const [event] = args;
|
||||||
|
|
||||||
|
this.log.error('session - a listener rejected', { event, message: error.message });
|
||||||
|
|
||||||
|
if (event !== 'sessionError') this.emit('sessionError', error);
|
||||||
|
}
|
||||||
|
|
||||||
constructor(options: SessionOptions) {
|
constructor(options: SessionOptions) {
|
||||||
super();
|
super({ captureRejections: true });
|
||||||
|
|
||||||
this.log = options.log ?? silentLog;
|
this.log = options.log ?? silentLog;
|
||||||
this.options = options;
|
this.options = options;
|
||||||
|
|||||||
+72
-1
@@ -1221,7 +1221,7 @@ describe('robustness', () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe('application hooks that throw', () => {
|
describe('application hooks that throw or reject', () => {
|
||||||
test('turns a throwing authenticate into a session error', async () => {
|
test('turns a throwing authenticate into a session error', async () => {
|
||||||
const smpp = await startServer({
|
const smpp = await startServer({
|
||||||
authenticate: () => { throw new Error('authenticate exploded'); },
|
authenticate: () => { throw new Error('authenticate exploded'); },
|
||||||
@@ -1291,6 +1291,77 @@ describe('application hooks that throw', () => {
|
|||||||
await smpp.close();
|
await smpp.close();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test('turns a rejecting async sms listener into a session error', async () => {
|
||||||
|
const smpp = await startServer();
|
||||||
|
const failed = once<Error>(resolve => {
|
||||||
|
smpp.on('session', session => {
|
||||||
|
session.on('sessionError', resolve);
|
||||||
|
session.on('sms', async sms => {
|
||||||
|
await sms.sendResp();
|
||||||
|
|
||||||
|
throw new Error('listener rejected');
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
|
const { session } = await connect(smpp, { responseTimeout: 200 });
|
||||||
|
|
||||||
|
assert.ok(session);
|
||||||
|
|
||||||
|
const sent = await session.sendSms({
|
||||||
|
from: '46701113311',
|
||||||
|
message: 'rejects after answering',
|
||||||
|
to: '46709771337',
|
||||||
|
});
|
||||||
|
const reported = await raceWithin(500, failed);
|
||||||
|
|
||||||
|
assert.equal(sent.err, undefined);
|
||||||
|
assert.ok(reported instanceof Error, 'a rejecting sms listener should reach the session');
|
||||||
|
assert.equal(reported.message, 'listener rejected');
|
||||||
|
|
||||||
|
session.close();
|
||||||
|
await smpp.close();
|
||||||
|
});
|
||||||
|
|
||||||
|
test('survives a sessionError listener that rejects as well', async () => {
|
||||||
|
const smpp = await startServer();
|
||||||
|
|
||||||
|
smpp.on('session', session => {
|
||||||
|
session.on('sessionError', () => Promise.reject(new Error('the reporter rejected too')));
|
||||||
|
session.on('sms', () => Promise.reject(new Error('listener rejected')));
|
||||||
|
});
|
||||||
|
|
||||||
|
const { session } = await connect(smpp, { responseTimeout: 200 });
|
||||||
|
|
||||||
|
assert.ok(session);
|
||||||
|
|
||||||
|
const sent = await session.sendSms({
|
||||||
|
from: '46701113311',
|
||||||
|
message: 'rejects in both listeners',
|
||||||
|
to: '46709771337',
|
||||||
|
});
|
||||||
|
|
||||||
|
assert.ok(sent.err instanceof Error);
|
||||||
|
|
||||||
|
session.close();
|
||||||
|
await smpp.close();
|
||||||
|
});
|
||||||
|
|
||||||
|
test('turns a rejecting session listener into a server error', async () => {
|
||||||
|
const smpp = await startServer();
|
||||||
|
const failed = once<Error>(resolve => { smpp.on('serverError', resolve); });
|
||||||
|
|
||||||
|
smpp.on('session', () => Promise.reject(new Error('session listener rejected')));
|
||||||
|
|
||||||
|
const { session } = await connect(smpp);
|
||||||
|
const reported = await raceWithin(500, failed);
|
||||||
|
|
||||||
|
assert.ok(reported instanceof Error, 'a rejecting session listener should reach the server');
|
||||||
|
assert.equal(reported.message, 'session listener rejected');
|
||||||
|
|
||||||
|
session?.close();
|
||||||
|
await smpp.close();
|
||||||
|
});
|
||||||
|
|
||||||
test('closes even when an application close listener throws', async () => {
|
test('closes even when an application close listener throws', async () => {
|
||||||
const smpp = await startServer();
|
const smpp = await startServer();
|
||||||
|
|
||||||
|
|||||||
@@ -5,7 +5,7 @@ rules there constrain every item below.
|
|||||||
|
|
||||||
## Status
|
## Status
|
||||||
|
|
||||||
The rewrite is **feature complete and green**: 231 tests, lint and typecheck clean, verified on Node
|
The rewrite is **feature complete and green**: 234 tests, lint and typecheck clean, verified on Node
|
||||||
18, 20, 22 and 24. What is left is release work and a few things worth adding before or after 1.0.0.
|
18, 20, 22 and 24. What is left is release work and a few things worth adding before or after 1.0.0.
|
||||||
|
|
||||||
```bash
|
```bash
|
||||||
@@ -57,6 +57,7 @@ Rules the API follows:
|
|||||||
| Merged multipart DLRs, reconnect, reassembly bounds, per-send abort, the segment cap | `test/session-extras.test.ts` |
|
| Merged multipart DLRs, reconnect, reassembly bounds, per-send abort, the segment cap | `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` |
|
||||||
|
| A listener that throws, or rejects, reaching `sessionError`/`serverError` rather than the process | `test/session.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/` |
|
||||||
|
|
||||||
@@ -110,15 +111,6 @@ session message is a change to every call site.
|
|||||||
|
|
||||||
## Worth doing, not blocking
|
## Worth doing, not blocking
|
||||||
|
|
||||||
- [ ] **An `async` event listener that rejects escapes the guard.** `Session.emit()` wraps
|
|
||||||
`super.emit()` in try/catch, which catches a listener that throws synchronously but not one
|
|
||||||
that returns a rejected promise — that surfaces as an unhandled rejection and takes the
|
|
||||||
process down, which hard rule 1 says must not happen. Every README example uses
|
|
||||||
`session.on('sms', async sms => …)`, so the shape is the one applications will write. The
|
|
||||||
library's own calls inside such a listener never reject, so the examples themselves are safe.
|
|
||||||
Fixing it means dispatching `rawListeners()` by hand in `emit()` and routing a rejection to
|
|
||||||
`sessionError` — a change to the hottest path, so it needs a decision before 1.0.0.
|
|
||||||
|
|
||||||
- [ ] **In-flight sends across a reconnect.** They currently fail with "Session closed before a
|
- [ ] **In-flight sends across a reconnect.** They currently fail with "Session closed before a
|
||||||
response arrived" and the caller retries. Re-queueing them automatically would be friendlier
|
response arrived" and the caller retries. Re-queueing them automatically would be friendlier
|
||||||
but risks duplicate delivery, so it needs a decision before it is built.
|
but risks duplicate delivery, so it needs a decision before it is built.
|
||||||
|
|||||||
Reference in New Issue
Block a user