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

net: allow controlling redirects #9007

Merged
merged 3 commits into from Mar 28, 2017
Merged

net: allow controlling redirects #9007

merged 3 commits into from Mar 28, 2017

Conversation

deepak1556
Copy link
Member

Introduces redirect mode option on net.request that takes one of follow, error, manual and defaults to follow. In manual mode a redirect event is emitted on the request object with statusCode, method, newURL, responseHeaders. Calling request.followRedirect will continue with the redirection in manual mode and request.abort to cancel the request.

Fixes #8868

@zeke
Copy link
Contributor

zeke commented Mar 24, 2017

@deepak1556 does this need some user-facing documentation changes?

@deepak1556
Copy link
Member Author

@zeke yup, have added the docs. Please take a look, thanks!

* `redirect` String (optional) - The redirect mode for this request. Should be
one of `follow`, `error` or `manual`. Defaults to `follow`. When mode is `error`,
any redirection will be aborted. When mode is `manual` the redirection will be
deferred until [`request.followRedirect`](#requestfollowRedirecct) is invoked, listen to [`redirect`](#event-redirect) event in
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

requestfollowRedirecct → requestfollowRedirect

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

invoked, listen to → invoked. Listen for the

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed, thanks!

@deepak1556
Copy link
Member Author

@arantes555 can you verify if this PR helps with your use case , thanks!

it('should throw if given an invalid redirect mode', function (done) {
const requestUrl = '/requestUrl'
try {
const urlRequest = net.request({
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would using assert.throws make sense here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was reusing the existing spec. Have changed it, Thanks!

@arantes555
Copy link

@deepak1556 Looks good to me! Would be even better if it were possible to know whether or not a redirect happened in follow mode :) But hey, it's possible to do it manually.

@kevinsawicki
Copy link
Contributor

Looks good to me 👍 Thanks @deepak1556 🎆

@kevinsawicki kevinsawicki merged commit 4441d55 into master Mar 28, 2017
@kevinsawicki kevinsawicki deleted the net_redirect_patch branch March 28, 2017 16:31
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

4 participants