Refactor request interceptors to modify HttpFetchOptions

src/core/public/http/fetch.test.ts

@@ -24,7 +24,7 @@ import { join } from 'path';
 
 import { Fetch } from './fetch';
 import { BasePath } from './base_path';
-import { IHttpResponse } from './types';
+import { IHttpResponse, HttpFetchOptionsWithPath } from './types';
 
 function delay<T>(duration: number) {
   return new Promise<T>(r => setTimeout(r, duration));
@@ -301,13 +301,18 @@ describe('Fetch', () => {
 
     it('should be able to manipulate request instance', async () => {
       fetchInstance.intercept({
-        request(request) {
-          request.headers.set('Content-Type', 'CustomContentType');
+        request(options) {
+          return {
+            headers: {
+              ...options.headers,
+              'Content-Type': 'CustomContentType',
+            },
+          };
         },
       });
       fetchInstance.intercept({
-        request(request) {
-          return new Request('/my/route', request);
+        request() {
+          return { path: '/my/route' };
         },
       });
 
@@ -318,7 +323,7 @@ describe('Fetch', () => {
       expect(fetchMock.lastOptions()!.headers).toMatchObject({
         'content-type': 'CustomContentType',
       });
-      expect(fetchMock.lastUrl()).toBe('/my/route');
+      expect(fetchMock.lastUrl()).toBe('http://localhost/myBase/my/route');
     });
 
     it('should call interceptors in correct order', async () => {
@@ -458,8 +463,8 @@ describe('Fetch', () => {
 
       fetchInstance.intercept({
         request: unusedSpy,
-        requestError({ request }) {
-          return new Request('/my/route', request);
+        requestError() {
+          return { path: '/my/route' };
         },
         response: usedSpy,
       });
@@ -479,16 +484,16 @@ describe('Fetch', () => {
 
     it('should accumulate request information', async () => {
       const routes = ['alpha', 'beta', 'gamma'];
-      const createRequest = jest.fn(
-        (request: Request) => new Request(`/api/${routes.shift()}`, request)
-      );
+      const createRequest = jest.fn((options: HttpFetchOptionsWithPath) => ({
+        path: `/api/${routes.shift()}`,
+      }));
 
       fetchInstance.intercept({
         request: createRequest,
       });
       fetchInstance.intercept({
         requestError(httpErrorRequest) {
-          return httpErrorRequest.request;
+          return httpErrorRequest.fetchOptions;
         },
       });
       fetchInstance.intercept({
@@ -506,9 +511,9 @@ describe('Fetch', () => {
       await expect(fetchInstance.fetch('/my/route')).resolves.toEqual({ foo: 'bar' });
       expect(fetchMock.called()).toBe(true);
       expect(routes.length).toBe(0);
-      expect(createRequest.mock.calls[0][0].url).toContain('/my/route');
-      expect(createRequest.mock.calls[1][0].url).toContain('/api/alpha');
-      expect(createRequest.mock.calls[2][0].url).toContain('/api/beta');
+      expect(createRequest.mock.calls[0][0].path).toContain('/my/route');
+      expect(createRequest.mock.calls[1][0].path).toContain('/api/alpha');
+      expect(createRequest.mock.calls[2][0].path).toContain('/api/beta');
       expect(fetchMock.lastCall()!.request.url).toContain('/api/gamma');
     });
 
@@ -606,7 +611,7 @@ describe('Fetch', () => {
 
       fetchInstance.intercept({
         requestError(httpErrorRequest) {
-          return httpErrorRequest.request;
+          return httpErrorRequest.fetchOptions;
         },
         response: usedSpy,
       });

src/core/public/http/fetch.ts

@@ -20,10 +20,16 @@
 import { merge } from 'lodash';
 import { format } from 'url';
 
-import { IBasePath, HttpInterceptor, HttpHandler, HttpFetchOptions, IHttpResponse } from './types';
+import {
+  IBasePath,
+  HttpInterceptor,
+  HttpHandler,
+  HttpFetchOptions,
+  IHttpResponse,
+  HttpFetchOptionsWithPath,
+} from './types';
 import { HttpFetchError } from './http_fetch_error';
 import { HttpInterceptController } from './http_intercept_controller';
-import { HttpResponse } from './response';
 import { interceptRequest, interceptResponse } from './intercept';
 import { HttpInterceptHaltError } from './http_intercept_halt_error';
 
@@ -60,23 +66,26 @@ export class Fetch {
   public readonly put = this.shorthand('PUT');
 
   public fetch: HttpHandler = async <TResponseBody>(
-    path: string,
+    pathOrOptions: string | HttpFetchOptionsWithPath,
     options: HttpFetchOptions = {}
   ) => {
-    const initialRequest = this.createRequest(path, options);
+    const optionsWithPath: HttpFetchOptionsWithPath =
+      typeof pathOrOptions === 'string' ? { ...options, path: pathOrOptions } : pathOrOptions;
+
     const controller = new HttpInterceptController();
 
     // We wrap the interception in a separate promise to ensure that when
     // a halt is called we do not resolve or reject, halting handling of the promise.
     return new Promise<TResponseBody | IHttpResponse<TResponseBody>>(async (resolve, reject) => {
       try {
-        const interceptedRequest = await interceptRequest(
-          initialRequest,
+        const interceptedOptions = await interceptRequest(
+          optionsWithPath,
           this.interceptors,
           controller
         );
-        const initialResponse = this.fetchResponse(interceptedRequest);
+        const initialResponse = this.fetchResponse(interceptedOptions);
         const interceptedResponse = await interceptResponse(
+          interceptedOptions,
           initialResponse,
           this.interceptors,
           controller
@@ -95,9 +104,15 @@ export class Fetch {
     });
   };
 
-  private createRequest(path: string, options: HttpFetchOptions = {}): Request {
+  private createRequest(options: HttpFetchOptionsWithPath): Request {
     // Merge and destructure options out that are not applicable to the Fetch API.
-    const { query, prependBasePath: shouldPrependBasePath, asResponse, ...fetchOptions } = merge(
+    const {
+      query,
+      prependBasePath: shouldPrependBasePath,
+      asResponse,
+      asSystemApi,
+      ...fetchOptions
+    } = merge(
       {
         method: 'GET',
         credentials: 'same-origin',
@@ -114,14 +129,15 @@ export class Fetch {
       }
     );
     const url = format({
-      pathname: shouldPrependBasePath ? this.params.basePath.prepend(path) : path,
+      pathname: shouldPrependBasePath ? this.params.basePath.prepend(options.path) : options.path,
       query,
     });
 
     return new Request(url, fetchOptions);
   }
 
-  private async fetchResponse(request: Request) {
+  private async fetchResponse(fetchOptions: HttpFetchOptionsWithPath): Promise<IHttpResponse<any>> {
+    const request = this.createRequest(fetchOptions);
     let response: Response;
     let body = null;
 
@@ -155,7 +171,7 @@ export class Fetch {
       throw new HttpFetchError(response.statusText, request, response, body);
     }
 
-    return new HttpResponse({ request, response, body });
+    return { fetchOptions, request, response, body };
   }
 
   private shorthand(method: string) {

src/core/public/http/intercept.ts

@@ -19,28 +19,31 @@
 
 import { HttpInterceptController } from './http_intercept_controller';
 import { HttpInterceptHaltError } from './http_intercept_halt_error';
-import { HttpInterceptor, IHttpResponse } from './types';
-import { HttpResponse } from './response';
+import { HttpInterceptor, IHttpResponse, HttpFetchOptionsWithPath } from './types';
 
 export async function interceptRequest(
-  request: Request,
+  options: HttpFetchOptionsWithPath,
   interceptors: ReadonlySet<HttpInterceptor>,
   controller: HttpInterceptController
-): Promise<Request> {
-  let next = request;
+): Promise<HttpFetchOptionsWithPath> {
+  let current: HttpFetchOptionsWithPath;
 
   return [...interceptors].reduceRight(
     (promise, interceptor) =>
       promise.then(
-        async (current: Request) => {
-          next = current;
+        async fetchOptions => {
+          current = fetchOptions;
           checkHalt(controller);
 
           if (!interceptor.request) {
-            return current;
+            return fetchOptions;
           }
 
-          return (await interceptor.request(current, controller)) || current;
+          const overrides = await interceptor.request(current, controller);
+          return {
+            ...current,
+            ...overrides,
+          };
         },
         async error => {
           checkHalt(controller, error);
@@ -49,21 +52,28 @@ export async function interceptRequest(
             throw error;
           }
 
-          const nextRequest = await interceptor.requestError({ error, request: next }, controller);
+          const overrides = await interceptor.requestError(
+            { error, fetchOptions: current },
+            controller
+          );
 
-          if (!nextRequest) {
+          if (!overrides) {
             throw error;
           }
 
-          next = nextRequest;
-          return next;
+          current = {
+            ...current,
+            ...overrides,
+          };
+          return current;
         }
       ),
-    Promise.resolve(request)
+    Promise.resolve(options)
   );
 }
 
 export async function interceptResponse(
+  fetchOptions: HttpFetchOptionsWithPath,
   responsePromise: Promise<IHttpResponse>,
   interceptors: ReadonlySet<HttpInterceptor>,
   controller: HttpInterceptController
@@ -83,10 +93,10 @@ export async function interceptResponse(
 
           const interceptorOverrides = (await interceptor.response(httpResponse, controller)) || {};
 
-          return new HttpResponse({
+          return {
             ...httpResponse,
             ...interceptorOverrides,
-          });
+          };
         },
         async error => {
           const request = error.request || (current && current.request);
@@ -101,6 +111,7 @@ export async function interceptResponse(
             const next = await interceptor.responseError(
               {
                 error,
+                fetchOptions,
                 request,
                 response: error.response || (current && current.response),
                 body: error.body || (current && current.body),
@@ -114,7 +125,7 @@ export async function interceptResponse(
               throw error;
             }
 
-            return new HttpResponse({ ...next, request });
+            return { ...next, request, fetchOptions };
           } catch (err) {
             checkHalt(controller, err);
             throw err;