API overhaul (comments welcome)

[agraboso]

For a while now I've been thinking about a relatively major overhaul of redux-api-middleware. Time and again, people have expressed their need for extracting different kinds of data from server responses, and processing them differently (see #4, #9, #10, #13). The original source of the code in this module, the real-world example in the redux repository, was probably though by Dan Abramov in the restrictive setting of exactly what he wanted to do in that example, and not in terms of how it could accommodate the needs of a large cross-section of the developers using redux.

I've just pushed a new branch to redux-api-middleware named next. As of now, the only new thing is the README. I've spent some time trying to come up with a schema that is not only flexible (allowing for the kind of access to server responses that people have asked for, and might ask for in the future), but also robust (in the sense of being able to deal with errors precisely and predictably), and that README is what I've come up with.

I haven't started writing code implementing the changes outlined there — I hope to start doing so in the next few days. For the moment, I'd love to get input from those of you that have been using (or playing with) redux-api-middleware, to see if this rethinking of the API suits your needs.

It's late already today, but I'll try to write another post tomorrow detailing some of the most important changes and why I went for them. Just giving everybody a heads up in the meantime — after all, I put quite a lot of effort into the README itself.

cc @svnlto @seokgyo @lraivio @mohanzhang @eventhough @RanzQ @vdemin @latentflip

[seokgyo]

Hi,
I've looked through the new document and it looks good to me. Actually I was under refactoring my code and came up with almost same approach.

And if headers could be a function w/ state as param, this would give much more flexibility to api middleware.

[lraivio]

Nice work on the README @agraboso.
Off the top of my head I can't come up with anything I would need that the new API would not provide. I really like the type description approach and the clear error handling. Those are definitely something I will need in the future.

As @seokgyo pointed out, it could be useful to have access to state in headers.
I'll be using redux-middle-ware more in the following weeks and I'll let you know if anything comes up.

[agraboso]

@seokgyo @lraivio Thanks for your input. I've given [CALL_API].headers access to the Redux store state per your request.

If you check out the next branch, you'll see I've implemented everything I described in the README. When I say "implemented", though, I just mean that I've written some code. It lints without errors, but more than half of the tests fail — obviously —, so there may be some trivial bugs.

In order to accommodate waiting for promises returned by custom payload and meta properties in type descriptors to resolve, I've introduced an ES7 async function in the source code. It's the first time I use one of those, so it is entirely possible that I've made a complete fool of myself there.

I'll get to work on those tests soon, but I thought I would share with you what I have done in the last couple of days. You may even detect errors in the code just by looking at it...

[agraboso]

After working on implementing what I set out to do in the README a few days ago, I have just put out a beta version, both on Github and npm — look for 1.0.0-beta1.

I had to change several things:

• My take on error handling was based purely on whatwg-fetch. In order to accommodate node-fetch, I had to simplify the scheme a bit. That came with a benefit, though: failure FSAs are now fully customizable — as opposed to how it was in my original proposal, in which there were exceptional cases that required user-provided payload properties to be ignored
• When I originally wrote the new lifecycle, I made a couple of mistakes in the usage of async functions (it was, after all, the first time I used them). That is now fixed — and, might I add, the code dealing with asynchronous stuff looks quite beautiful now (to me, at least).

Please let me know if you find any bugs, or if you fundamentally disagree with any of my design decisions.

[jonnyreeves]

Hey @agraboso, thanks for taking the lead on this middleware.

Quick question regarding a new API, do you have any thoughts on how requests can be cancelled (aborted) and how to incorperate middleware for apply common business logic before requests are sent / when a request completes (for example, doing the OAuth Grant dance when the Access Token expires).

[agraboso]

@jonnyreeves Requests cannot be cancelled: the fetch API does not support it (at least not for the moment, though it is being discussed).

With respect to the OAuth Grant dance: if your token is expired, the server should respond with a 401 status, which would result in a failure FSA. I haven't given any thought to how to get a refresh token and reissue the request... (see #2 though)

