Allow parsing of error object by given transform

[Limess]

request-promise doesn't seem to apply the given options.transform function to the response object attached to a StatusCodeError.

I'd expect that transform to be applied, given the relation implied by "Rejected promises and the simple option" example. In this case, the standard request library would have parsed the request body before it was filtered by status code. The response at this point is not an error with the request so I see no gain in not parsing the response.

I realise this would be a breaking change, so was wondering if it would be of interest to pass an additional option to enable this behaviour?

For example, something along the lines of

} else if (self._rp_options.simple && !(/^2/.test('' + response.statusCode))) {
    var statusCodeError;
    if (self._rp_options.transformError && isFunction(self._rp_options.transform)) {
        try {
            body = self._rp_options.transform(body, response, self._rp_options.resolveWithFullResponse);
        } catch (e) {
            return self._rp_reject(e);
        }
     }
     self._rp_reject(assign(new errors.StatusCodeError(response.statusCode, body), {
        error: body,
        options: self._rp_options,
        response: response
     });

Thanks

[analog-nico]

Hi @Limess good point. Thinking about that I have several APIs I call which usually return JSON but HTML for e.g. a 404. In this case the transform function would need to check the response.statusCode (second argument) and transform appropriately. So I wonder how useful this would actually be. Could you describe me your use cases for which you would like to use the new option?

[Limess]

@analog-nico
I was actually under the impression that request-promise didn't automatically parse the response body as JSON when raising this issue, I've since realised that isn't the case!

Problems do still arise however. We're dealing with APIs which always return JSON. There's a current case when they return non-400s and we use the simple flag:
The message set on StatusCodeError is Error: ${error.statusCode} - [Object object] as it is set without transform, and there is no option to do this transform with simple. It'd be easier if we could transform the body to a readable message and useful error response here.

Admittedly that could be done by catching the error and re-throwing appropriately, but it's caught us out that we get unintelligible error messages with non-200s by default, and it may be useful to have an optional transform in this specific case.

It'd be nice to have your thoughts on how to handle these scenarios from an error handling point of view!

[analog-nico]

I can't really tell what is better because I do not use the transform feature personally. But since transform was introduced for usability it now seems natural to me to also transform non-200 responses. The transform function reduced another .then(...) for 200 responses and would now also reduce the additional .catch(...) for non-200 responses. The only drawback is the breaking change. Anyway, I think I will include this in the next major release.

[analog-nico]

Hi @Limess quick question: What do you think about adding a responseTransformed parameter to the StatusCodeError. Then response will still be the untouched response and responseTransformed the transformed response if a transform function is provided.

I see the advantage that (1) it is backwards compatible and (2) if the API returns a weird response the transform function could crash without screwing up the response handling. (responseTransformed would hold the Error thrown by the transform.) The developer implementing the transform function doesn't have to think about all error cases upfront - which would likely be impossible to do anyway.

As a disadvantage I see the inconsistent API.

Alternatively, we could put the transformed response into response and the original into responseNotTransformed.

[analog-nico]

I just released request-promise@3.0.0 which solves this issue.

StatusCodeError.response is the transformed response now. See the migration instructions at the top of the README for more info.