From 75e8275c0629198555b52aa045a01ec49d30fd6b Mon Sep 17 00:00:00 2001 From: FaberVitale Date: Sat, 12 Feb 2022 23:29:30 +0100 Subject: [PATCH] feat(alm): add meaningful cancellation reasons to TaskAbortError and listenerApi.signal listenerApi.signal.reason can be one of - 'listener-cancelled' - 'listener-completed' forkApi.signal.reason can be one of - 'listener-cancelled' - 'listener-completed' - 'task-cancelled' - 'task-completed' BREAKING CHANGE: renamed TaskAbortError.reason -> TaskAbortError.code --- - Build log ```bash $ yarn workspace @rtk-incubator/action-listener-middleware run build Build "actionListenerMiddleware" to dist/esm: 1739 B: index.modern.js.gz 1566 B: index.modern.js.br Build "actionListenerMiddleware" to dist/module: 2.41 kB: index.js.gz 2.15 kB: index.js.br Build "actionListenerMiddleware" to dist/cjs: 2.4 kB: index.js.gz 2.15 kB: index.js.br ``` - Test log ```bash $ yarn workspace @rtk-incubator/action-listener-middleware run test --coverage PASS src/tests/listenerMiddleware.test.ts (5.738 s) PASS src/tests/effectScenarios.test.ts PASS src/tests/fork.test.ts PASS src/tests/useCases.test.ts ---------------|---------|----------|---------|---------|------------------- File | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s ---------------|---------|----------|---------|---------|------------------- All files | 97.72 | 91.38 | 94.64 | 97.55 | exceptions.ts | 100 | 0 | 100 | 100 | 17 index.ts | 97.4 | 97.5 | 92.5 | 97.32 | 190,214,252-253 task.ts | 97.06 | 80 | 100 | 96.3 | 30 utils.ts | 100 | 85.71 | 100 | 100 | 52 ---------------|---------|----------|---------|---------|------------------- Test Suites: 4 passed, 4 total Tests: 72 passed, 72 total Snapshots: 0 total Time: 6.796 s Ran all test suites. ``` --- .../src/exceptions.ts | 19 +++++-- .../action-listener-middleware/src/index.ts | 49 ++++++++++++------- .../action-listener-middleware/src/task.ts | 13 +++-- .../src/tests/fork.test.ts | 25 +++++++--- .../src/tests/listenerMiddleware.test.ts | 37 +++++++++++++- .../action-listener-middleware/src/types.ts | 6 +++ .../action-listener-middleware/src/utils.ts | 40 +++++++++++++++ 7 files changed, 152 insertions(+), 37 deletions(-) diff --git a/packages/action-listener-middleware/src/exceptions.ts b/packages/action-listener-middleware/src/exceptions.ts index 0e6bdf095a..8374085619 100644 --- a/packages/action-listener-middleware/src/exceptions.ts +++ b/packages/action-listener-middleware/src/exceptions.ts @@ -1,7 +1,20 @@ -export class TaskAbortError implements Error { +import type { SerializedError } from '@reduxjs/toolkit' + +const task = 'task' +const listener = 'listener' +const completed = 'completed' +const cancelled = 'cancelled' + +/* TaskAbortError error codes */ +export const taskCancelled = `${task}-${cancelled}` as const +export const taskCompleted = `${task}-${completed}` as const +export const listenerCancelled = `${listener}-${cancelled}` as const +export const listenerCompleted = `${listener}-${completed}` as const + +export class TaskAbortError implements SerializedError { name = 'TaskAbortError' message = '' - constructor(public reason = 'unknown') { - this.message = `task cancelled (reason: ${reason})` + constructor(public code = 'unknown') { + this.message = `task cancelled (reason: ${code})` } } diff --git a/packages/action-listener-middleware/src/index.ts b/packages/action-listener-middleware/src/index.ts index 0a1c56c943..24a5af3ee4 100644 --- a/packages/action-listener-middleware/src/index.ts +++ b/packages/action-listener-middleware/src/index.ts @@ -29,8 +29,18 @@ import type { TypedRemoveListener, TypedStopListening, } from './types' -import { assertFunction, catchRejection } from './utils' -import { TaskAbortError } from './exceptions' +import { + abortControllerWithReason, + assertFunction, + catchRejection, +} from './utils' +import { + listenerCancelled, + listenerCompleted, + TaskAbortError, + taskCancelled, + taskCompleted, +} from './exceptions' import { runTask, promisifyAbortSignal, @@ -75,21 +85,24 @@ const createFork = (parentAbortSignal: AbortSignal) => { assertFunction(taskExecutor, 'taskExecutor') const childAbortController = new AbortController() const cancel = () => { - childAbortController.abort() + abortControllerWithReason(childAbortController, taskCancelled) } - const result = runTask(async (): Promise => { - validateActive(parentAbortSignal) - validateActive(childAbortController.signal) - const result = (await taskExecutor({ - pause: createPause(childAbortController.signal), - delay: createDelay(childAbortController.signal), - signal: childAbortController.signal, - })) as T - validateActive(parentAbortSignal) - validateActive(childAbortController.signal) - return result - }, cancel) + const result = runTask( + async (): Promise => { + validateActive(parentAbortSignal) + validateActive(childAbortController.signal) + const result = (await taskExecutor({ + pause: createPause(childAbortController.signal), + delay: createDelay(childAbortController.signal), + signal: childAbortController.signal, + })) as T + validateActive(parentAbortSignal) + validateActive(childAbortController.signal) + return result + }, + () => abortControllerWithReason(childAbortController, taskCompleted) + ) return { result, @@ -211,7 +224,7 @@ const createClearListenerMiddleware = ( return () => { listenerMap.forEach((entry) => { entry.pending.forEach((controller) => { - controller.abort() + abortControllerWithReason(controller, listenerCancelled) }) }) @@ -363,7 +376,7 @@ export function createListenerMiddleware< cancelActiveListeners: () => { entry.pending.forEach((controller, _, set) => { if (controller !== internalTaskController) { - controller.abort() + abortControllerWithReason(controller, listenerCancelled) set.delete(controller) } }) @@ -378,7 +391,7 @@ export function createListenerMiddleware< }) } } finally { - internalTaskController.abort() // Notify that the task has completed + abortControllerWithReason(internalTaskController, listenerCompleted) // Notify that the task has completed entry.pending.delete(internalTaskController) } } diff --git a/packages/action-listener-middleware/src/task.ts b/packages/action-listener-middleware/src/task.ts index dbf92a0d5b..8644223b55 100644 --- a/packages/action-listener-middleware/src/task.ts +++ b/packages/action-listener-middleware/src/task.ts @@ -1,6 +1,6 @@ import { TaskAbortError } from './exceptions' -import type { TaskResult } from './types' -import { noop, catchRejection } from './utils' +import type { AbortSignalWithReason, TaskResult } from './types' +import { catchRejection } from './utils' /** * Synchronously raises {@link TaskAbortError} if the task tied to the input `signal` has been cancelled. @@ -8,9 +8,9 @@ import { noop, catchRejection } from './utils' * @param reason * @see {TaskAbortError} */ -export const validateActive = (signal: AbortSignal, reason?: string): void => { +export const validateActive = (signal: AbortSignal): void => { if (signal.aborted) { - throw new TaskAbortError(reason) + throw new TaskAbortError((signal as AbortSignalWithReason).reason) } } @@ -20,12 +20,11 @@ export const validateActive = (signal: AbortSignal, reason?: string): void => { * @returns */ export const promisifyAbortSignal = ( - signal: AbortSignal, - reason?: string + signal: AbortSignalWithReason ): Promise => { return catchRejection( new Promise((_, reject) => { - const notifyRejection = () => reject(new TaskAbortError(reason)) + const notifyRejection = () => reject(new TaskAbortError(signal.reason)) if (signal.aborted) { notifyRejection() diff --git a/packages/action-listener-middleware/src/tests/fork.test.ts b/packages/action-listener-middleware/src/tests/fork.test.ts index fa6cae9ea4..813d4e7827 100644 --- a/packages/action-listener-middleware/src/tests/fork.test.ts +++ b/packages/action-listener-middleware/src/tests/fork.test.ts @@ -4,6 +4,7 @@ import { configureStore, createSlice } from '@reduxjs/toolkit' import type { PayloadAction } from '@reduxjs/toolkit' import type { ForkedTaskExecutor, TaskResult } from '../types' import { createListenerMiddleware, TaskAbortError } from '../index' +import { listenerCancelled, taskCancelled } from '../exceptions' function delay(ms: number) { return new Promise((resolve) => setTimeout(resolve, ms)) @@ -122,7 +123,9 @@ describe('fork', () => { store.dispatch(increment()) store.dispatch(increment()) - expect(await deferredForkedTaskError).toEqual(new TaskAbortError()) + expect(await deferredForkedTaskError).toEqual( + new TaskAbortError(listenerCancelled) + ) }) it('synchronously throws TypeError error if the provided executor is not a function', () => { @@ -193,7 +196,10 @@ describe('fork', () => { desc: 'sync exec - sync cancel', executor: () => 42, cancelAfterMs: -1, - expected: { status: 'cancelled', error: new TaskAbortError() }, + expected: { + status: 'cancelled', + error: new TaskAbortError(taskCancelled), + }, }, { desc: 'sync exec - async cancel', @@ -208,7 +214,10 @@ describe('fork', () => { throw new Error('2020') }, cancelAfterMs: 10, - expected: { status: 'cancelled', error: new TaskAbortError() }, + expected: { + status: 'cancelled', + error: new TaskAbortError(taskCancelled), + }, }, { desc: 'async exec - success', @@ -300,7 +309,7 @@ describe('fork', () => { expect(await deferredResult).toEqual({ status: 'cancelled', - error: new TaskAbortError(), + error: new TaskAbortError(taskCancelled), }) }) @@ -357,12 +366,12 @@ describe('fork', () => { actionCreator: increment, effect: async (_, listenerApi) => { const forkedTask = listenerApi.fork(async (forkApi) => { - await forkApi.pause(delay(30)) + await forkApi.pause(delay(1_000)) return 4 }) - await listenerApi.delay(10) + await Promise.resolve() forkedTask.cancel() deferredResult.resolve(await forkedTask.result) }, @@ -372,7 +381,7 @@ describe('fork', () => { expect(await deferredResult).toEqual({ status: 'cancelled', - error: new TaskAbortError(), + error: new TaskAbortError(taskCancelled), }) }) @@ -396,7 +405,7 @@ describe('fork', () => { expect(await deferredResult).toEqual({ status: 'cancelled', - error: new TaskAbortError(), + error: new TaskAbortError(listenerCancelled), }) }) }) diff --git a/packages/action-listener-middleware/src/tests/listenerMiddleware.test.ts b/packages/action-listener-middleware/src/tests/listenerMiddleware.test.ts index 11af019fac..97d545b084 100644 --- a/packages/action-listener-middleware/src/tests/listenerMiddleware.test.ts +++ b/packages/action-listener-middleware/src/tests/listenerMiddleware.test.ts @@ -25,7 +25,12 @@ import type { Unsubscribe, ListenerMiddleware, } from '../index' -import type { AddListenerOverloads, TypedRemoveListener } from '../types' +import type { + AbortSignalWithReason, + AddListenerOverloads, + TypedRemoveListener, +} from '../types' +import { listenerCancelled, listenerCompleted } from '../exceptions' const middlewareApi = { getState: expect.any(Function), @@ -537,6 +542,36 @@ describe('createListenerMiddleware', () => { } ) + test('listenerApi.signal has correct reason when listener is cancelled or completes', async () => { + const notifyDeferred = createAction>('notify-deferred') + + startListening({ + actionCreator: notifyDeferred, + async effect({ payload }, { signal, cancelActiveListeners, delay }) { + signal.addEventListener( + 'abort', + () => { + payload.resolve((signal as AbortSignalWithReason).reason) + }, + { once: true } + ) + + cancelActiveListeners() + delay(10) + }, + }) + + const deferredCancelledSignalReason = store.dispatch( + notifyDeferred(deferred()) + ).payload + const deferredCompletedSignalReason = store.dispatch( + notifyDeferred(deferred()) + ).payload + + expect(await deferredCancelledSignalReason).toBe(listenerCancelled) + expect(await deferredCompletedSignalReason).toBe(listenerCompleted) + }) + test('"can unsubscribe via middleware api', () => { const effect = jest.fn( (action: TestAction1, api: ListenerEffectAPI) => { diff --git a/packages/action-listener-middleware/src/types.ts b/packages/action-listener-middleware/src/types.ts index 19d343a961..312bfe197d 100644 --- a/packages/action-listener-middleware/src/types.ts +++ b/packages/action-listener-middleware/src/types.ts @@ -9,6 +9,12 @@ import type { } from '@reduxjs/toolkit' import type { TaskAbortError } from './exceptions' +/** + * @internal + * At the time of writing `lib.dom.ts` does not provide `abortSignal.reason`. + */ +export type AbortSignalWithReason = AbortSignal & { reason?: T } + /** * Types copied from RTK */ diff --git a/packages/action-listener-middleware/src/utils.ts b/packages/action-listener-middleware/src/utils.ts index cc8044fecc..0d12eb8e90 100644 --- a/packages/action-listener-middleware/src/utils.ts +++ b/packages/action-listener-middleware/src/utils.ts @@ -1,3 +1,5 @@ +import type { AbortSignalWithReason } from './types' + export const assertFunction: ( func: unknown, expected: string @@ -20,3 +22,41 @@ export const catchRejection = ( return promise } + +/** + * Calls `abortController.abort(reason)` and patches `signal.reason`. + * if it is not supported. + * + * At the time of writing `signal.reason` is available in FF chrome, edge node 17 and deno. + * @param abortController + * @param reason + * @returns + * @see https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal/reason + */ +export const abortControllerWithReason = ( + abortController: AbortController, + reason: T +): void => { + type Consumer = (val: T) => void + + const signal = abortController.signal as AbortSignalWithReason + + if (signal.aborted) { + return + } + + // Patch `reason` if necessary. + // - We use defineProperty here because reason is a getter of `AbortSignal.__proto__`. + // - We need to patch 'reason' before calling `.abort()` because listeners to the 'abort' + // event are are notified immediately. + if (!('reason' in signal)) { + Object.defineProperty(signal, 'reason', { + enumerable: true, + value: reason, + configurable: true, + writable: true, + }) + } + + ;(abortController.abort as Consumer)(reason) +}