Skip to content

Commit

Permalink
Adding support for temporary redirect (microsoft#768)
Browse files Browse the repository at this point in the history 
* Adding support for temporary redirect

* WIP

* Remove unused variable

* Adding nock scope restore

* WIP

* Adding logs in tests

* Fixing tests
  • Loading branch information
@hectorhdzg
hectorhdzg authored Jun 9, 2021
1 parent b3e871b commit 5741272
Show file tree
Hide file tree
Showing 4 changed files with 113 additions and 52 deletions.
48 changes: 25 additions & 23 deletions Library/Sender.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -190,24 +190,35 @@ class Sender {
}
}
// Redirect handling
if (res.statusCode === 308) { // Permanent Redirect
// Try to get redirect header
const locationHeader = res.headers["location"] ? res.headers["location"].toString() : null;
if (locationHeader) {
this._handleRedirect(locationHeader);
if (res.statusCode === 307 || // Temporary Redirect
res.statusCode === 308) { // Permanent Redirect
this._numConsecutiveRedirects++;
// To prevent circular redirects
if (this._numConsecutiveRedirects < 10) {
// Try to get redirect header
const locationHeader = res.headers["location"] ? res.headers["location"].toString() : null;
if (locationHeader) {
this._redirectedHost = locationHeader;
// Send to redirect endpoint as HTTPs library doesn't handle redirect automatically
this.send(envelopes, callback);
}
}
else {
if (typeof callback === "function") {
callback("Error sending telemetry because of circular redirects.");
}
}

}
else {
this._numConsecutiveRedirects = 0;
}

Logging.info(Sender.TAG, responseString);
if (typeof this._onSuccess === "function") {
this._onSuccess(responseString);
}

if (typeof callback === "function") {
callback(responseString);
if (typeof callback === "function") {
callback(responseString);
}
Logging.info(Sender.TAG, responseString);
if (typeof this._onSuccess === "function") {
this._onSuccess(responseString);
}
}
});
};
Expand Down Expand Up @@ -260,7 +271,6 @@ class Sender {
private _isRetriable(statusCode: number) {
return (
statusCode === 206 || // Retriable
statusCode === 308 || // Permanent Redirect
statusCode === 408 || // Timeout
statusCode === 429 || // Throttle
statusCode === 439 || // Quota
Expand All @@ -269,14 +279,6 @@ class Sender {
);
}

private _handleRedirect(location: string) {
this._numConsecutiveRedirects++;
// To prevent circular redirects
if (this._numConsecutiveRedirects < 10) {
this._redirectedHost = location;
}
}

private _runICACLS(args: string[], callback: (err: Error) => void) {
var aclProc = child_process.spawn(Sender.ICACLS_PATH, args, <any>{ windowsHide: true });
aclProc.on("error", (e: Error) => callback(e));
Expand Down
93 changes: 76 additions & 17 deletions Tests/Library/Sender.tests.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ describe("Library/Sender", () => {
var testEnvelope = new Contracts.Envelope();
var sandbox: sinon.SinonSandbox;
let interceptor: nock.Interceptor;
let nockScope: nock.Scope;

before(() => {
interceptor = nock(Constants.DEFAULT_BREEZE_ENDPOINT)
Expand All @@ -36,6 +37,9 @@ describe("Library/Sender", () => {

afterEach(() => {
sandbox.restore();
if (nockScope && nockScope.restore) {
nockScope.restore();
}
});

after(() => {
Expand Down Expand Up @@ -75,7 +79,8 @@ describe("Library/Sender", () => {
diskEnvelope.name = "DiskEnvelope";
sender["_storeToDisk"]([diskEnvelope]);
var sendSpy = sandbox.spy(sender, "send");
interceptor.reply(200, breezeResponse).persist();
nockScope = interceptor.reply(200, breezeResponse);
nockScope.persist();
sender["_resendInterval"] = 100;
sender.send([testEnvelope], (responseText) => {
// Wait for resend timer
Expand All @@ -91,7 +96,7 @@ describe("Library/Sender", () => {
it("should put telemetry in disk when retryable code is returned", (done) => {
var envelope = new Contracts.Envelope();
envelope.name = "TestRetryable";
interceptor.reply(408, null);
nockScope = interceptor.reply(408, null);
var storeStub = sandbox.stub(sender, "_storeToDisk");
sender.send([envelope], (responseText) => {
assert.ok(storeStub.calledOnce);
Expand Down Expand Up @@ -120,7 +125,7 @@ describe("Library/Sender", () => {
newEnvelope.name = "TestPartial" + i;
envelopes.push(newEnvelope);
}
interceptor.reply(206, breezeResponse);
nockScope = interceptor.reply(206, breezeResponse);
var storeStub = sandbox.stub(sender, "_storeToDisk");
sender.send(envelopes, () => {
assert.ok(storeStub.calledOnce);
Expand All @@ -137,7 +142,7 @@ describe("Library/Sender", () => {
sender = new SenderMock(new Config("1aa11111-bbbb-1ccc-8ddd-eeeeffff3333"));
});

after(()=>{
after(() => {
sender.setDiskRetryMode(false);
});

Expand Down Expand Up @@ -165,19 +170,47 @@ describe("Library/Sender", () => {

describe("#endpoint redirect", () => {
it("should change ingestion endpoint when redirect response code is returned (308)", (done) => {
interceptor.reply(308, {}, { "Location": "testLocation" });
let redirectHost = "https://test";
let redirectLocation = redirectHost + "/v2.1/track";
// Fake redirect endpoint
let redirectInterceptor = nock(redirectHost)
.post("/v2.1/track", (body: string) => {
return true;
});
redirectInterceptor.reply(200, {});

nockScope = interceptor.reply(308, {}, { "Location": redirectLocation });
var testSender = new Sender(new Config("2bb22222-bbbb-1ccc-8ddd-eeeeffff3333"));
testSender.setDiskRetryMode(true);
var storeStub = sandbox.stub(testSender, "_storeToDisk");
var sendSpy = sandbox.spy(testSender, "send");
testSender.send([testEnvelope], (responseText) => {
assert.equal(testSender["_redirectedHost"], "testLocation");
assert.ok(storeStub.calledOnce);
assert.equal(testSender["_redirectedHost"], redirectLocation);
assert.ok(sendSpy.callCount === 2); // Original and redirect calls
done();
});
});

it("should change ingestion endpoint when temporary redirect response code is returned (307)", (done) => {
let redirectHost = "https://test";
let redirectLocation = redirectHost + "/v2.1/track";
// Fake redirect endpoint
let redirectInterceptor = nock(redirectHost)
.post("/v2.1/track", (body: string) => {
return true;
});
redirectInterceptor.reply(200, {});

nockScope = interceptor.reply(307, {}, { "Location": redirectLocation });
var testSender = new Sender(new Config("2bb22222-bbbb-1ccc-8ddd-eeeeffff3333"));
var sendSpy = sandbox.spy(testSender, "send");
testSender.send([testEnvelope], (responseText) => {
assert.equal(testSender["_redirectedHost"], redirectLocation);
assert.ok(sendSpy.callCount === 2); // Original and redirect calls
done();
});
});

it("should not change ingestion endpoint if redirect is not triggered", (done) => {
interceptor.reply(200, {}, { "Location": "testLocation" });
nockScope = interceptor.reply(200, {}, { "Location": "testLocation" });
var testSender = new Sender(new Config("2bb22222-bbbb-1ccc-8ddd-eeeeffff3333"));
testSender.send([testEnvelope], (responseText) => {
assert.equal(testSender["_redirectedHost"], null);
Expand All @@ -186,24 +219,50 @@ describe("Library/Sender", () => {
});

it("should use redirect URL for following requests", (done) => {
let redirectHost = "https://testLocation";
let redirectHost = "https://testlocation";
let redirectLocation = redirectHost + "/v2.1/track";
// Fake redirect endpoint
nock(redirectHost)
let redirectInterceptor = nock(redirectHost)
.post("/v2.1/track", (body: string) => {
return true;
}).reply(200, { "redirectProperty": true });
interceptor.reply(308, {}, { "Location": redirectLocation });
});

redirectInterceptor.reply(200, { "redirectProperty": true }).persist();

nockScope = interceptor.reply(308, {}, { "Location": redirectLocation });
var testSender = new Sender(new Config("2bb22222-bbbb-1ccc-8ddd-eeeeffff3333"));
testSender.send([testEnvelope], () => {
var sendSpy = sandbox.spy(testSender, "send");
testSender.send([testEnvelope], (resposneText) => {
assert.equal(testSender["_redirectedHost"], redirectLocation);
testSender.send([testEnvelope], (responseText) => {
assert.equal(responseText, '{"redirectProperty":true}');
assert.equal(resposneText, '{"redirectProperty":true}');
assert.ok(sendSpy.calledTwice);
testSender.send([testEnvelope], (secondResponseText) => {
assert.equal(secondResponseText, '{"redirectProperty":true}');
assert.ok(sendSpy.calledThrice);
done();
});
});
});

it("should stop redirecting when circular redirect is triggered", (done) => {
let redirectHost = "https://circularredirect";
// Fake redirect endpoint
let redirectInterceptor = nock(redirectHost)
.post("/v2.1/track", (body: string) => {
return true;
});
redirectInterceptor.reply(307, {}, { "Location": Constants.DEFAULT_BREEZE_ENDPOINT + "/v2.1/track" }).persist();

nockScope = interceptor.reply(307, {}, { "Location": redirectHost + "/v2.1/track" });
var testSender = new Sender(new Config("2bb22222-bbbb-1ccc-8ddd-eeeeffff3333"));
var sendSpy = sandbox.spy(testSender, "send");
testSender.send([testEnvelope], (responseText) => {
assert.equal(responseText, "Error sending telemetry because of circular redirects.");
assert.equal(sendSpy.callCount, 10);
done();
});
});

});

describe("#fileCleanupTask", () => {
Expand Down
22 changes: 11 additions & 11 deletions package-lock.json
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@
"typescript": "4.1.2"
},
"dependencies": {
"@azure/core-http": "^1.2.4",
"@azure/core-http": "^1.2.5",
"@opentelemetry/api": "^0.18.1",
"@opentelemetry/tracing": "^0.19.0",
"cls-hooked": "^4.2.2",
Expand Down

0 comments on commit 5741272

Please sign in to comment.