redux-routine casts anything to an Error object from a rejected promise.

[TimothyBJacobs]

Describe the bug
When an apiFetch is yielded from a control, it is impossible to access the response error object because it is cast to an Error object which only accepts strings.

To Reproduce
Steps to reproduce the behavior:
Setup a apiFetch control. I'm using the definition that GB uses in core-data.

In an action or resolver, yield an apiFetch that server will return a non 200 error for.

function* myAction() {
  try {
    yield apiFetch( /* ... */ );
  } catch ( e ) {
    console.dir( e );
  }
}

e is an Error object with the message [object Object].

This seems to happen because castError in redux-routine will cast anything rejected from a promise to an Error object which only supports string messages.

See related Slack discussion: https://wordpress.slack.com/archives/C02QB2JS7/p1543350222369000

Expected behavior
To be able to access the original REST Response body.

Workarounds
Change the control definition to:

API_FETCH( { request } ) {
	return triggerApiFetch( request ).catch( ( error ) => {
		throw new Error( JSON.stringify( error ) );
	} );
},

This would then require parsing the JSON again in any consuming code.

[nerrad]

I'm keen on exploring whether we should throw actual Error instances (or custom error objects extending Error`) from apiFetch?

[TimothyBJacobs]

I opened a PR that fixes this using the approach of throwing actual Error instances.

Another possibility is that we expose a responseToError function that creates the ErrorResponse object from a response object instead of always doing it. This would maintain 100% BC and then the control definition can be changed to:

function responseToError( response ) {
	throw new ErrorResponse( response );
}

API_FETCH( { request } ) {
	return triggerApiFetch( request ).catch( responseToError );
},

Though I think this would end up giving us a slightly less useful stack trace.

[nerrad]

In #13315, I have an alternative (or complementary possibly) fix to this which changes how castError handles the error argument value. I think we still need to resolve the issue here because the currently proposed fixes by @TimothyBJacobs only address apiFetch. castError still needs to handle any promises yielded by controls (not just apiFetch promises).

[nerrad]

@TimothyBJacobs #13315 has been merged which should fix this issue. However, I think it's still up in the air whether apiFetch could serve to be improved or not. So I'll leave it up to you whether you leave this issue and your pull open. I'm kind of on the fence with it.

[aduth]

Can we update the issue title with whatever it ends up being oriented toward, if kept open to serve a different purpose from what's already been fixed?

[aduth]

Let's consider this as satisfied by #13315. If further improvements are desired, a new issue should be created.

(Commented similarly at #12376 (comment))

[TimothyBJacobs]

Sorry, yes the changes in #13315 seem sufficient to me.