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.

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

Add headers attribute in StatusError object #84

Closed
wants to merge 2 commits into from
Closed

Add headers attribute in StatusError object #84

wants to merge 2 commits into from

Conversation

Chillaso
Copy link

Hi, I think error response should contains headers. I was trying to follow a redirect in a 301 status code but would be impossible if location header is not present.

What do you think about this approach? Would be another good solutions in order to follow redirects?

Thanks so much for reviewing.

@mikeal
Copy link
Owner

mikeal commented Mar 23, 2020

first of all, if you are expecting a 301 you should probably put that status code in as valid and then just check the status code on success :) wrapping that in a try/catch on purpose will make the code perform much worse (prevents lots of VM optimizations).

that said, i have no problem with this feature, it just needs to be identical for the browser side and it needs a test.

@Chillaso
Copy link
Author

Ok, now I see what troubles I had. The first approach I tried was do statusCode checking on success, but the problem was how manage encoding, when I was defining bent object was like bent('json', 200, 301). Hence, I couldn't access to statusCode, so I read the documentation and tried bent(200,301) which allow me to access directly to response raw object.

The big problem is that with that approach we're missing encoding cool feature and we have to decode response data, which means implement getBuffer method present in nodejs.js file already. Probably we could make public getBuffer or a wrapper function for each encoding and document an example of this approach.

What do you think about this? I'm doing right? I'm not sure if i'm misunderstanding bent library.

@mikeal
Copy link
Owner

mikeal commented Mar 30, 2020

Probably we could make public getBuffer or a wrapper function for each encoding and document an example of this approach.

This would introduce a sizable divergence from the Browser.

How about we just match the browser’s fetch methods for this, so in the returned response object we add methods that return promises like .json()?

@mikeal mikeal closed this Apr 13, 2020
@mikeal
Copy link
Owner

mikeal commented Apr 13, 2020

This feature was added in a recent version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants