request/request-promise

Option for not transforming error responses

stevage opened this issue · 4 comments

I realise I'm too late on this one, and just discovering request-promise now, but this (#86) seems like a really weird change. I'm using transform to extract part of the body of a successful API query. I can't really imagine ever wanting to apply the same function to both successful and unsuccessful queries.

Before:

        transform: function(body) {
            return body.files[Object.keys(body.files)[0]].content; // find the contents of the first file in the gist
        }

After:

        transform: function(body, response) {
            if (!(/^2/.test('' + response.statusCode))) { 
                return response
            }
            return body.files[Object.keys(body.files)[0]].content; // find the contents of the first file in the gist
        }

Seems like a lot of ugly boilerplate for nothing.

Perhaps an option, transformErrors: false, to restore the previous behaviour.

Well, I have to admit that I am not aware of many different use cases for the transform option. In your case I can fully understand that this change is weird to you. Sorry.

Anyway, the change at least decoupled the implementation of the function passed to transform from the value used for the simple option. Before v3 you had to implement the transform function the same way as your "After" example if you used simple = false.

Considering the gained decoupling – which we should keep – I take your suggestion of the transformError and adapt it a little bit:

  • I'd call the new option transform2xxOnly with default false => Keeps v3's current behaviour
  • Using transform2xxOnly = true and simple = true fixes your use case – you can go back to "Before"
  • Using transform2xxOnly = true and simple = false, transformWithFullResponse = true would also only transform 2xx responses – the gained decoupling is preserved
  • Using transform2xxOnly = true and simple = false, transformWithFullResponse = false would throw an error when invoking the request because .then(function (body) { /* likely no way to know if transform happened or not */ })

What do you think?

Yeah, that looks good. I'm using simple = true.

Hi @stevage , I just released request-promise@4.0.0 which implements the transform2xxOnly option. Thanks for bringing this up!

Oh, nice one :)