redux-saga: Inconsistent error handling

TL;DR

1. Can’t catch exceptions fired by put.
2. Unhandled exceptions are not always being caught by a custom global handler (if specified).
3. If a custom global handler is specified, the default console logging (by the saga middleware) is not being suppressed.
4. Handling exceptions of a spawned task is not considered as handling by the saga middleware.

Related issues:

• #250 Handling uncaught exceptions
• #374 Question: handling uncaught exceptions - spawn vs fork
• #441 Weird (incorrect?) error handling behaviors
• #626 Contradiction in docs whether put is blocking

The problem

We have a shopping cart on our website, and the ability to store it locally and restore afterwards at some point.

During a restore we can encounter some errors, e.g.:

• storage is not available
• restored data is broken (and can’t be rendered)

We want to be able to catch and handle any of these cases (here and actually in other sagas in general):

• at least report the error to a backend like NewRelic
• recover from the error, don’t break the whole app (don’t abort the root saga)

Initial code

Here is our code (slightly simplified, but very close to reality).

function fetchCart(restaurantId) {
    // Fetches the cart and returns a Promise, which returns cart items.
}

function reportError(e) {
    // Logs errors, sends them to some backend like NewRelic, etc.
}

const sagaMiddleware = createSagaMiddleware({
    onError: reportError
});

function* restoreCartHandler(action) {
    const { restaurantId } = action.payload;
    try {
        const cartItems = yield call(fetchCart, restaurantId);
        yield put(cartRestored(cartItems));
    } catch (e) {
        // If something goes wrong, we clear the cart and allow user to start from the beginning.
        reportError(e);
        yield put(clearCart(restaurantId));
    }
}

function* restoreCartSaga(context) {
    yield takeEvery(RESTORE_REQUESTED, restoreCartHandler);
}

If the data is broken and put throws (e.g. from a reducer or from a component’s render method), we can’t catch it in restoreCartHandler. put is async (non-blocking), and it always returns ASAP without exceptions. Exceptions bubble up to the redux-saga middleware and are finally console.logged as uncaught at restoreCartHandler. Still, we can catch other exceptions: e.g. syntactic and from the fetchCart (e.g. storage is not available).

If the data is broken, this is what I see in the browser console:

So here are two problems:

1. We can’t catch exceptions triggered by put and handle the case in the restoreCartHandler.
2. Moreover, this unhandled exception is not being caught by the global handler.

Solution 1: wrapped fork and spawn

Based on #250 and #374 I tried to implement my own takeEvery which could catch exceptions from put effects and report them.

function* forkSafely(saga, ...args) {
    const task = yield fork(saga, ...args);
    task.done.catch(e => {
        console.log('Exception caught inside forkSafely', e);
    });
    return task;
}

function* spawnSafely(saga, ...args) {
    const task = yield spawn(saga, ...args);
    task.done.catch(e => {
        console.log('Exception caught inside spawnSafely', e);
    });
    return task;
}

function forkWrapped(saga, ...args) {
    return call(forkSafely, saga, ...args);
}

function spawnWrapped(saga, ...args) {
    return call(spawnSafely, saga, ...args);
}

export function* takeEverySafely(pattern, saga, ...args) {
    while (true) {
        const action = yield take(pattern);
        yield forkWrapped(saga, ...args.concat(action));
        
        // Or, instead:
        // yield spawnWrapped(saga, ...args.concat(action));
    }
}

export function* restoreCartSaga(context) {
    yield takeEverySafely(RESTORE_REQUESTED, restoreCartHandler, context);
}

The result using wrapped fork:

Neither Exception caught inside forkSafely, nor reporting the error from the global handler.

The result using wrapped spawn is the same.

So, in both cases (fork and spawn):

1. We can’t catch an exception triggered by put and handle it in the handler attached to the task.done.catch
2. Again, this unhandled exception is not being caught by the global handler.

Modified case: exception not in put

It was interesting to check what is the error handling behaviour if exceptions are fired somewhere else in the handler, not by the put.

function* restoreCartHandler(context, action) {
    // Let's just throw here and leave it unhandled
    throw Error('Sorry man');
}

const sagaMiddleware = createSagaMiddleware({
    onError: e => {
        // And let's modify this one just for clarity on screen shots
        console.log('Global saga error handler', e);
    }
});

I was expecting that fork and spawn return a task in that case, and task.done.catch will work ok.

Let’s see the result of using fork:

1. task.done.catch handler is not being called, no Exception caught inside forkSafely.
2. The exception bubbles up to the root saga, the root saga is being aborted.
3. The global handler is being called, but the default console logging is not being suppressed.

Let’s see the result of using spawn:

1. task.done.catch handler is being called, but after all the other handlers.
2. The exception doesn’t bubble up to the root saga.
3. Despite of the fact that the exception is handled by the handler attached to the spawned task:
   • the task is still considered uncaught (uncaught at restoreCartHandler)
   • the global handler is still being called

There were big hopes to the “spawn-and-catch” approach, but currently it doesn’t seem to be very usable due to its implementation. “Fork-and-catch” doesn’t work at all.

Solution 2: put().done.catch

The idea was to check whether I can asynchronously catch and handle errors of the put itself, since it seemed to behave similar to fork and spawn.

function* restoreCartHandler(action) {
    const { restaurantId } = action.payload;
    try {
        const cartItems = yield call(fetchCart, restaurantId);
        const task = yield put(cartRestored(cartItems));
        task.done.catch(e => {
            console.log('Exception caught inside put', e);
        });
    } catch (e) {
        // If something goes wrong, we clear the cart and allow user to start from the beginning.
        reportError(e);
        yield put(clearCart(restaurantId));
    }
}

function* restoreCartSaga(context) {
    yield takeEvery(RESTORE_REQUESTED, restoreCartHandler);
}

The result is:

1. put returns undefined, which leads to an exception inside the try block of the restoreCartHandler
   • this exception is being caught and reported
2. put is being executed as usual, with the “uncaught at…” result
   • so the original exception is not being caught, even by the global handler

Solution 3: put.sync

We used put everywhere in the codebase as we believed that it’s blocking. Since it happens that it was not true either from the beginning, or from some version (we’ve upgraded from 0.10 to 0.12), I decided to check put.sync.

function* restoreCartHandler(action) {
    const { restaurantId } = action.payload;
    try {
        const cartItems = yield call(fetchCart, restaurantId);
        yield put.sync(cartRestored(cartItems));
    } catch (e) {
        // If something goes wrong, we clear the cart and allow user to start from the beginning.
        reportError(e);
        yield put(clearCart(restaurantId));
    }
}

function* restoreCartSaga(context) {
    yield takeEvery(RESTORE_REQUESTED, restoreCartHandler);
}

The result is finally as expected:

1. put.sync throws exception inside the try block.
2. It is being caught, reported and handled properly.
3. The exception doesn’t bubble up to our global handler, and is not being logged by the middleware.

To sum up

Seems that the only viable solution for us - to use put.sync for the time being.

Still, I suppose error handling could have been more consistent:

• if put is non-blocking, it should return a task
• exceptions fired by put should be able to be caught
• what’s caught locally (both sync and async way), if there’s no re-throw:
  • should be treated as handled
  • should not bubble up to the root
• if a custom global handler is specified, no default logging should be done

This is what basically seems right from our perspective. Would be great to hear your opinion.