redux-observable: RFC: default error handling of Epics

Perhaps the most minimum thing we should do is:

Catch all epic errors that reach the middleware (aren’t caught by your epics) and log a warning in dev mode about how uncaught errors “terminate” your entire epic chain and that you might be interested in the various ways you can handle that–linking out to a new docs page that includes a few, but not all, examples of ways you might want to deal with it. We’d then rethrow the error we caught, so it still shows up in the console/logging infra.

If you have an alternative suggestion, please comment and folks can 👍 or 👎 as a way to vote and get insight into who’s still following this discussion and cares.

This is a quite long discussion to swallow… Any place where we can find a sumup of the recommended practices?

Here’s some ready-to-use code based on what the others said above:

// lib/helpers/epic.js
export const wrapEpic = epic => (...args) =>
  epic(...args).catch((error, source) => {
    setTimeout(() => { throw error }, 0)
    return source
  })

// modules/auth/epic.js
import { combineEpics } from "redux-observable"
import { wrapEpic } from "lib/helpers/epic"
export const epic = combineEpics( ...[
  epic1,
  epic2,
].map(wrapEpic) )

// app/rootEpic.js
import { combineEpics } from "redux-observable"
import { wrapEpic } from "lib/helpers/epic"
import authEpic from "modules/auth/epic"
export default combineEpics( ...[
  authEpic,
].map(wrapEpic) )

I added the wrapEpic to every place I use combineEpics and now all the epics handle the errors properly. I think it’s necessary - if I added it only to the rootEpic, then some of the epics weren’t restarted properly.

EDIT Though I’ve got some problems in HMR: errors that were thrown before are being thrown again after hot reload, even though the new epic is properly replaced and the old epics shouldn’t be working anymore (?). Any1 has a guess how to fix it?

P.S. If you still want your epic to stop and never restart, then instead of throwing an error, there are also other ways do it (f.e. https://github.com/Reactive-Extensions/RxJS/blob/master/doc/api/core/operators/never.md ) .

@slorber people can opt into automatic retry already today with the .retry() operator:

const somethingEpic = action$
  action$.ofType(SOMETHING)
    .mergeMap(action => Observable.throw('BAD'))
    .retry();

Edit: this swallows the error though! That’s probably bad. Instead, you can do this:

const somethingEpic = action$
  action$.ofType(SOMETHING)
    .mergeMap(action => Observable.throw('BAD'))
    .catch((e, source) => {
      // rethrow error so it shows up in the console
      setTimeout(() => { throw e; }, 0);
      // source is the chain above the catch, so this "retries"
      return source;
    });

You could make this a custom operator, of course.

Perhaps the answer to this whole problem is just to make this operator + usage more obvious in the docs…but I dunno.

This whole effort was mostly because I felt people would practically always want an epic to restart if it errors out. The argument against I guess is that those errors are uncaught exceptions for a reason–we can’t guarantee the behavior of the app continues correctly if we always restart them. However, in JS world I would argue people are very much used to apps which may have errors but try to chug along anyway, even if some things are completely broken. In the redux-observable case, if a single epic errors out without being caught, all of their epics will terminate. This cascade is pretty dubious and more analogous to traditional apps with a “crash”–but the user won’t receive any notification it happened (unless the dev adds one).

I’m definitely in favor of redux-observable not handling any errors for the reasons Ben mentioned:

1. Developers can configure their error handling in whatever way they choose.
2. Developers forced to learn error handling techniques in Rx.

I’d rather see recipes/patterns for error handling, exponential backoff, and retrying.

@hally9k I would recommend instead using function composition in your root Epic:

Assuming rxjs v6 and redux-observable 1.0.0-beta.1:

// TODO: replace doSomethingAndReturnObservable
const combineEpicsAndCatchErrors = (...epics) =>
  return (action$, state$) => {
    epics = epics.map(epic => (action$, state$) => epic(action$, state$).pipe(
      catchError(e => doSomethingAndReturnObservable(e))
    ));
    return combineEpics(...epics)(action$, state$);
  };
};

