Two consecutive request FSAs dispatched on network error

[sgoll]

Contrary to the documentation, the current implementation dispatches two consecutive request FSAs when a network error occurs.

In README.md it says in section "Lifecycle", bullet point no. 3:

Now that redux-api-middleware is sure it has received a valid RSAA, it will try making the API call. If everything is alright, a request FSA will be dispatched with the following property: (…)
(…)
If such an error occurs, a different request FSA will be dispatched (instead of the one described above). It will contain the following properties: (…)

The important thing to note here is "a different request FSA (…) instead of the one described above".

However, the actual code in src/middleware.js sends two request FSAs in case of exceptions thrown during the fetch call:

// We can now dispatch the request FSA
next(await actionWith(
  requestType,
  // (…)
));

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,
      // (…)
    },
    // (…)
  ));
}

This is problematic as we now cannot be sure which of the two request FSAs is which, or rather: once the non-error request FSA has fired, we should be able to assume that the request is either handled successfully, i.e. a success FSA shows up, or fails with a failure FSA. Receiving another request FSA, now with error set, is counter-intuitive (and doesn't match the documentation).

However, I'm not really sure how best to proceed from here. Since the fetch call can fail for all kinds of reasons, even late in the request (server accepted but then closed connection unexpectedly), it seems impossible to simply delay the first, non-error request FSA until we can be sure that no error will be thrown.

Instead, we should probably always send a failure FSA, i.e. both when the server responds with a status code other than 2xx and when a network error occurs. In this case, it would be sufficient to change the documentation and simply replace requestType in the code above with failureType.

Incidentally, this seems to also match the documentation regarding Failure Type Descriptors where the code sample shows the following if-check:

meta: (action, state, res) => {
  if (res) {
    return {
      status: res.status
      statusText: res.statusText
    };
  } else {
    return {
      status: 'Network request failed'
    }
  }
}

In the current implementation, the res argument to meta is always set. With the proposed change, it would either be set (server responded with code other than 2xx) or unset (network error occurred).

In case this issue is accepted, please let me know if I should prepare a pull request for the proposed change (documentation and code).

[agraboso]

@sgoll Thank you for the report and the detailed analysis.

When arriving at this beta, I first wrote the documentation, reflecting what I wanted redux-api-middleware to do, and then wrote the code. I had to make some changes along the way that muddled up the issue of network failures—error-reporting in isomorphic-fetch is less than ideal.

As you can probably guess by the fact that I haven't been able to respond to several issues over the last month or so, I have been very busy—and will continue to be for a couple of weeks. I'll look carefully into it as soon as I can. A PR might actually help me reason about it quick, so I'd very much welcome it.

[smazurov]

@agraboso any chance we could arrive at a 1.0.0?

[barrystaes]

By now v1.0.0 is out, and the issue of a 2nd FSA being issued is tackled in PR #43 #26

[sgoll]

@barrystaes: How is #43 applicable here? It does not seem to address the issue at hand.

[barrystaes]

Whoops typo, i meant to link to the other PR: #26