enRose / enRose.github.io

2 stars 4 forks source link

Are try-catch wrapper and Global error handler alternative options? #4

Open MNF opened 3 years ago

MNF commented 3 years ago

According to your https://enrose.github.io/React/redux-saga-error-handling.html post there are 2 alternative options. As I understand, they have different purpose: “ try-catch wrapper” handlers errors within saga functions(generators), “ Global error handler” (Option 1) in middleware handles errors happened in reducer, not sagas.

According to https://redux.js.org/advanced/middleware Redux middleware provides a third-party extension point between dispatching an action, and the moment it reaches the reducer. So it couldn’t catch the errors in sagas. Please correct me, if my understanding wrong.

sagaMiddleware.run(rootSaga).done.catch ( option 2) doesn’t allow to compensate/handle error.

From https://redux-saga.js.org/docs/basics/ErrorHandling.html:

Errors in forked tasks bubble up to their parents until it is caught or reaches the root saga. If an error propagates to the root saga the whole saga tree is already terminated.

So you can only log the error and terminate application, as sagas are corrupted.

enRose commented 3 years ago

Hi Michael,

First of all, thanks for reading my article.

The safe try catch wrapper is for when we have specific error for a specific generator we are interested in handling.

Whereas the global error handler is relying on the fact that if there is no try-catch in a generator, the error will bubble up to the root saga then it will be handled in redux middleware. Its main application is to ensure we do not litter try-catch everywhere all over our code base. And the good old Unix error handling principle states that if we don't have anything interesting to say keep it quiet. So, if we don't have anything we can contribute to in a specific error scenario, we should let the root catch it and handle it in a unified way for us. e.g. ASP.NET web API & .net core has this default global error handler which umbrellas across all uncaught errors and translates them into an InternalServer error.

Redux mw error handler should be used just like that - as a blanket that rules over all un-caught errors and translates it into a generic or user-friendly error message. At lest, this is how my team uses it.

If I understand your question correctly, you were saying that putting an error handler in mw does not catch errors thrown from the saga generators. I can assure you it does! And we are using it in production code. It is called the onError hook. I quote:

'onError: (error: Error, { sagaStack: string }) - if provided, the middleware will call it with uncaught errors from Sagas. useful for sending uncaught exceptions to error tracking services.'

So this onError hook is what I listed in the article as option 1.

Option 2, will also work because mw is a promise, so promise will have try-catch clause too. I am aware the latest mw might have changed the API so the below code should work if you're on latest version:

sagaMiddleware.run(rootSagas, store.dispatch).toPromise().catch(e => {
        logger.error({ message: e.message, source: 'sagaError', stacktrace: e.sagaStack });
    });

And you don't use option 1 & 2 at the same time. You use them exclusively.

I hope I've answered your question happy to talk more if not.

michael-freidgeim-webjet commented 3 years ago

Sorry for the late response. Unfortunately, in Global error handler ( we are using sagaMiddleware.run(rootSaga).done.catch -option 2) you can only log the error and close the application, but can’t ignore the error in specific saga function and continue to work. Example of the error we have is

TypeError: Cannot read property 'layout' of null at flightsFetchSuccess$ (webpack-internal:/ ./app/SearchResults/sagas.js:573) at tryCatch (webpack-internal:///./node_modules/regenerator-runtime/runtime.js:65) …. webpack-internal:///./node_modules/@redux-saga/core/dist/io-6de156f3.js:179 The above error occurred in task flightsFetchSuccess created by takeEvery(FETCH_FLIGHT_FINISH, flightsFetchSuccess) created by rootSaga Tasks cancelled due to error: takeEvery(FETCH_FLIGHT_FINISH, flightsFetchSuccess) takeEvery(INIT_APP, initApp) …. List all dozens of my takeEvery sagas

In our experience only ‘ Option 1: try-catch wrapper’ can handle errors and allow application to continue, ‘ Option 2: Global error handler‘ better to name ‘Global error logger