Skip to content

Commit 672ce66

Browse files
committed
transport: replace manual signal linking with AbortSignal.any()
The external signal support added in 46744a4 manually managed an abort event listener and cleanup function to link req.signal to the internal controller. AbortSignal.any() does this in a single line and the platform handles lifetime — no listener, no cleanup, no cleanupExternalSignal field on RegisteredTurn. Removes two tests that asserted removeEventListener was called, since there is no manual listener to remove. The five behavioral tests (signal fires, already-aborted, start rejection, in-flight stream cancellation, Ably cancel independence) are unchanged.
1 parent 46744a4 commit 672ce66

File tree

2 files changed

+9
-52
lines changed

2 files changed

+9
-52
lines changed

src/core/transport/server-transport.ts

Lines changed: 9 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -50,10 +50,10 @@ interface RegisteredTurn {
5050
turnId: string;
5151
clientId: string;
5252
controller: AbortController;
53+
/** Composite signal that fires when either the internal controller or the external signal aborts. */
54+
signal: AbortSignal;
5355
onCancel?: (request: CancelRequest) => Promise<boolean>;
5456
onError?: (error: Ably.ErrorInfo) => void;
55-
/** Removes the listener from the external signal, if one was provided. */
56-
cleanupExternalSignal?: () => void;
5757
}
5858

5959
// ---------------------------------------------------------------------------
@@ -118,7 +118,6 @@ class DefaultServerTransport<TEvent, TMessage> implements ServerTransport<TEvent
118118
this._logger?.trace('DefaultServerTransport.close();');
119119
this._channel.unsubscribe(EVENT_CANCEL, this._channelListener);
120120
for (const reg of this._registeredTurns.values()) {
121-
reg.cleanupExternalSignal?.();
122121
reg.controller.abort();
123122
}
124123
this._registeredTurns.clear();
@@ -259,37 +258,19 @@ class DefaultServerTransport<TEvent, TMessage> implements ServerTransport<TEvent
259258
let started = false;
260259
let ended = false;
261260

262-
// Link external signal (e.g. req.signal) to the internal controller so
263-
// platform-level cancellation (client disconnect, function timeout) aborts
264-
// the turn through the same path as Ably cancel messages.
265-
let cleanupExternalSignal: (() => void) | undefined;
266-
if (externalSignal) {
267-
if (externalSignal.aborted) {
268-
this._logger?.debug('DefaultServerTransport._createTurn(); external signal already aborted', { turnId });
269-
controller.abort();
270-
} else {
271-
const listener = () => {
272-
this._logger?.debug('DefaultServerTransport._createTurn(); external signal fired, aborting turn', {
273-
turnId,
274-
});
275-
controller.abort();
276-
};
277-
externalSignal.addEventListener('abort', listener, { once: true });
278-
cleanupExternalSignal = () => {
279-
externalSignal.removeEventListener('abort', listener);
280-
};
281-
this._logger?.debug('DefaultServerTransport._createTurn(); linked external signal', { turnId });
282-
}
283-
}
261+
// Compose the internal controller signal with the external signal (e.g.
262+
// req.signal) so platform-level cancellation (client disconnect, function
263+
// timeout) aborts the turn through the same path as Ably cancel messages.
264+
const signal = externalSignal ? AbortSignal.any([controller.signal, externalSignal]) : controller.signal;
284265

285266
// Spec: AIT-ST3a — register immediately so early cancels can fire the abort signal.
286267
const registration: RegisteredTurn = {
287268
turnId,
288269
clientId: turnClientId ?? '',
289270
controller,
271+
signal,
290272
onCancel,
291273
onError: turnOnError,
292-
cleanupExternalSignal,
293274
};
294275
this._registeredTurns.set(turnId, registration);
295276

@@ -307,15 +288,15 @@ class DefaultServerTransport<TEvent, TMessage> implements ServerTransport<TEvent
307288
return turnId;
308289
},
309290
get abortSignal() {
310-
return controller.signal;
291+
return signal;
311292
},
312293

313294
// Spec: AIT-ST4, AIT-ST4a, AIT-ST4b
314295
start: async (): Promise<void> => {
315296
logger?.trace('Turn.start();', { turnId });
316297

317298
// Spec: AIT-ST4a
318-
if (controller.signal.aborted) {
299+
if (signal.aborted) {
319300
throw new Ably.ErrorInfo(
320301
`unable to start turn; turn ${turnId} was cancelled before start()`,
321302
ErrorCode.InvalidArgument,
@@ -443,7 +424,6 @@ class DefaultServerTransport<TEvent, TMessage> implements ServerTransport<TEvent
443424
}
444425
await attachPromise;
445426

446-
const signal = turnManager.getSignal(turnId);
447427
const turnOwnerClientId = turnManager.getClientId(turnId);
448428

449429
// Per-operation parent overrides the turn-level default.
@@ -496,7 +476,6 @@ class DefaultServerTransport<TEvent, TMessage> implements ServerTransport<TEvent
496476
throw errInfo;
497477
} finally {
498478
registeredTurns.delete(turnId);
499-
cleanupExternalSignal?.();
500479
}
501480

502481
logger?.debug('Turn.end(); turn ended', { turnId, reason });

test/core/transport/server-transport.test.ts

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -616,18 +616,6 @@ describe('ServerTransport', () => {
616616
expect(result.reason).toBe('cancelled');
617617
});
618618

619-
it('removes the external signal listener when the turn ends', async () => {
620-
const externalController = new AbortController();
621-
const signal = externalController.signal;
622-
const removeSpy = vi.spyOn(signal, 'removeEventListener');
623-
624-
const turn = transport.newTurn({ turnId: 'turn-1', signal });
625-
await turn.start();
626-
await turn.end('complete');
627-
628-
expect(removeSpy).toHaveBeenCalledWith('abort', expect.any(Function));
629-
});
630-
631619
it('does not interfere with Ably cancel routing', async () => {
632620
const externalController = new AbortController();
633621
const turn = transport.newTurn({ turnId: 'turn-1', clientId: 'user-a', signal: externalController.signal });
@@ -661,15 +649,5 @@ describe('ServerTransport', () => {
661649
expect(channel.unsubscribe).toHaveBeenCalledWith(EVENT_CANCEL, expect.any(Function));
662650
});
663651

664-
it('removes external signal listeners on close', () => {
665-
const externalController = new AbortController();
666-
const removeSpy = vi.spyOn(externalController.signal, 'removeEventListener');
667-
668-
transport.newTurn({ turnId: 'turn-1', signal: externalController.signal });
669-
670-
transport.close();
671-
672-
expect(removeSpy).toHaveBeenCalledWith('abort', expect.any(Function));
673-
});
674652
});
675653
});

0 commit comments

Comments
 (0)