[grahamlyus]

Thanks for producing this. I noticed that the requestType action doesn't get emitted until the fetch completes in the new API. Also perhaps it should emit the failureType in the catch. Maybe it should be?:

    next(await actionWith(
      requestType,
      [action, getState()]
    ));

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

[jonnyreeves]

@agraboso, I'm having trouble getting my head around how a refresh flow would be implemented in redux, in my mind:

1. React component dispatches an action to fetch some data.
2. api-middleware makes the request but the server returns 401; failure FSA is dispatched.
3. Observer / a new piece of middleware picks up on the failure FSA, identifies it as a grant-expiration failure and starts a refresh of the user's grant (which is async and will result in a change to the store when the new grant is retrieved).
4. Refresh completes, the original request is replayed with the new accessToken.

The main issue I'm having is how can the observer / new piece of middleware replay the original request - it will not have access to the original FSAA.

Any thoughts would be greatly appreciated.

[agraboso]

@jonnyreeves: you can specify a custom payload or meta property in the failure type descriptor that returns an object containing a copy of the original RSAA. That'd give the next middleware the information it needs to replay the request.

Maybe you should do so only in case res.status === 401 to avoid sending said original RSAA with other failure FSAs.

[agraboso]

@grahamlyus: you're right, the request FSA should be emitted right before calling fetch.

About changing the type of the FSA in the catch block: I have some reservations. In the original plan for the API overhaul, I had request FSAs dispatched in case of a malformed fetch request, and failure FSAs for network failures. This made payload and meta properties of failure FSAs more complicated: for network failures res was undefined, while for server responses outside the 200 range it was not. This meant forcing consumers of redux-api-middleware to always check whether res is undefined or not before trying to access it.

This extra complexity still made some sense because I thought fetch returned different types of errors for malformed requests and for network failures. Alas, it is only whatwg-fetch that does so: node-fetch doesn't. So I went with error request FSAs for network failures.

In summary, I'm proposing this:

    next(await actionWith(
      requestType,
      [action, getState()]
    ));

    try {
      // Make the API call
      var res = await fetch(endpoint, { method, body, credentials, 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()]
      ));
    }

The upside is that failure FSAs are clearly delimited to server responses outside of the 200 range. The downside is the possibility of two request FSAs being dispatched — but I don't think that'd make reasoning about redux-api-middleware any more complicated. Please let me know of any comments you (or others) might have about this.

At the end of the day, I'm left with the feeling that whatwg-fetch and node-fetch could have a more coordinated error handling scheme, and that the Fetch API specification can still evolve to be more flexible.

[grahamlyus]

Thanks for the response. I'm still pretty new to React/Redux. I noticed the issue whilst trying to figure out how to show a global Spinner component whilst Signing In via an API. I'm not sure of my approach, but I ended up creating additional middleware to check the meta property of actions to see if apiRequestStart or apiRequestFinish were true, to show or hide my Spinner, respectively. I also hacked the dispatch of a location change into the meta function of my success type, as I understand it's bad form to do it in the reducer (and the reducer doesn't have access to dispatch currently).

The possibility of dispatching the requestType action twice is a little confusing. Here's my current signIn action for reference:

export function signIn(email, password, dispatch, location) {
  return {
    [CALL_API] : {
      endpoint: `${baseUrl}/sign_in.json`,
      method: 'POST',
      headers: { 'Content-Type': 'application/json' },
      body: JSON.stringify({
        user: {
          email,
          password
        }
      }),
      types: [
        {
          type: SIGN_IN_REQUEST,
          meta: { apiRequestStart: true }
        },
        {
          type: SIGN_IN_SUCCESS,
          meta: () => {
            gotoNextPathname(dispatch, location);
            return { apiRequestFinish: true };
          }
        },
        {
          type: SIGN_IN_FAILURE,
          meta: { apiRequestFinish: true },
          payload: (action, state, res) => {
            let msg;
            if (res.status === 401) {
              if (res.data && res.data.error) {
                msg = err.data.error;
              } else {
                msg = 'Incorrect email or password';
              }
            } else {
              msg = 'Unknown error occurred :-(. Please, try again later.';
            }

            return Error(msg);
          }
        }
      ]
    }
  };
}

