agraboso / redux-api-middleware

Redux middleware for calling an API.
MIT License
1.49k stars 195 forks source link

Error propagation #163

Open niccolomeloni opened 6 years ago

niccolomeloni commented 6 years ago

Hi,

i'd like to ask you if is expected on next future to have error propagation (when REQUEST fail or FAILURE). At the moment i'm using following pattern:

dispatch(rsaa_action())
   .then(response => {
      if(response.error) /* ...A... */
      else /* ...B... */
   })

but i think it could be better if the error were propagated in order to use normal pattern:

dispatch(rsaa_action())
   .then(response => /* ...A... */)
   .catch(err => /* ...B...*/)

Please, could you let me know something about? Did i fell in a concept mistake?

My best, Niccolò

nason commented 6 years ago

Hi @niccolomeloni,

This is a great question. Can you take a look at https://github.com/agraboso/redux-api-middleware#lifecycle and let me know if that helps make sense of the middleware flow.

The error flag means different things at different times:

I'm curious how we can make this more clear / useful 🤔

niccolomeloni commented 6 years ago

Hi @nason,

i read the link you have shared and the source code of middleware. In order to accomplish error propagation i thing that it could be a solution in catch clauses perform next action with no return and then throw the typed error.

For example, instead of

try {
      // Make the API call
      var res = await fetch(endpoint, {
        ...options,
        method,
        body,
        credentials,
        headers: headers || {}
      });
    } catch (e) {
      // The request was malformed, or there was a network error
      return next(
        await actionWith(
          {
            ...requestType,
            payload: new RequestError(e.message),
            error: true
          },
          [action, getState()]
        )
      );
    }

middleware could perform

try {
      // Make the API call
      var res = await fetch(endpoint, {
        ...options,
        method,
        body,
        credentials,
        headers: headers || {}
      });
    } catch (e) {
      var err = new RequestError(e.message);
      next(
        await actionWith(
          {
            ...requestType,
            payload: err,
            error: true
          },
          [action, getState()]
        )
      );
      throw err; // return Promise.reject(err);
    }

This pattern could be applied for each catch clause in order to perform:

dispatch(rsaa_action())
   .then(response => /* ...A... */)
   .catch(err => /* ...B...*/)

Let me know what do you think about it.

My best, Niccolò

nason commented 6 years ago

Ah, gotcha! I think there's certainly value in doing something like this. The first things that come to my mind are: how would this change affect existing users, what would a migration plan look like, and so on.

I'm swamped for the rest of this week, but I'll think about this some more and chime back in soon.