Skip to content

Commit 906aa2b

Browse files
fix: don't swallow createAsyncThunk aborts that happen before pending (#5314)
When a thunk's `AbortController` was aborted before the `pending` action was dispatched - either because an already-aborted external `signal` was passed, or because `abort()` was called while an async `condition` was still pending - the resulting error was reported as a `ConditionError` with the message "Aborted due to condition callback returning false." Because `meta.condition` was then `true`, the `rejected` action was skipped by default (unless `dispatchConditionRejection` was set), so the abort was silently swallowed and the error was mislabeled even though no `condition` callback ever returned `false`. This was inconsistent with aborting the same signal moments later (once the thunk is running), which correctly produces a `rejected` action with an `AbortError`, and it contradicted the documented "Canceling While Running" behavior. The condition-check now only throws a `ConditionError` when `condition` actually returns `false`. If the controller was aborted, it instead throws an `AbortError` carrying the real abort reason, so the abort is reported and dispatched consistently regardless of timing. Co-authored-by: Arya Emami <aryaemami59@yahoo.com>
1 parent 970895a commit 906aa2b

2 files changed

Lines changed: 71 additions & 7 deletions

File tree

packages/toolkit/src/createAsyncThunk.ts

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -636,13 +636,25 @@ export const createAsyncThunk = /* @__PURE__ */ (() => {
636636
conditionResult = await conditionResult
637637
}
638638

639-
if (conditionResult === false || abortController.signal.aborted) {
639+
if (conditionResult === false) {
640640
// eslint-disable-next-line no-throw-literal
641641
throw {
642642
name: 'ConditionError',
643643
message: 'Aborted due to condition callback returning false.',
644644
}
645645
}
646+
if (abortController.signal.aborted) {
647+
// The request was aborted (e.g. via an already-aborted external
648+
// signal, or `abort()` called during an async `condition`) before
649+
// the `pending` action was dispatched. Treat this as an abort
650+
// rather than a condition rejection, so the reason is preserved
651+
// and the `rejected` action is not silently swallowed.
652+
// eslint-disable-next-line no-throw-literal
653+
throw {
654+
name: 'AbortError',
655+
message: abortReason || 'Aborted',
656+
}
657+
}
646658

647659
const abortedPromise = new Promise<never>((_, reject) => {
648660
abortHandler = () => {

packages/toolkit/src/tests/createAsyncThunk.test.ts

Lines changed: 58 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -620,19 +620,30 @@ describe('conditional skipping of asyncThunks', () => {
620620
)
621621
})
622622

623-
test('async condition with AbortController signal first', async () => {
623+
test('aborting during an async condition dispatches a rejected action with an AbortError', async () => {
624624
const condition = async () => {
625625
await delay(25)
626626
return true
627627
}
628628
const asyncThunk = createAsyncThunk('test', payloadCreator, { condition })
629629

630+
let result: any
630631
try {
631632
const thunkPromise = asyncThunk(arg)(dispatch, getState, extra)
632-
thunkPromise.abort()
633-
await thunkPromise
633+
thunkPromise.abort('TEST_ABORT')
634+
result = await thunkPromise
634635
} catch (err) {}
635-
expect(dispatch).not.toHaveBeenCalled()
636+
// The payload creator should never run, since the abort happened
637+
// before the `condition` resolved.
638+
expect(payloadCreator).not.toHaveBeenCalled()
639+
// Aborting a running thunk should dispatch a `rejected` action with an
640+
// `AbortError`, not be silently swallowed as a condition rejection.
641+
expect(dispatch).toHaveBeenCalledOnce()
642+
expect(asyncThunk.rejected.match(result)).toBe(true)
643+
expect(result.error.name).toBe('AbortError')
644+
expect(result.error.message).toBe('TEST_ABORT')
645+
expect(result.meta.aborted).toBe(true)
646+
expect(result.meta.condition).toBe(false)
636647
})
637648

638649
test('rejected action is not dispatched by default', async () => {
@@ -1028,18 +1039,59 @@ describe('dispatch config', () => {
10281039
'External signal was aborted',
10291040
)
10301041
})
1042+
test('an already-aborted external signal is not silently swallowed when options are provided', async () => {
1043+
const dispatched: any[] = []
1044+
const dispatch = vi.fn((action: any) => {
1045+
dispatched.push(action)
1046+
return action
1047+
})
1048+
const payloadCreator = vi.fn(async () => 42)
1049+
// Providing any options object (here, a `condition`) used to cause the
1050+
// abort to be reported as a `ConditionError`, which is then skipped by
1051+
// default - so the abort was silently swallowed with no action dispatched.
1052+
const asyncThunk = createAsyncThunk('test', payloadCreator, {
1053+
condition: () => true,
1054+
})
1055+
1056+
const signal = AbortSignal.abort()
1057+
const result: any = await asyncThunk(undefined, { signal })(
1058+
dispatch,
1059+
() => ({}),
1060+
undefined,
1061+
)
1062+
1063+
expect(payloadCreator).not.toHaveBeenCalled()
1064+
expect(asyncThunk.rejected.match(result)).toBe(true)
1065+
expect(result.error.name).toBe('AbortError')
1066+
expect(result.error.message).toBe('External signal was aborted')
1067+
expect(result.meta.aborted).toBe(true)
1068+
expect(result.meta.condition).toBe(false)
1069+
// The `rejected` action must actually be dispatched, not swallowed.
1070+
expect(dispatched).toHaveLength(1)
1071+
expect(asyncThunk.rejected.match(dispatched[0])).toBe(true)
1072+
})
10311073
test('handles already aborted external signal', async () => {
1032-
const asyncThunk = createAsyncThunk('test', async (_: void, { signal }) => {
1074+
const payloadCreator = vi.fn(async (_: void, { signal }) => {
10331075
signal.throwIfAborted()
10341076
const { promise, reject } = promiseWithResolvers<never>()
10351077
signal.addEventListener('abort', () => reject(signal.reason))
10361078
return promise
10371079
})
1080+
const asyncThunk = createAsyncThunk('test', payloadCreator)
10381081

10391082
const signal = AbortSignal.abort()
10401083
const promise = store.dispatch(asyncThunk(undefined, { signal }))
1084+
// An already-aborted external signal should be treated the same as
1085+
// aborting the signal while the thunk is running: a `rejected` action
1086+
// with an `AbortError`, not a `ConditionError` that gets swallowed.
10411087
await expect(promise.unwrap()).rejects.toThrow(
1042-
'Aborted due to condition callback returning false.',
1088+
'External signal was aborted',
10431089
)
1090+
const result = await promise
1091+
expect(payloadCreator).not.toHaveBeenCalled()
1092+
expect(asyncThunk.rejected.match(result)).toBe(true)
1093+
expect((result as any).error.name).toBe('AbortError')
1094+
expect((result as any).meta.aborted).toBe(true)
1095+
expect((result as any).meta.condition).toBe(false)
10441096
})
10451097
})

0 commit comments

Comments
 (0)