Normalise a listener's rejection reason before routing it to sessionError
This commit is contained in:
@@ -218,8 +218,9 @@ exactly 140.
|
|||||||
`[EventEmitter.captureRejectionSymbol]`, which lands a rejected `async` listener on `sessionError`
|
`[EventEmitter.captureRejectionSymbol]`, which lands a rejected `async` listener on `sessionError`
|
||||||
or `serverError` beside the synchronous guard in `emit()`. Dispatching `rawListeners()` from
|
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
|
`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
|
forbids. A rejection reason is `unknown`, so the handler normalises it the way the synchronous
|
||||||
the same cast — which is why `test/*.test.ts` turns `no-misused-promises` off.
|
guard does. `Session.on()` still types its listeners as void-returning, because widening that needs
|
||||||
|
the same cast — which is why `test/*.test.ts` turns off `no-misused-promises` on arguments.
|
||||||
|
|
||||||
- **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
|
||||||
|
|||||||
+3
-1
@@ -51,7 +51,9 @@ export default tseslint.config(
|
|||||||
{
|
{
|
||||||
// An async event listener is a documented, supported shape; EventEmitter types listeners void.
|
// An async event listener is a documented, supported shape; EventEmitter types listeners void.
|
||||||
files: ['test/*.test.ts'],
|
files: ['test/*.test.ts'],
|
||||||
rules: { '@typescript-eslint/no-misused-promises': 'off' },
|
rules: {
|
||||||
|
'@typescript-eslint/no-misused-promises': ['error', { checksVoidReturn: { arguments: false } }],
|
||||||
|
},
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
files: ['eslint.config.js'],
|
files: ['eslint.config.js'],
|
||||||
|
|||||||
+2
-1
@@ -91,10 +91,11 @@ export class SmppServer extends EventEmitter<ServerEvents> {
|
|||||||
|
|
||||||
/** The same guard for a listener that rejects rather than throws; captureRejections routes here. */
|
/** The same guard for a listener that rejects rather than throws; captureRejections routes here. */
|
||||||
override [EventEmitter.captureRejectionSymbol](
|
override [EventEmitter.captureRejectionSymbol](
|
||||||
error: Error,
|
reason: unknown,
|
||||||
...args: [event: keyof ServerEvents, ...rest: unknown[]]
|
...args: [event: keyof ServerEvents, ...rest: unknown[]]
|
||||||
): void {
|
): void {
|
||||||
const [event] = args;
|
const [event] = args;
|
||||||
|
const error = reason instanceof Error ? reason : new Error(String(reason));
|
||||||
|
|
||||||
this.log.error('server - a listener rejected', { event, message: error.message });
|
this.log.error('server - a listener rejected', { event, message: error.message });
|
||||||
|
|
||||||
|
|||||||
+2
-1
@@ -78,10 +78,11 @@ export class Session extends EventEmitter<SessionEvents> {
|
|||||||
|
|
||||||
/** The same guard for a listener that rejects rather than throws; captureRejections routes here. */
|
/** The same guard for a listener that rejects rather than throws; captureRejections routes here. */
|
||||||
override [EventEmitter.captureRejectionSymbol](
|
override [EventEmitter.captureRejectionSymbol](
|
||||||
error: Error,
|
reason: unknown,
|
||||||
...args: [event: keyof SessionEvents, ...rest: unknown[]]
|
...args: [event: keyof SessionEvents, ...rest: unknown[]]
|
||||||
): void {
|
): void {
|
||||||
const [event] = args;
|
const [event] = args;
|
||||||
|
const error = reason instanceof Error ? reason : new Error(String(reason));
|
||||||
|
|
||||||
this.log.error('session - a listener rejected', { event, message: error.message });
|
this.log.error('session - a listener rejected', { event, message: error.message });
|
||||||
|
|
||||||
|
|||||||
@@ -1291,15 +1291,16 @@ describe('application hooks that throw or reject', () => {
|
|||||||
await smpp.close();
|
await smpp.close();
|
||||||
});
|
});
|
||||||
|
|
||||||
test('turns a rejecting async sms listener into a session error', async () => {
|
test('normalises whatever a rejecting async sms listener threw into a session error', async () => {
|
||||||
const smpp = await startServer();
|
const smpp = await startServer();
|
||||||
|
const reason: unknown = null;
|
||||||
const failed = once<Error>(resolve => {
|
const failed = once<Error>(resolve => {
|
||||||
smpp.on('session', session => {
|
smpp.on('session', session => {
|
||||||
session.on('sessionError', resolve);
|
session.on('sessionError', resolve);
|
||||||
session.on('sms', async sms => {
|
session.on('sms', async sms => {
|
||||||
await sms.sendResp();
|
await sms.sendResp();
|
||||||
|
|
||||||
throw new Error('listener rejected');
|
throw reason;
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
@@ -1316,7 +1317,7 @@ describe('application hooks that throw or reject', () => {
|
|||||||
|
|
||||||
assert.equal(sent.err, undefined);
|
assert.equal(sent.err, undefined);
|
||||||
assert.ok(reported instanceof Error, 'a rejecting sms listener should reach the session');
|
assert.ok(reported instanceof Error, 'a rejecting sms listener should reach the session');
|
||||||
assert.equal(reported.message, 'listener rejected');
|
assert.equal(reported.message, 'null');
|
||||||
|
|
||||||
session.close();
|
session.close();
|
||||||
await smpp.close();
|
await smpp.close();
|
||||||
|
|||||||
@@ -111,6 +111,14 @@ session message is a change to every call site.
|
|||||||
|
|
||||||
## Worth doing, not blocking
|
## Worth doing, not blocking
|
||||||
|
|
||||||
|
- [ ] **Should `on()` accept a promise-returning listener in its types?** A listener that rejects is
|
||||||
|
routed to `sessionError` now, but `EventEmitter` types every listener as void-returning, so the
|
||||||
|
`session.on('sms', async sms => …)` the README documents trips `no-misused-promises` in a
|
||||||
|
consumer's project exactly as it does in this repository's own tests. Widening the parameter
|
||||||
|
needs a cast, which hard rule 4 forbids; a declaration overload over an implementation
|
||||||
|
signature Node's types accept might not. Raised by review, 2026-08-27; worth deciding while
|
||||||
|
the surface is still movable.
|
||||||
|
|
||||||
- [ ] **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