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

[Recorder] Ensure redirected requests are passed to the recorder in Node #21355

Merged
merged 2 commits into from
Apr 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 18 additions & 8 deletions sdk/test-utils/recorder/src/recorder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,21 +79,31 @@ export class Recorder {
* - PipelineRequest -> core-v2 (recorderHttpPolicy calls this method on the request to modify and hit the proxy-tool with appropriate headers.)
*/
private redirectRequest(request: WebResource | PipelineRequest): void {
if (!isLiveMode() && !request.headers.get("x-recording-id")) {
const upstreamUrl = new URL(request.url);
const redirectedUrl = new URL(request.url);
const testProxyUrl = new URL(this.url);

// Sometimes, due to the service returning a redirect or due to the retry policy, redirectRequest
// may be called multiple times. We only want to update the request the second time if the request's
// URL has been changed between calls (this may happen in the case of a redirect, but generally
// not in the case of a retry). Otherwise, we might accidentally update the X-Recording-Upstream-Base-Uri
// header to point to the test proxy instead of the true upstream.
const requestAlreadyRedirected =
upstreamUrl.host === testProxyUrl.host &&
upstreamUrl.port === testProxyUrl.port &&
upstreamUrl.protocol === testProxyUrl.protocol;

if (!isLiveMode() && !requestAlreadyRedirected) {
if (this.recordingId === undefined) {
throw new RecorderError("Recording ID must be defined to redirect a request");
}

request.headers.set("x-recording-id", this.recordingId);
request.headers.set("x-recording-mode", getTestMode());

const upstreamUrl = new URL(request.url);
const redirectedUrl = new URL(request.url);
const providedUrl = new URL(this.url);

redirectedUrl.host = providedUrl.host;
redirectedUrl.port = providedUrl.port;
redirectedUrl.protocol = providedUrl.protocol;
redirectedUrl.host = testProxyUrl.host;
redirectedUrl.port = testProxyUrl.port;
redirectedUrl.protocol = testProxyUrl.protocol;
request.headers.set("x-recording-upstream-base-uri", upstreamUrl.toString());
request.url = redirectedUrl.toString();

Expand Down
28 changes: 17 additions & 11 deletions sdk/test-utils/recorder/test/testProxyClient.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,17 +57,23 @@ describe("TestProxyClient functions", () => {
});

["record", "playback"].forEach((testMode) => {
it(`${testMode} mode: ` + "request unchanged if `x-recording-id` in headers", function () {
env.TEST_MODE = testMode;
testRedirectedRequest(
client,
() => ({
...initialRequest,
headers: createHttpHeaders({ "x-recording-id": "dummy-recording-id" }),
}),
(req) => req
);
});
it(
`${testMode} mode: ` + "request unchanged if request URL already points to test proxy",
function () {
env.TEST_MODE = testMode;
testRedirectedRequest(
client,
() => ({
...initialRequest,
url: "http://localhost:5000/dummy_path?sas=sas",
headers: createHttpHeaders({
"x-recording-upstream-uri": "https://dummy_url.windows.net/dummy_path?sas=sas",
}),
}),
(req) => req
);
}
);

it(
`${testMode} mode: ` + "url and headers get updated if no `x-recording-id` in headers",
Expand Down
41 changes: 41 additions & 0 deletions sdk/test-utils/recorder/test/testProxyTests.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the MIT license.

import { ServiceClient } from "@azure/core-client";
import { isNode } from "@azure/core-util";
import { CustomMatcherOptions, isPlaybackMode, Recorder } from "../src";
import { isLiveMode, TestMode } from "../src/utils/utils";
import { getTestServerUrl, makeRequestAndVerifyResponse, setTestMode } from "./utils/utils";
Expand Down Expand Up @@ -36,6 +37,46 @@ import { getTestServerUrl, makeRequestAndVerifyResponse, setTestMode } from "./u
);
});

it("redirect (redirect location has host)", async function (this: Mocha.Context) {
await recorder.start({ envSetupForPlayback: {} });

if (!isNode) {
// In the browser, redirects get handled by fetch/XHR and we can't guarantee redirect behavior.
this.skip();
}

await makeRequestAndVerifyResponse(
client,
{ path: `/redirectWithHost`, method: "GET" },
{ val: "abc" }
);
});

it("redirect (redirect location is relative)", async function (this: Mocha.Context) {
await recorder.start({ envSetupForPlayback: {} });

if (!isNode) {
// In the browser, redirects get handled by fetch/XHR and we can't guarantee redirect behavior.
this.skip();
}

await makeRequestAndVerifyResponse(
client,
{ path: `/redirectWithoutHost`, method: "GET" },
{ val: "abc" }
);
});

it("retry", async () => {
await recorder.start({ envSetupForPlayback: {} });
await makeRequestAndVerifyResponse(
client,
{ path: "/reset_retry", method: "GET" },
undefined
);
await makeRequestAndVerifyResponse(client, { path: "/retry", method: "GET" }, { val: "abc" });
});

it("sample_response with random string in path", async () => {
await recorder.start({ envSetupForPlayback: {} });

Expand Down
34 changes: 33 additions & 1 deletion sdk/test-utils/recorder/test/utils/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,39 @@ app.get("/", (_, res) => {
res.send("Hello world!");
});

app.get("/sample_response", (_, res) => {
app.get("/redirectWithHost", (req, res) => {
res.redirect(307, `http://${req.hostname}:${port}/sample_response`);
});

app.get("/redirectWithoutHost", (_, res) => {
res.redirect(307, `/sample_response`);
});

let sendRetryResponse = true;

app.get("/reset_retry", (_, res) => {
sendRetryResponse = true;
res.send("The retry flag was reset. The next call to /retry will return a 429 status.");
});

app.get("/retry", (_, res) => {
if (sendRetryResponse) {
res
.status(429)
.header("Retry-After", new Date().toUTCString())
.send({ error: "429 Too Many Requests" });
sendRetryResponse = false;
} else {
res.send({ val: "abc" });
}
});

app.get("/sample_response", (req, res) => {
if (req.header("x-recording-id") !== undefined) {
res.status(400).send({ error: "This request bypassed the proxy tool!" });
return;
}

res.send({ val: "abc" });
});

Expand Down