Fix fetchHttpClient issue with redirects (#21051)

* Fix fetchHttpClient issue with redirects Browsers expect to handle redirects automatically when using fetch, much like XMLHttpRequest's default behavior. The fact that fetch() lets you set a "manual" mode, isn't intended for common use, but rather for a specific Service Worker scenario. The response sent back is heavily redacted and contains neither the true status or any HTTP headers from the response.
Azure · Mar 28, 2022 · ff6f0ac · ff6f0ac
1 parent fe4bd93
commit ff6f0ac
Show file tree

Hide file tree

Showing 4 changed files with 13 additions and 2 deletions.
diff --git a/sdk/core/core-rest-pipeline/CHANGELOG.md b/sdk/core/core-rest-pipeline/CHANGELOG.md
@@ -11,6 +11,7 @@
 ### Bugs Fixed
 
 - Updated `redirectPolicy` to remove the `Authorization` header from redirected requests. [#21026](https://github.com/Azure/azure-sdk-for-js/pull/21026)
+- Fixed an issue introduced in 1.6.0 where redirects were not properly followed in the browser. [#21051](https://github.com/Azure/azure-sdk-for-js/pull/21051)
 
 ### Other Changes
 

diff --git a/sdk/core/core-rest-pipeline/src/createPipelineFromOptions.ts b/sdk/core/core-rest-pipeline/src/createPipelineFromOptions.ts
@@ -69,7 +69,11 @@ export function createPipelineFromOptions(options: InternalPipelineOptions): Pip
   pipeline.addPolicy(setClientRequestIdPolicy());
   pipeline.addPolicy(defaultRetryPolicy(options.retryOptions), { phase: "Retry" });
   pipeline.addPolicy(tracingPolicy(options.userAgentOptions), { afterPhase: "Retry" });
-  pipeline.addPolicy(redirectPolicy(options.redirectOptions), { afterPhase: "Retry" });
+  if (isNode) {
+    // Both XHR and Fetch expect to handle redirects automatically,
+    // so only include this policy when we're in Node.
+    pipeline.addPolicy(redirectPolicy(options.redirectOptions), { afterPhase: "Retry" });
+  }
   pipeline.addPolicy(logPolicy(options.loggingOptions), { afterPhase: "Retry" });
 
   return pipeline;

diff --git a/sdk/core/core-rest-pipeline/src/fetchHttpClient.ts b/sdk/core/core-rest-pipeline/src/fetchHttpClient.ts
@@ -69,13 +69,18 @@ async function makeRequest(request: PipelineRequest): Promise<PipelineResponse>
     const headers = buildFetchHeaders(request.headers);
     const requestBody = buildRequestBody(request);
 
+    /**
+     * Developers of the future:
+     * Do not set redirect: "manual" as part
+     * of request options.
+     * It will not work as you expect.
+     */
     const response = await fetch(request.url, {
       body: requestBody,
       method: request.method,
       headers: headers,
       signal: abortController.signal,
       credentials: request.withCredentials ? "include" : "same-origin",
-      redirect: "manual",
       cache: "no-store",
     });
     return buildPipelineResponse(response, request);

diff --git a/sdk/core/core-rest-pipeline/src/policies/redirectPolicy.ts b/sdk/core/core-rest-pipeline/src/policies/redirectPolicy.ts
@@ -29,6 +29,7 @@ export interface RedirectPolicyOptions {
 /**
  * A policy to follow Location headers from the server in order
  * to support server-side redirection.
+ * In the browser, this policy is not used.
  * @param options - Options to control policy behavior.
  */
 export function redirectPolicy(options: RedirectPolicyOptions = {}): PipelinePolicy {