It doesn't quite feel right to me, I feel my previous version, using axios is easier to understand, although (apologies) this may now be getting a bit OT:

export function signIn(email, password, location) {
  return async (dispatch) => {
    try {
      dispatch(showSpinner());

      const { data } = await axios.post(`${baseUrl}/sign_in.json`, {
        user: {
          email,
          password
        }
      });

      dispatch({ type: SIGN_IN_SUCCESS, payload: data });

      gotoNextPathname(dispatch, location);
    } catch (err) {
      let msg;
      if (err.status === 401) {
        if (err.data && err.data.error) {
          msg = err.data.error;
        } else {
          msg = 'Incorrect email or password';
        }
      } else {
        msg = 'Unknown error occurred :-(. Please, try again later.';
      }

      const error = Error(msg);

      dispatch({ type: SIGN_IN_FAILURE, payload: error });
    } finally {
      dispatch(hideSpinner());
    }
  };
}

[agraboso]

@grahamlyus The duplication of request FSAs is easy to reason about: you get at most one with an error property set to true, and at most one with an error property set to false (and, though it is not guaranteed, I'd expect the first one to be always dispatched before the second should both be issued).

It seems you might want to show your spinner on a non-error request FSA, and hide it on an error request FSA—but then again I might be misunderstanding your code (after all, I don't know what dispatch is or what your intention with it is, and this is the first time I hear of axios).

I'm happy to keep talking about your issue, but I feel it might be outside of the realm of redux-api-middleware, and more an issue of application design in a react/redux setting.

[agraboso]

@grahamlyus Another thing you made me realize is that custom payload and meta properties are not wrapped in any error-handling mechanism: if they throw, redux-api-middleware blows up. Me thinks I should add a try-catch block in actionWith...

[grahamlyus]

@agraboso Glad I was of some help ;) dispatch is the Redux dispatch function, and axios is a 'Promise based HTTP client for the browser and node.js'.

[agraboso]

@grahamlyus Oh, now I see what you are doing. Indeed, a side effect in a a custom meta function is kind of hacky (the intention was always to have it be a pure function, really). More natural would be to use redux-rx as @mohanzhang explains in #2.

[traversal]

Thank you so much for this overhaul of the middleware @agraboso it's exactly what I needed.

Your documentation is extremely clear, and your type descriptor design is very elegant. I had built a number of screens in my app which all needed normalizr, which worked fine with custom middleware based on the CALL_API from Redux examples. But then of course I needed to call on data which was a bit more freeform, which would have meant duplicating a large percentage of the plumbing into another middleware... yuck.

This middleware allowed me to easily create custom type descriptors to normalize responses, and now I should easily be able to pull in the other data without the normalize step. While I was there I also fixed up a lot of my Flux actions which didn't conform to the FSA spec, so I learnt something there too. :)

Keep up the good work, I really appreciate it :)

[riophae]

Is the project currently still under maintained?

[agraboso]

@riophae: I haven't abandoned it, though I admit it might look like it: I have been very busy the last couple of months. I hope to be able to devote some time to redux-api-middleware this coming week.

[riophae]

@agraboso Excited to hear that. Keep up the awesome work!

[agraboso]

Closing: v1.0.0 is now out. If you would like any lingering ideas from this thread to be considered for a future version, please open an issue (or, even better, submit a PR) specific to it.

[AlbertoLopSie]

@jonnyreeves: you can specify a custom payload or meta property in the failure type descriptor that returns an object containing a copy of the original RSAA. That'd give the next middleware the information it needs to replay the request.

Maybe you should do so only in case res.status === 401 to avoid sending said original RSAA with other failure FSAs.

Hi!

Any example on this?

I´m trying to add redux-refresh-token to your middleware but the code is quite old and doesn´t work fine.