Rework createAsyncThunk error handling behavior

- Removed `finished` action
- Serialized `Error` objects to a plain object
- Ensured errors in `fulfilled` dispatches won't get caught wrongly
- Changed to re-throw errors in case the user wants to handle them

Author: markerikson

src/createAsyncThunk.test.ts

@@ -1,4 +1,4 @@
-import { createAsyncThunk } from './createAsyncThunk'
+import { createAsyncThunk, miniSerializeError } from './createAsyncThunk'
 import { configureStore } from './configureStore'
 
 describe('createAsyncThunk', () => {
@@ -7,7 +7,6 @@ describe('createAsyncThunk', () => {
 
     expect(thunkActionCreator.fulfilled.type).toBe('testType/fulfilled')
     expect(thunkActionCreator.pending.type).toBe('testType/pending')
-    expect(thunkActionCreator.finished.type).toBe('testType/finished')
     expect(thunkActionCreator.rejected.type).toBe('testType/rejected')
   })
 
@@ -29,7 +28,7 @@ describe('createAsyncThunk', () => {
 
     await store.dispatch(thunkActionCreator())
 
-    expect(timesReducerCalled).toBe(3)
+    expect(timesReducerCalled).toBe(2)
   })
 
   it('accepts arguments and dispatches the actions on resolve', async () => {
@@ -65,11 +64,6 @@ describe('createAsyncThunk', () => {
       2,
       thunkActionCreator.fulfilled(result, generatedRequestId, args)
     )
-
-    expect(dispatch).toHaveBeenNthCalledWith(
-      3,
-      thunkActionCreator.finished(generatedRequestId, args)
-    )
   })
 
   it('accepts arguments and dispatches the actions on reject', async () => {
@@ -90,21 +84,23 @@ describe('createAsyncThunk', () => {
 
     const thunkFunction = thunkActionCreator(args)
 
-    await thunkFunction(dispatch, undefined, undefined)
+    try {
+      await thunkFunction(dispatch, undefined, undefined)
+    } catch (e) {}
 
     expect(dispatch).toHaveBeenNthCalledWith(
       1,
       thunkActionCreator.pending(generatedRequestId, args)
     )
 
-    expect(dispatch).toHaveBeenNthCalledWith(
-      2,
-      thunkActionCreator.rejected(error, generatedRequestId, args)
-    )
+    expect(dispatch).toHaveBeenCalledTimes(2)
 
-    expect(dispatch).toHaveBeenNthCalledWith(
-      3,
-      thunkActionCreator.finished(generatedRequestId, args)
-    )
+    console.log(dispatch.mock.calls)
+
+    // Have to check the bits of the action separately since the error was processed
+    const errorAction = dispatch.mock.calls[1][0]
+    expect(errorAction.error).toEqual(miniSerializeError(error))
+    expect(errorAction.meta.requestId).toBe(generatedRequestId)
+    expect(errorAction.meta.args).toBe(args)
   })
 })

src/createAsyncThunk.ts

@@ -9,6 +9,36 @@ type AsyncThunksArgs<S, E, D extends Dispatch = Dispatch> = {
   requestId: string
 }
 
+interface SimpleError {
+  name?: string
+  message?: string
+  stack?: string
+  code?: string
+}
+
+const commonProperties: (keyof SimpleError)[] = [
+  'name',
+  'message',
+  'stack',
+  'code'
+]
+
+// Reworked from https://github.com/sindresorhus/serialize-error
+export const miniSerializeError = (value: any): any => {
+  if (typeof value === 'object' && value !== null) {
+    const simpleError: SimpleError = {}
+    for (const property of commonProperties) {
+      if (typeof value[property] === 'string') {
+        simpleError[property] = value[property]
+      }
+    }
+
+    return simpleError
+  }
+
+  return value
+}
+
 /**
  *
  * @param type
@@ -52,16 +82,6 @@ export function createAsyncThunk<
     }
   )
 
-  const finished = createAction(
-    type + '/finished',
-    (requestId: string, args: ActionParams) => {
-      return {
-        payload: undefined,
-        meta: { args, requestId }
-      }
-    }
-  )
-
   const rejected = createAction(
     type + '/rejected',
     (error: Error, requestId: string, args: ActionParams) => {
@@ -81,32 +101,34 @@ export function createAsyncThunk<
     ) => {
       const requestId = nanoid()
 
+      let result: Returned
       try {
         dispatch(pending(requestId, args))
-        // TODO Also ugly types
-        const result = (await payloadCreator(args, {
+
+        result = (await payloadCreator(args, {
           dispatch,
           getState,
           extra,
           requestId
         } as TA)) as Returned
-
-        // TODO How do we avoid errors in here from hitting the catch clause?
-        return dispatch(fulfilled(result, requestId, args))
       } catch (err) {
-        // TODO Errors aren't serializable
-        dispatch(rejected(err, requestId, args))
-      } finally {
-        // TODO IS there really a benefit from a "finished" action?
-        dispatch(finished(requestId, args))
+        const serializedError = miniSerializeError(err)
+        dispatch(rejected(serializedError, requestId, args))
+        // Rethrow this so the user can handle if desired
+        throw err
       }
+
+      // We dispatch "success" _after_ the catch, to avoid having any errors
+      // here get swallowed by the try/catch block,
+      // per https://twitter.com/dan_abramov/status/770914221638942720
+      // and https://redux-toolkit.js.org/tutorials/advanced-tutorial#async-error-handling-logic-in-thunks
+      return dispatch(fulfilled(result!, requestId, args))
     }
   }
 
   actionCreator.pending = pending
   actionCreator.rejected = rejected
   actionCreator.fulfilled = fulfilled
-  actionCreator.finished = finished
 
   return actionCreator
 }

type-tests/files/createAsyncThunk.typetest.ts

@@ -22,6 +22,5 @@ function fn() {}
         expectType<number>(action.payload)
       })
       .addCase(async.rejected, (_, action) => {})
-      .addCase(async.finished, (_, action) => {})
   )
 }