Pass along request object to all HTTP interceptors (#47258) (#48697)

* Pass along request object to all HTTP interceptors

* Do not trigger response interceptors from request errors; make request readonly

* Update core API

* Fix failing test

* Add tests to ensure that interceptors accumulate request and response across calls

* Make request readonly for request error interception, simplify response interception return types

* Update docs from request and response interception API change

* Add missing InterceptedHttpResponse generated docs

docs/development/core/public/kibana-plugin-public.httperrorresponse.body.md

@@ -1,11 +0,0 @@
-<!-- Do not edit this file. It is automatically generated by API Documenter. -->
-
-[Home](./index.md) &gt; [kibana-plugin-public](./kibana-plugin-public.md) &gt; [HttpErrorResponse](./kibana-plugin-public.httperrorresponse.md) &gt; [body](./kibana-plugin-public.httperrorresponse.body.md)
-
-## HttpErrorResponse.body property
-
-<b>Signature:</b>
-
-```typescript
-body?: HttpBody;
-```

docs/development/core/public/kibana-plugin-public.httperrorresponse.md

@@ -8,15 +8,12 @@
 <b>Signature:</b>
 
 ```typescript
-export interface HttpErrorResponse 
+export interface HttpErrorResponse extends HttpResponse 
 ```
 
 ## Properties
 
 |  Property | Type | Description |
 |  --- | --- | --- |
-|  [body](./kibana-plugin-public.httperrorresponse.body.md) | <code>HttpBody</code> |  |
 |  [error](./kibana-plugin-public.httperrorresponse.error.md) | <code>Error &#124; IHttpFetchError</code> |  |
-|  [request](./kibana-plugin-public.httperrorresponse.request.md) | <code>Request</code> |  |
-|  [response](./kibana-plugin-public.httperrorresponse.response.md) | <code>Response</code> |  |
 

docs/development/core/public/kibana-plugin-public.httperrorresponse.request.md

@@ -1,11 +0,0 @@
-<!-- Do not edit this file. It is automatically generated by API Documenter. -->
-
-[Home](./index.md) &gt; [kibana-plugin-public](./kibana-plugin-public.md) &gt; [HttpErrorResponse](./kibana-plugin-public.httperrorresponse.md) &gt; [request](./kibana-plugin-public.httperrorresponse.request.md)
-
-## HttpErrorResponse.request property
-
-<b>Signature:</b>
-
-```typescript
-request?: Request;
-```

docs/development/core/public/kibana-plugin-public.httperrorresponse.response.md

@@ -1,11 +0,0 @@
-<!-- Do not edit this file. It is automatically generated by API Documenter. -->
-
-[Home](./index.md) &gt; [kibana-plugin-public](./kibana-plugin-public.md) &gt; [HttpErrorResponse](./kibana-plugin-public.httperrorresponse.md) &gt; [response](./kibana-plugin-public.httperrorresponse.response.md)
-
-## HttpErrorResponse.response property
-
-<b>Signature:</b>
-
-```typescript
-response?: Response;
-```

docs/development/core/public/kibana-plugin-public.httpinterceptor.response.md

@@ -9,7 +9,7 @@ Define an interceptor to be executed after a response is received.
 <b>Signature:</b>
 
 ```typescript
-response?(httpResponse: HttpResponse, controller: IHttpInterceptController): Promise<HttpResponse> | HttpResponse | void;
+response?(httpResponse: HttpResponse, controller: IHttpInterceptController): Promise<InterceptedHttpResponse> | InterceptedHttpResponse | void;
 ```
 
 ## Parameters
@@ -21,5 +21,5 @@ response?(httpResponse: HttpResponse, controller: IHttpInterceptController): Pro
 
 <b>Returns:</b>
 
-`Promise<HttpResponse> | HttpResponse | void`
+`Promise<InterceptedHttpResponse> | InterceptedHttpResponse | void`
 

docs/development/core/public/kibana-plugin-public.httpinterceptor.responseerror.md

@@ -9,7 +9,7 @@ Define an interceptor to be executed if a response interceptor throws an error o
 <b>Signature:</b>
 
 ```typescript
-responseError?(httpErrorResponse: HttpErrorResponse, controller: IHttpInterceptController): Promise<HttpResponse> | HttpResponse | void;
+responseError?(httpErrorResponse: HttpErrorResponse, controller: IHttpInterceptController): Promise<InterceptedHttpResponse> | InterceptedHttpResponse | void;
 ```
 
 ## Parameters
@@ -21,5 +21,5 @@ responseError?(httpErrorResponse: HttpErrorResponse, controller: IHttpInterceptC
 
 <b>Returns:</b>
 
-`Promise<HttpResponse> | HttpResponse | void`
+`Promise<InterceptedHttpResponse> | InterceptedHttpResponse | void`
 

docs/development/core/public/kibana-plugin-public.httpresponse.body.md

@@ -1,11 +0,0 @@
-<!-- Do not edit this file. It is automatically generated by API Documenter. -->
-
-[Home](./index.md) &gt; [kibana-plugin-public](./kibana-plugin-public.md) &gt; [HttpResponse](./kibana-plugin-public.httpresponse.md) &gt; [body](./kibana-plugin-public.httpresponse.body.md)
-
-## HttpResponse.body property
-
-<b>Signature:</b>
-
-```typescript
-body?: HttpBody;
-```

docs/development/core/public/kibana-plugin-public.httpresponse.md

@@ -8,14 +8,12 @@
 <b>Signature:</b>
 
 ```typescript
-export interface HttpResponse 
+export interface HttpResponse extends InterceptedHttpResponse 
 ```
 
 ## Properties
 
 |  Property | Type | Description |
 |  --- | --- | --- |
-|  [body](./kibana-plugin-public.httpresponse.body.md) | <code>HttpBody</code> |  |
-|  [request](./kibana-plugin-public.httpresponse.request.md) | <code>Request</code> |  |
-|  [response](./kibana-plugin-public.httpresponse.response.md) | <code>Response</code> |  |
+|  [request](./kibana-plugin-public.httpresponse.request.md) | <code>Readonly&lt;Request&gt;</code> |  |
 

docs/development/core/public/kibana-plugin-public.httpresponse.request.md

@@ -7,5 +7,5 @@
 <b>Signature:</b>
 
 ```typescript
-request?: Request;
+request: Readonly<Request>;
 ```

docs/development/core/public/kibana-plugin-public.httpresponse.response.md

@@ -1,11 +0,0 @@
-<!-- Do not edit this file. It is automatically generated by API Documenter. -->
-
-[Home](./index.md) &gt; [kibana-plugin-public](./kibana-plugin-public.md) &gt; [HttpResponse](./kibana-plugin-public.httpresponse.md) &gt; [response](./kibana-plugin-public.httpresponse.response.md)
-
-## HttpResponse.response property
-
-<b>Signature:</b>
-
-```typescript
-response?: Response;
-```

docs/development/core/public/kibana-plugin-public.interceptedhttpresponse.body.md

@@ -0,0 +1,11 @@
+<!-- Do not edit this file. It is automatically generated by API Documenter. -->
+
+[Home](./index.md) &gt; [kibana-plugin-public](./kibana-plugin-public.md) &gt; [InterceptedHttpResponse](./kibana-plugin-public.interceptedhttpresponse.md) &gt; [body](./kibana-plugin-public.interceptedhttpresponse.body.md)
+
+## InterceptedHttpResponse.body property
+
+<b>Signature:</b>
+
+```typescript
+body?: HttpBody;
+```

docs/development/core/public/kibana-plugin-public.interceptedhttpresponse.md

@@ -0,0 +1,20 @@
+<!-- Do not edit this file. It is automatically generated by API Documenter. -->
+
+[Home](./index.md) &gt; [kibana-plugin-public](./kibana-plugin-public.md) &gt; [InterceptedHttpResponse](./kibana-plugin-public.interceptedhttpresponse.md)
+
+## InterceptedHttpResponse interface
+
+
+<b>Signature:</b>
+
+```typescript
+export interface InterceptedHttpResponse 
+```
+
+## Properties
+
+|  Property | Type | Description |
+|  --- | --- | --- |
+|  [body](./kibana-plugin-public.interceptedhttpresponse.body.md) | <code>HttpBody</code> |  |
+|  [response](./kibana-plugin-public.interceptedhttpresponse.response.md) | <code>Response</code> |  |
+

docs/development/core/public/kibana-plugin-public.interceptedhttpresponse.response.md

@@ -0,0 +1,11 @@
+<!-- Do not edit this file. It is automatically generated by API Documenter. -->
+
+[Home](./index.md) &gt; [kibana-plugin-public](./kibana-plugin-public.md) &gt; [InterceptedHttpResponse](./kibana-plugin-public.interceptedhttpresponse.md) &gt; [response](./kibana-plugin-public.interceptedhttpresponse.response.md)
+
+## InterceptedHttpResponse.response property
+
+<b>Signature:</b>
+
+```typescript
+response?: Response;
+```

docs/development/core/public/kibana-plugin-public.md

@@ -61,6 +61,7 @@ The plugin integrates with the core system via lifecycle events: `setup`<!-- -->
 |  [IContextContainer](./kibana-plugin-public.icontextcontainer.md) | An object that handles registration of context providers and configuring handlers with context. |
 |  [IHttpFetchError](./kibana-plugin-public.ihttpfetcherror.md) |  |
 |  [IHttpInterceptController](./kibana-plugin-public.ihttpinterceptcontroller.md) | Used to halt a request Promise chain in a [HttpInterceptor](./kibana-plugin-public.httpinterceptor.md)<!-- -->. |
+|  [InterceptedHttpResponse](./kibana-plugin-public.interceptedhttpresponse.md) |  |
 |  [LegacyCoreSetup](./kibana-plugin-public.legacycoresetup.md) | Setup interface exposed to the legacy platform via the <code>ui/new_platform</code> module. |
 |  [LegacyCoreStart](./kibana-plugin-public.legacycorestart.md) | Start interface exposed to the legacy platform via the <code>ui/new_platform</code> module. |
 |  [LegacyNavLink](./kibana-plugin-public.legacynavlink.md) |  |

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

@@ -24,6 +24,7 @@ import fetchMock from 'fetch-mock/es5/client';
 import { readFileSync } from 'fs';
 import { join } from 'path';
 import { setup, SetupTap } from '../../../test_utils/public/http_test_setup';
+import { HttpResponse } from './types';
 
 function delay<T>(duration: number) {
   return new Promise<T>(r => setTimeout(r, duration));
@@ -394,12 +395,12 @@ describe('interception', () => {
 
     const unusedSpy = jest.fn();
 
-    http.intercept({ response: unusedSpy });
     http.intercept({
       responseError(response, controller) {
         controller.halt();
       },
     });
+    http.intercept({ response: unusedSpy, responseError: unusedSpy });
 
     http.post('/my/path').then(unusedSpy, unusedSpy);
     await delay(1000);
@@ -416,21 +417,21 @@ describe('interception', () => {
       request: unusedSpy,
       requestError: usedSpy,
       response: unusedSpy,
-      responseError: usedSpy,
+      responseError: unusedSpy,
     });
     http.intercept({
       request() {
         throw new Error('Interception Error');
       },
       response: unusedSpy,
-      responseError: usedSpy,
+      responseError: unusedSpy,
     });
-    http.intercept({ request: usedSpy, response: unusedSpy, responseError: usedSpy });
+    http.intercept({ request: usedSpy, response: unusedSpy, responseError: unusedSpy });
 
     await expect(http.fetch('/my/path')).rejects.toThrow(/Interception Error/);
     expect(fetchMock.called()).toBe(false);
     expect(unusedSpy).toHaveBeenCalledTimes(0);
-    expect(usedSpy).toHaveBeenCalledTimes(5);
+    expect(usedSpy).toHaveBeenCalledTimes(2);
   });
 
   it('should succeed if request throws but caught by interceptor', async () => {
@@ -458,26 +459,76 @@ describe('interception', () => {
     expect(usedSpy).toHaveBeenCalledTimes(4);
   });
 
-  describe('request availability during interception', () => {
-    it('should not be available to responseError when request throws', async () => {
-      expect.assertions(3);
+  it('should accumulate request information', async () => {
+    const routes = ['alpha', 'beta', 'gamma'];
+    const createRequest = jest.fn(
+      (request: Request) => new Request(`/api/${routes.shift()}`, request)
+    );
 
-      let spiedRequest: Request | undefined;
-
-      http.intercept({
-        request() {
-          throw new Error('Internal Server Error');
-        },
-        responseError({ request }) {
-          spiedRequest = request;
-        },
-      });
-
-      await expect(http.fetch('/my/path')).rejects.toThrow();
-      expect(fetchMock.called()).toBe(false);
-      expect(spiedRequest).toBeUndefined();
+    http.intercept({
+      request: createRequest,
+    });
+    http.intercept({
+      requestError(httpErrorRequest) {
+        return httpErrorRequest.request;
+      },
+    });
+    http.intercept({
+      request(request) {
+        throw new Error('Invalid');
+      },
+    });
+    http.intercept({
+      request: createRequest,
+    });
+    http.intercept({
+      request: createRequest,
     });
 
+    await expect(http.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(fetchMock.lastCall()!.request.url).toContain('/api/gamma');
+  });
+
+  it('should accumulate response information', async () => {
+    const bodies = ['alpha', 'beta', 'gamma'];
+    const createResponse = jest.fn((httpResponse: HttpResponse) => ({
+      body: bodies.shift(),
+    }));
+
+    http.intercept({
+      response: createResponse,
+    });
+    http.intercept({
+      response: createResponse,
+    });
+    http.intercept({
+      response(httpResponse) {
+        throw new Error('Invalid');
+      },
+    });
+    http.intercept({
+      responseError({ error, ...httpResponse }) {
+        return httpResponse;
+      },
+    });
+    http.intercept({
+      response: createResponse,
+    });
+
+    await expect(http.fetch('/my/route')).resolves.toEqual('gamma');
+    expect(fetchMock.called()).toBe(true);
+    expect(bodies.length).toBe(0);
+    expect(createResponse.mock.calls[0][0].body).toEqual({ foo: 'bar' });
+    expect(createResponse.mock.calls[1][0].body).toBe('alpha');
+    expect(createResponse.mock.calls[2][0].body).toBe('beta');
+  });
+
+  describe('request availability during interception', () => {
     it('should be available to responseError when response throws', async () => {
       let spiedRequest: Request | undefined;
 
@@ -514,22 +565,6 @@ describe('interception', () => {
       await expect(http.fetch('/my/path')).rejects.toThrow();
       expect(spiedResponse).toBeDefined();
     });
-
-    it('should not be available to responseError when request throws', async () => {
-      let spiedResponse: Response | undefined;
-
-      http.intercept({
-        request() {
-          throw new Error('Internal Server Error');
-        },
-        responseError({ response }) {
-          spiedResponse = response;
-        },
-      });
-
-      await expect(http.fetch('/my/path')).rejects.toThrow();
-      expect(spiedResponse).toBeUndefined();
-    });
   });
 
   it('should actually halt request interceptors in reverse order', async () => {

src/core/public/http/http_setup.ts

@@ -110,15 +110,14 @@ export const setup = (
       (promise, interceptor) =>
         promise.then(
           async (current: Request) => {
+            next = current;
             checkHalt(controller);
 
             if (!interceptor.request) {
               return current;
             }
 
-            next = (await interceptor.request(current, controller)) || current;
-
-            return next;
+            return (await interceptor.request(current, controller)) || current;
           },
           async error => {
             checkHalt(controller, error);
@@ -155,17 +154,21 @@ export const setup = (
       (promise, interceptor) =>
         promise.then(
           async httpResponse => {
+            current = httpResponse;
             checkHalt(controller);
 
             if (!interceptor.response) {
               return httpResponse;
             }
 
-            current = (await interceptor.response(httpResponse, controller)) || httpResponse;
-
-            return current;
+            return {
+              ...httpResponse,
+              ...((await interceptor.response(httpResponse, controller)) || {}),
+            };
           },
           async error => {
+            const request = error.request || (current && current.request);
+
             checkHalt(controller, error);
 
             if (!interceptor.responseError) {
@@ -176,7 +179,7 @@ export const setup = (
               const next = await interceptor.responseError(
                 {
                   error,
-                  request: error.request || (current && current.request),
+                  request,
                   response: error.response || (current && current.response),
                   body: error.body || (current && current.body),
                 },
@@ -189,17 +192,14 @@ export const setup = (
                 throw error;
               }
 
-              return next;
+              return { ...next, request };
             } catch (err) {
               checkHalt(controller, err);
               throw err;
             }
           }
         ),
-      responsePromise.then(httpResponse => {
-        current = httpResponse;
-        return httpResponse;
-      })
+      responsePromise
     );
 
     return finalHttpResponse.body;
@@ -249,18 +249,23 @@ export const setup = (
     // 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(async (resolve, reject) => {
-      try {
-        const value = await interceptResponse(
-          interceptRequest(initialRequest, controller).then(fetcher),
-          controller
-        );
-
-        resolve(value);
-      } catch (err) {
+      function rejectIfNotHalted(err: any) {
         if (!(err instanceof HttpInterceptHaltError)) {
           reject(err);
         }
       }
+
+      try {
+        const request = await interceptRequest(initialRequest, controller);
+
+        try {
+          resolve(await interceptResponse(fetcher(request), controller));
+        } catch (err) {
+          rejectIfNotHalted(err);
+        }
+      } catch (err) {
+        rejectIfNotHalted(err);
+      }
     });
   }
 

src/core/public/http/types.ts

@@ -226,12 +226,16 @@ export type HttpHandler = (path: string, options?: HttpFetchOptions) => Promise<
 export type HttpBody = BodyInit | null | any;
 
 /** @public */
-export interface HttpResponse {
-  request?: Request;
+export interface InterceptedHttpResponse {
   response?: Response;
   body?: HttpBody;
 }
 
+/** @public */
+export interface HttpResponse extends InterceptedHttpResponse {
+  request: Readonly<Request>;
+}
+
 /** @public */
 export interface IHttpFetchError extends Error {
   readonly request: Request;
@@ -248,11 +252,8 @@ export interface IHttpFetchError extends Error {
 }
 
 /** @public */
-export interface HttpErrorResponse {
+export interface HttpErrorResponse extends HttpResponse {
   error: Error | IHttpFetchError;
-  request?: Request;
-  response?: Response;
-  body?: HttpBody;
 }
 /** @public */
 export interface HttpErrorRequest {
@@ -295,7 +296,7 @@ export interface HttpInterceptor {
   response?(
     httpResponse: HttpResponse,
     controller: IHttpInterceptController
-  ): Promise<HttpResponse> | HttpResponse | void;
+  ): Promise<InterceptedHttpResponse> | InterceptedHttpResponse | void;
 
   /**
    * Define an interceptor to be executed if a response interceptor throws an error or returns a rejected Promise.
@@ -305,7 +306,7 @@ export interface HttpInterceptor {
   responseError?(
     httpErrorResponse: HttpErrorResponse,
     controller: IHttpInterceptController
-  ): Promise<HttpResponse> | HttpResponse | void;
+  ): Promise<InterceptedHttpResponse> | InterceptedHttpResponse | void;
 }
 
 /**

src/core/public/index.ts

@@ -113,6 +113,7 @@ export {
   IBasePath,
   IHttpInterceptController,
   IHttpFetchError,
+  InterceptedHttpResponse,
 } from './http';
 
 export {

src/core/public/public.api.md

@@ -426,15 +426,9 @@ export interface HttpErrorRequest {
 }
 
 // @public (undocumented)
-export interface HttpErrorResponse {
-    // (undocumented)
-    body?: HttpBody;
+export interface HttpErrorResponse extends HttpResponse {
     // (undocumented)
     error: Error | IHttpFetchError;
-    // (undocumented)
-    request?: Request;
-    // (undocumented)
-    response?: Response;
 }
 
 // @public
@@ -463,8 +457,8 @@ export interface HttpHeadersInit {
 export interface HttpInterceptor {
     request?(request: Request, controller: IHttpInterceptController): Promise<Request> | Request | void;
     requestError?(httpErrorRequest: HttpErrorRequest, controller: IHttpInterceptController): Promise<Request> | Request | void;
-    response?(httpResponse: HttpResponse, controller: IHttpInterceptController): Promise<HttpResponse> | HttpResponse | void;
-    responseError?(httpErrorResponse: HttpErrorResponse, controller: IHttpInterceptController): Promise<HttpResponse> | HttpResponse | void;
+    response?(httpResponse: HttpResponse, controller: IHttpInterceptController): Promise<InterceptedHttpResponse> | InterceptedHttpResponse | void;
+    responseError?(httpErrorResponse: HttpErrorResponse, controller: IHttpInterceptController): Promise<InterceptedHttpResponse> | InterceptedHttpResponse | void;
 }
 
 // @public
@@ -486,13 +480,9 @@ export interface HttpRequestInit {
 }
 
 // @public (undocumented)
-export interface HttpResponse {
+export interface HttpResponse extends InterceptedHttpResponse {
     // (undocumented)
-    body?: HttpBody;
-    // (undocumented)
-    request?: Request;
-    // (undocumented)
-    response?: Response;
+    request: Readonly<Request>;
 }
 
 // @public (undocumented)
@@ -563,6 +553,14 @@ export interface IHttpInterceptController {
     halted: boolean;
 }
 
+// @public (undocumented)
+export interface InterceptedHttpResponse {
+    // (undocumented)
+    body?: HttpBody;
+    // (undocumented)
+    response?: Response;
+}
+
 // @public
 export type IToasts = Pick<ToastsApi, 'get$' | 'add' | 'remove' | 'addSuccess' | 'addWarning' | 'addDanger' | 'addError'>;