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

HTTP.post() or any non-GET request does not follow redirects #2808

Closed
rclai opened this issue Oct 13, 2014 · 5 comments
Closed

HTTP.post() or any non-GET request does not follow redirects #2808

rclai opened this issue Oct 13, 2014 · 5 comments

Comments

@rclai
Copy link

@rclai rclai commented Oct 13, 2014

Regardless of setting the followRedirects option to true, non-GET requests will not follow redirects because of how request works.

According to here:
https://github.com/mikeal/request#requestoptions-callback

The followAllRedirects options must be set to true in order for non-GET requests to follow redirects.

In the code here: https://github.com/meteor/meteor/blob/devel/packages/http/httpcall_server.js

We need to be able to pass the followAllRedirects argument in.

@glasser
Copy link
Member

@glasser glasser commented Oct 21, 2014

You're right, the behavior does not match our docs.

@rclai
Copy link
Author

@rclai rclai commented Dec 8, 2014

I am just going to use the request npm. This is no biggie.

@rclai rclai closed this Dec 8, 2014
@glasser glasser reopened this Dec 9, 2014
@glasser
Copy link
Member

@glasser glasser commented Dec 9, 2014

I actually do think we should fix this. It's one of the few options we implement and document, so it should work properly! (That said, I also think we should minimize the amount of options we expose in packages like http so that we don't run into these sorts of details in the future, and it's totally reasonable to use the request npm module directly if you want to do more advanced things with http!)

@rclai
Copy link
Author

@rclai rclai commented Dec 9, 2014

Oh.. okay

estark37 pushed a commit that referenced this issue Dec 21, 2014
Fixes #2808.
estark37 pushed a commit that referenced this issue Dec 29, 2014
@rclai
Copy link
Author

@rclai rclai commented Dec 29, 2014

Thanks! :)

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

Successfully merging a pull request may close this issue.

2 participants