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 a beforeRedirect callback option #108

Merged
merged 4 commits into from Sep 25, 2015
Merged

Add a beforeRedirect callback option #108

merged 4 commits into from Sep 25, 2015

Conversation

@rogatty
Copy link
Contributor

rogatty commented Sep 23, 2015

PR based on #97.

I've added a new option - beforeRedirect - which is called before redirection is made. My use case for this was that we send all requests to the same URL but modify header Host to set the request target (we use Consul). Example of beforeRedirect callback from http://github.com/Wikia/mercury:

/**
 * We send requests to Consul URL and the target wiki is passed in the Host header.
 * When Wreck gets a redirection response it updates URL only, not headers.
 * That's why we need to update Host header manually.
 *
 * @param redirectMethod
 * @param statusCode
 * @param location
 * @param redirectOptions
 */
var beforeRedirect = (redirectMethod: string, statusCode: number, location: string, redirectOptions: any): void => {
    var redirectHost: string = Url.parse(location).hostname;

    if (redirectHost) {
        redirectOptions.headers.Host = redirectHost;
    }
};
@@ -1002,7 +1048,7 @@ describe('request()', function () {
setTimeout(function () {

res.writeHead(200);
res.write(payload);
res.write(internals.payload);

This comment has been minimized.

Copy link
@rogatty

rogatty Sep 23, 2015

Author Contributor

I've changed this line because tests were failing with message:

Multiple callbacks or thrown errors received in test "request() handles read timeout" (error): payload is not defined
      at null._onTimeout (C:\workspace\wreck\test\index.js:1051:27)
      at Timer.listOnTimeout [as ontimeout] (timers.js:110:15)
@geek geek added the feature label Sep 23, 2015
@geek geek added this to the 6.2.1 milestone Sep 23, 2015
@geek geek self-assigned this Sep 23, 2015
@geek

This comment has been minimized.

Copy link
Member

geek commented Sep 23, 2015

@rogatty everything looks good. Thoughts on renaming it to redirecting to keep it consistent with redirected?

@geek

This comment has been minimized.

Copy link
Member

geek commented Sep 23, 2015

@rogatty you may also want to advertise that wikia is using hapi here: http://hapijs.com/community

You can open a PR for that here https://github.com/hapijs/hapijs.com

here is an example: outmoded/hapijs.com@6900761

@rogatty

This comment has been minimized.

Copy link
Contributor Author

rogatty commented Sep 25, 2015

@geek redirecting sounds like something that's already in progress. I think that beforeRedirect is more standard and verbose (also afterRedirect would be better than redirected IMO). But feel free to modify it if you believe it's necessary.

@geek

This comment has been minimized.

Copy link
Member

geek commented Sep 25, 2015

@rogatty agreed... I think I will add afterRedirect.

geek added a commit that referenced this pull request Sep 25, 2015
Add a beforeRedirect callback option
@geek geek merged commit 9d376d8 into hapijs:master Sep 25, 2015
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@rogatty rogatty deleted the rogatty:before-redirect branch Oct 3, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.