Accept an async listener in the types and normalise any rejection reason
This commit is contained in:
@@ -42,6 +42,7 @@ src/
|
||||
sms.ts The live handle emitted as the 'sms' event (sendResp/sendDlr)
|
||||
dlr.ts Delivery receipts: text and TLV parsing, receipt status codes
|
||||
dlr-merger.ts DlrMerger: per-segment receipts counted into one MessageDlr
|
||||
error-from.ts errorFrom(): whatever was thrown or rejected, as an Error
|
||||
expiring-groups.ts ExpiringGroups: the capped, expiring store both of those share
|
||||
incoming-requests.ts Every request the peer sends: messages, receipts, links, unknown commands
|
||||
link-timers.ts LinkTimers: the enquire_link heartbeat and the idle timeout
|
||||
@@ -218,9 +219,18 @@ exactly 140.
|
||||
`[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. A rejection reason is `unknown`, so the handler normalises it the way the synchronous
|
||||
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.
|
||||
forbids. A rejection reason is `unknown` and `String()` throws on a null-prototype object, so both
|
||||
handlers normalise through `errorFrom()` rather than inline — a route out of the handler would land
|
||||
on a bare `process.nextTick` with nothing to catch it.
|
||||
|
||||
- **Both emitters re-declare their listener methods to accept a promise.** Maintainer's call,
|
||||
2026-08-27: `EventEmitter` types every listener as void-returning, so the
|
||||
`session.on('sms', async sms => …)` the README documents reads as a misused promise in any strict
|
||||
consumer. `declare on: …` and its six siblings re-type the inherited methods to return `unknown`,
|
||||
which emits nothing, needs no cast and leaves the runtime method on the prototype. Overriding them
|
||||
as real methods instead cannot work: the `super.on()` call needs a cast to satisfy the conditional
|
||||
`Listener` type. `unknown` rather than `void | Promise<void>` because a listener may return
|
||||
anything — `session.on('close', () => set.delete(session))` returns a boolean.
|
||||
|
||||
- **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
|
||||
|
||||
@@ -48,13 +48,6 @@ export default tseslint.config(
|
||||
files: ['src/defs/encodings.ts'],
|
||||
rules: { 'no-control-regex': 'off' },
|
||||
},
|
||||
{
|
||||
// An async event listener is a documented, supported shape; EventEmitter types listeners void.
|
||||
files: ['test/*.test.ts'],
|
||||
rules: {
|
||||
'@typescript-eslint/no-misused-promises': ['error', { checksVoidReturn: { arguments: false } }],
|
||||
},
|
||||
},
|
||||
{
|
||||
files: ['eslint.config.js'],
|
||||
extends: [tseslint.configs.disableTypeChecked],
|
||||
|
||||
@@ -0,0 +1,10 @@
|
||||
/** Whatever was thrown or rejected, as an Error. `String()` throws on some values; this cannot. */
|
||||
export function errorFrom(reason: unknown): Error {
|
||||
if (reason instanceof Error) return reason;
|
||||
|
||||
try {
|
||||
return new Error(String(reason));
|
||||
} catch {
|
||||
return new Error('A thrown value that cannot be converted to a string');
|
||||
}
|
||||
}
|
||||
+16
-4
@@ -9,6 +9,7 @@ import { bindTypeFromCommand, checkSessionOptions, undeclaredInterfaceVersion }
|
||||
import { createServer as createNetServer } from 'node:net';
|
||||
import { createServer as createTlsServer } from 'node:tls';
|
||||
import { defaultInterfaceVersion } from './defs/constants.ts';
|
||||
import { errorFrom } from './error-from.ts';
|
||||
import { paramText } from './defs/types.ts';
|
||||
import { silentLog } from './log.ts';
|
||||
|
||||
@@ -50,8 +51,19 @@ const defaults = {
|
||||
systemId: defaultSystemId,
|
||||
};
|
||||
|
||||
/** A listener may return a promise: an `async` one that rejects is routed like one that throws. */
|
||||
type ServerListener<K extends keyof ServerEvents> = (...args: ServerEvents[K]) => unknown;
|
||||
|
||||
/** A listening SMPP server. Sessions arrive as `session` events; `close()` stops listening. */
|
||||
export class SmppServer extends EventEmitter<ServerEvents> {
|
||||
declare addListener: <K extends keyof ServerEvents>(event: K, listener: ServerListener<K>) => this;
|
||||
declare off: <K extends keyof ServerEvents>(event: K, listener: ServerListener<K>) => this;
|
||||
declare on: <K extends keyof ServerEvents>(event: K, listener: ServerListener<K>) => this;
|
||||
declare once: <K extends keyof ServerEvents>(event: K, listener: ServerListener<K>) => this;
|
||||
declare prependListener: <K extends keyof ServerEvents>(event: K, listener: ServerListener<K>) => this;
|
||||
declare prependOnceListener: <K extends keyof ServerEvents>(event: K, listener: ServerListener<K>) => this;
|
||||
declare removeListener: <K extends keyof ServerEvents>(event: K, listener: ServerListener<K>) => this;
|
||||
|
||||
readonly sessions = new Set<Session>();
|
||||
|
||||
private readonly log: SmppLog;
|
||||
@@ -78,7 +90,7 @@ export class SmppServer extends EventEmitter<ServerEvents> {
|
||||
try {
|
||||
return super.emit(event, ...args);
|
||||
} catch (thrown: unknown) {
|
||||
const err = thrown instanceof Error ? thrown : new Error(String(thrown));
|
||||
const err = errorFrom(thrown);
|
||||
|
||||
this.log.error('server - a listener threw', { event, message: err.message });
|
||||
|
||||
@@ -95,7 +107,7 @@ export class SmppServer extends EventEmitter<ServerEvents> {
|
||||
...args: [event: keyof ServerEvents, ...rest: unknown[]]
|
||||
): void {
|
||||
const [event] = args;
|
||||
const error = reason instanceof Error ? reason : new Error(String(reason));
|
||||
const error = errorFrom(reason);
|
||||
|
||||
this.log.error('server - a listener rejected', { event, message: error.message });
|
||||
|
||||
@@ -109,7 +121,7 @@ export class SmppServer extends EventEmitter<ServerEvents> {
|
||||
try {
|
||||
session.close();
|
||||
} catch (thrown: unknown) {
|
||||
this.emit('serverError', thrown instanceof Error ? thrown : new Error(String(thrown)));
|
||||
this.emit('serverError', errorFrom(thrown));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -332,7 +344,7 @@ export function server(options: ServerOptions = {}): Promise<Result<{ server: Sm
|
||||
resolve({ server: smpp });
|
||||
});
|
||||
} catch (thrown: unknown) {
|
||||
onStartupError(thrown instanceof Error ? thrown : new Error(String(thrown)));
|
||||
onStartupError(errorFrom(thrown));
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
+15
-3
@@ -15,6 +15,7 @@ import { PduFramer } from './pdu-framer.ts';
|
||||
import { PendingRequests } from './pending-requests.ts';
|
||||
import { ReconnectLoop } from './reconnect-loop.ts';
|
||||
import { SendWindow } from './send-window.ts';
|
||||
import { errorFrom } from './error-from.ts';
|
||||
import { optionalParamsMinVersion } from './defs/constants.ts';
|
||||
import { bindCarries, bindCommands, defaultSystemId, defaults } from './session-options.ts';
|
||||
import { isResp, objToPdu, pduReturn, pduToObj } from './pdu.ts';
|
||||
@@ -33,7 +34,18 @@ export type {
|
||||
export type { BindType };
|
||||
export { bindCommands, defaultSystemId };
|
||||
|
||||
/** A listener may return a promise: an `async` one that rejects is routed like one that throws. */
|
||||
type SessionListener<K extends keyof SessionEvents> = (...args: SessionEvents[K]) => unknown;
|
||||
|
||||
export class Session extends EventEmitter<SessionEvents> {
|
||||
declare addListener: <K extends keyof SessionEvents>(event: K, listener: SessionListener<K>) => this;
|
||||
declare off: <K extends keyof SessionEvents>(event: K, listener: SessionListener<K>) => this;
|
||||
declare on: <K extends keyof SessionEvents>(event: K, listener: SessionListener<K>) => this;
|
||||
declare once: <K extends keyof SessionEvents>(event: K, listener: SessionListener<K>) => this;
|
||||
declare prependListener: <K extends keyof SessionEvents>(event: K, listener: SessionListener<K>) => this;
|
||||
declare prependOnceListener: <K extends keyof SessionEvents>(event: K, listener: SessionListener<K>) => this;
|
||||
declare removeListener: <K extends keyof SessionEvents>(event: K, listener: SessionListener<K>) => this;
|
||||
|
||||
/** Replaced on reconnect, so hold the session rather than this. */
|
||||
sock: Socket;
|
||||
readonly log: SmppLog;
|
||||
@@ -65,7 +77,7 @@ export class Session extends EventEmitter<SessionEvents> {
|
||||
try {
|
||||
return super.emit(event, ...args);
|
||||
} catch (thrown: unknown) {
|
||||
const err = thrown instanceof Error ? thrown : new Error(String(thrown));
|
||||
const err = errorFrom(thrown);
|
||||
|
||||
this.log.error('session - a listener threw', { event, message: err.message });
|
||||
|
||||
@@ -82,7 +94,7 @@ export class Session extends EventEmitter<SessionEvents> {
|
||||
...args: [event: keyof SessionEvents, ...rest: unknown[]]
|
||||
): void {
|
||||
const [event] = args;
|
||||
const error = reason instanceof Error ? reason : new Error(String(reason));
|
||||
const error = errorFrom(reason);
|
||||
|
||||
this.log.error('session - a listener rejected', { event, message: error.message });
|
||||
|
||||
@@ -379,7 +391,7 @@ export class Session extends EventEmitter<SessionEvents> {
|
||||
this.emit('incomingPduObj', pduObj);
|
||||
// Every application hook and listener reached from an incoming PDU funnels through here.
|
||||
void this.incoming.handle(pduObj).catch((thrown: unknown) => {
|
||||
const err = thrown instanceof Error ? thrown : new Error(String(thrown));
|
||||
const err = errorFrom(thrown);
|
||||
|
||||
this.log.error('session - a handler threw', { message: err.message });
|
||||
this.emit('sessionError', err);
|
||||
|
||||
@@ -0,0 +1,17 @@
|
||||
import assert from 'node:assert/strict';
|
||||
import test from 'node:test';
|
||||
import { errorFrom } from '../src/error-from.ts';
|
||||
|
||||
test('carries an Error through and describes anything else, including what String() refuses', () => {
|
||||
const original = new Error('the original');
|
||||
const undescribable: unknown = Object.create(null);
|
||||
|
||||
assert.equal(errorFrom(original), original);
|
||||
assert.equal(errorFrom('a string').message, 'a string');
|
||||
assert.equal(errorFrom(null).message, 'null');
|
||||
assert.equal(errorFrom(undefined).message, 'undefined');
|
||||
assert.equal(
|
||||
errorFrom(undescribable).message,
|
||||
'A thrown value that cannot be converted to a string',
|
||||
);
|
||||
});
|
||||
@@ -5,7 +5,7 @@ rules there constrain every item below.
|
||||
|
||||
## Status
|
||||
|
||||
The rewrite is **feature complete and green**: 234 tests, lint and typecheck clean, verified on Node
|
||||
The rewrite is **feature complete and green**: 235 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.
|
||||
|
||||
```bash
|
||||
@@ -57,7 +57,7 @@ Rules the API follows:
|
||||
| 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` |
|
||||
| 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` |
|
||||
| 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` |
|
||||
| CI on Node 18/20/22/24, Renovate, tag-triggered publish | `.github/workflows/` |
|
||||
|
||||
@@ -111,14 +111,6 @@ session message is a change to every call site.
|
||||
|
||||
## 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
|
||||
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.
|
||||
|
||||
Reference in New Issue
Block a user