Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error propagation #163

Open
niccolomeloni opened this issue Jan 2, 2018 · 3 comments
Open

Error propagation #163

niccolomeloni opened this issue Jan 2, 2018 · 3 comments
Labels

Comments

@niccolomeloni
Copy link

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
Copy link
Collaborator

nason commented Jan 2, 2018

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:

  • RSAA validation errors
  • RSAA function (headers, bailout, etc) options throw
  • fetch throws
  • network fails
  • non-2xx response code on response

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

@niccolomeloni
Copy link
Author

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 nason added the question label Jan 3, 2018
@nason
Copy link
Collaborator

nason commented Jan 3, 2018

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants