Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Option for not transforming error responses #131

Closed
stevage opened this issue Jun 23, 2016 · 4 comments
Closed

Option for not transforming error responses #131

stevage opened this issue Jun 23, 2016 · 4 comments
Labels
enhancement

Comments

@stevage
Copy link

stevage commented Jun 23, 2016

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.
@analog-nico
Copy link
Member

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?

@stevage
Copy link
Author

stevage commented Jun 24, 2016

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

@analog-nico
Copy link
Member

Hi @stevage , I just released [email protected] which implements the transform2xxOnly option. Thanks for bringing this up!

@stevage
Copy link
Author

stevage commented Jul 17, 2016

Oh, nice one :)

@gorjanz gorjanz mentioned this issue Jan 10, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@stevage @analog-nico