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

Author: timovv

sdk/test-utils/recorder/src/recorder.ts

@@ -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();
 

sdk/test-utils/recorder/test/testProxyClient.spec.ts

@@ -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",

sdk/test-utils/recorder/test/testProxyTests.spec.ts

@@ -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";
@@ -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: {} });
 

sdk/test-utils/recorder/test/utils/server.ts

@@ -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" });
 });