const rootEpic = combineEpicsAndCatchErrors(...epics);

If you don’t need to do it individually for each epic, it’s easier to just do it to the rootEpic (but has different behavior):

const rootEpic = (action$, state$) =>
  combineEpics(...epics)(action$, state$).pipe(
    catchError(e => doSomethingAndReturnObservable(e))
  );

Perhaps an eslint plugin would be of help? Linting would ensure that you have proper error handling on every single epic. An analogy from Java is where you are forced to catch exceptions when you use a certain method (e.g. public void writeSomeStuff() throws IOException), ensuring that exceptions are handled.

In React it is kind of the same, the uncaught error would be quite devastating for the application if left unhandled by the developer. To mitigate this, react provides a so-called error bounderies that lets you, mutch like try-catch, handle this where it makes sense.

In redux-observable, if you wanted to map uncaught exception and – for example – dispatch them to a general application-level Redux action, you could wrap them with something like:

const fetchUsersEpic = applicationEpic(($action) => (
     // carefree epic here 😅 
)}) // Dispatches { type: 'APP_UNCAUGHT_ERROR' } when errors occurs 
`

For us, a way to handle errors, for telemetry if nothing else, is quite valuable. Silently breaking the application is not too great. We’ve put together a small library which implements a supervisor pattern as some have discussed above, and also supports restarting epics. This is useful for more complex SPA, where if one component of the application fails, you might want to try giving it a kick instead of allowing it to remain broken: https://github.com/mixer/epic-supervisor

The current status quo is that it’s still not clear what people would prefer, but a majority of users haven’t complained so I’ve been inclined to keep things as they are. Always open to hear people’s thoughts!

@jayphelps I think we should mention this issue in Readme and provide a workaround solution.

I think anything redux-observable does for you to catch errors or report on errors should be opt-in and well documented. I prefer the flexibility of RxJS to handle any types of errors appropriate to the situation that might throw them.

@braco

If unadorned RxJS is important, noisier dev would be a reasonable compromise imo, and would go a long way to guide users in self-diagnosis. Flow control is nasty business, and I would assume this library will usually be the gateway drug to RxJS instead of the opposite.

As long as this library is a minimalistic glue layer between redux and RxJS (which I hope it remains) I think users are going to find it much easier to learn RxJS without the added complexity of interacting with redux and then learning to use them together, rather than using redux-observable as the gateway direction.

If this helps any rank amateurs in the meantime, I’m doing this:

https://gist.github.com/braco/28dc2006e9ce04a004d2d9c4f5b9a4b6

Convention here is

epics.js

import reducerName1 from 'reducerName1/epics';
...

export default wrapEpics({ reducerName1 });

reducerName1/epics.js

export const epicName = epic$ => ...
...

Then you get debug messages like reducerName/epicName errored with ... and reducerName/epicName exited

@jayphelps That sounds reasonable.

@meticoeus Just to be clear, would you be opposed to the default (not opt-in) behavior of: errors that caused the termination of every epic were console.error logged, but not caught? So a friendly message would let you know all epics were terminated, but the original error itself would still get caught by things like window.onerror etc and also show up in the console. The separate console.error will contain info the normal error won’t, like the “all epics have terminated due to an uncaught error in somethingEpic”

I think this would be useful in every situation, even for Rx pros.

ah that makes a lot of sense, thanks!

I’m also in favor of redux-observable not handling errors, but I think it would be really helpful if it would log uncaught exceptions so you can find out which epic is letting errors slip through.

I’ve also found that it silently crashes when exceptions are thrown in reducers, making some simple syntax errors hard to track down.

As a data point, this is how we do error handling in our (react native) app. A custom combineEpics that adds a repeat() to all epics.

return output$.repeat();

The point of the repeat() is to allow error catching in the epic with catch(x => Observable.of(errorActionCreator(x))). This pattern can then be used on both epics (hot observables) and cold ones that would just end up in an endless loop if adopting the catch((x, stream) => Observable.merge(Observable.of(errorActionCreator(x)), stream)) pattern everywhere.

On our top level combined epic we add a catch-all just as described earlier in this thread. This catch-all is only to be used as a last resort. All epics are expected to take care of their own errors.

It would be interesting to know what patterns people have adopted, especially for turning errors into actions.

@slorber

I guess you keep a hot action stream for event replay / hot code reloading, and make epic stream cold on purpose to avoid receiving past events on failure. Makes sense to me.

Not sure I follow? To clarify: if you use .retry() on the top-level stream returned in an epic, it will not replay previous actions. On error, it will simply “restart” the epic, listening for future actions.

The real stream of actions that redux-observable maintains internally is hot, which is one of the reasons why previous actions are not replayed–by design.

Related problems are experienced by redux-saga users regularly.

I think the solution is to implement “supervision strategy” that actor systems have. Sagas are generally implemented on the backend by using actor systems like Akka (even Greg Young, best advocate of eventsourcing/DDD said Akka is a saga framework).

The idea of the actor systems resilience is that the system should be able to heal itself on its own with supervision strategy. If one part of the actor tree fails, we can declare which strategy to adopt for that part of the tree.

The epic restarting itself is actually one of the most used supervision strategy in actor systems.

I think the behavior should rather not be encoded directly inside combineEpics but rather as a wrapper for each epic, because one should be able to setup fine-grained behavior per epic

More infos on this issue: https://github.com/yelouafi/redux-saga/issues/570

Turns out I was wrong saying that option 1 would allow unrelated epics to maintain their internal state. Not sure why I thought that in hindsight (confirmation bias). Effectively option 1 is in fact still letting the entire root epic terminate and then “restarting” it entirely with a new subscription.

I’ve updated the original post above to reflect that, as well as the the jsbin to reflect this–the console will show an incrementing number which will reset back to zero whenever the other ajax epic errors.

Again, that means option 1, catching errors at the root epic level, would cause all epics to lose their internal state, and restart.

Option 2 is catching them individually inside combineEpics:

export const combineEpics = (...epics) => (...args) =>
  merge(
    ...epics.map(epic =>
      epic(...args)
        // When epic emits error, rethrow it async and resubscribe.
        // Otherwise, errors will cause cascading termination of all epics. 
        ::_catch((error, source) => {
          setTimeout(() => { throw error; }, 0);
          return source;
        })
    )
  );

This however has its own caveats.

• If you don’t use combineEpics, obviously it won’t catch them–we could also catch the root epic as well, so that seems like a doable solution?
• Having combineEpics do this feels like magic that will most often go unknown to people and confuse them why this behavior doesn’t happen when they invoke epics manually when doing higher-order epics aka composition.

const epic1 = (action$, store) => stuff();
const epic2 = (action$, store) => epic1(moreStuff(), store);

We could later mitigate this by introducing a paved way to “fork” epics, as we’ve been experimenting with for a while. Forking is just like composition except you don’t need to pass along the store cause it’ll be passed along for you. We could catch errors in forks as well.

• Lastly, this would break some people who are right now catching their own errors and logging them. We’ve actually helped several people do this, and it would be particularly onerous because if they wanted to continue they’d have to effectively re-create combineEpic that _doesn’t have this catching behavior.

const rootEpic = (action$, store) =>
  combineEpics(epic1, epic2)(action$, store)
    .catch((e, source) => {
      logTheErrorToAPIOrSomething(e);
      return source;
    });

Summary

I feel kinda back to the drawing board on this one. Perhaps we should just add some clear documentation around the existing behavior and call it good for now until/if we come up with a real solution?

I’ve decided on do option 1 for now, feedback welcome. PR to come shortly.