[7.x] Make request and response properties conditionally avail… (#46725)

* Make request and response properties conditionally available during HTTP interception

* Address review comments, fix request reference in http service tests

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

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

docs/development/core/public/kibana-plugin-public.httperrorresponse.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; [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,12 +8,15 @@
 <b>Signature:</b>
 
 ```typescript
-export interface HttpErrorResponse extends HttpResponse 
+export interface HttpErrorResponse 
 ```
 
 ## Properties
 
 |  Property | Type | Description |
 |  --- | --- | --- |
+|  [body](./kibana-plugin-public.httperrorresponse.body.md) | <code>HttpBody</code> |  |
 |  [error](./kibana-plugin-public.httperrorresponse.error.md) | <code>Error &#124; HttpFetchError</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

@@ -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; [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

@@ -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; [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.httpinterceptcontroller.halt.md

@@ -0,0 +1,15 @@
+<!-- Do not edit this file. It is automatically generated by API Documenter. -->
+
+[Home](./index.md) &gt; [kibana-plugin-public](./kibana-plugin-public.md) &gt; [HttpInterceptController](./kibana-plugin-public.httpinterceptcontroller.md) &gt; [halt](./kibana-plugin-public.httpinterceptcontroller.halt.md)
+
+## HttpInterceptController.halt() method
+
+<b>Signature:</b>
+
+```typescript
+halt(): void;
+```
+<b>Returns:</b>
+
+`void`
+

docs/development/core/public/kibana-plugin-public.httpinterceptcontroller.halted.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; [HttpInterceptController](./kibana-plugin-public.httpinterceptcontroller.md) &gt; [halted](./kibana-plugin-public.httpinterceptcontroller.halted.md)
+
+## HttpInterceptController.halted property
+
+<b>Signature:</b>
+
+```typescript
+readonly halted: boolean;
+```

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

@@ -0,0 +1,24 @@
+<!-- Do not edit this file. It is automatically generated by API Documenter. -->
+
+[Home](./index.md) &gt; [kibana-plugin-public](./kibana-plugin-public.md) &gt; [HttpInterceptController](./kibana-plugin-public.httpinterceptcontroller.md)
+
+## HttpInterceptController class
+
+<b>Signature:</b>
+
+```typescript
+export declare class HttpInterceptController 
+```
+
+## Properties
+
+|  Property | Modifiers | Type | Description |
+|  --- | --- | --- | --- |
+|  [halted](./kibana-plugin-public.httpinterceptcontroller.halted.md) |  | <code>boolean</code> |  |
+
+## Methods
+
+|  Method | Modifiers | Description |
+|  --- | --- | --- |
+|  [halt()](./kibana-plugin-public.httpinterceptcontroller.halt.md) |  |  |
+

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

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

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

@@ -14,6 +14,7 @@ The plugin integrates with the core system via lifecycle events: `setup`<!-- -->
 
 |  Class | Description |
 |  --- | --- |
+|  [HttpInterceptController](./kibana-plugin-public.httpinterceptcontroller.md) |  |
 |  [SavedObjectsClient](./kibana-plugin-public.savedobjectsclient.md) | Saved Objects is Kibana's data persisentence mechanism allowing plugins to use Elasticsearch for storing plugin state. The client-side SavedObjectsClient is a thin convenience library around the SavedObjects HTTP API for interacting with Saved Objects. |
 |  [SimpleSavedObject](./kibana-plugin-public.simplesavedobject.md) | This class is a very simple wrapper for SavedObjects loaded from the server with the [SavedObjectsClient](./kibana-plugin-public.savedobjectsclient.md)<!-- -->.<!-- -->It provides basic functionality for creating/saving/deleting saved objects, but doesn't include any type-specific implementations. |
 |  [ToastsApi](./kibana-plugin-public.toastsapi.md) |  |

src/core/public/http/http_fetch_error.ts

@@ -18,7 +18,12 @@
  */
 
 export class HttpFetchError extends Error {
-  constructor(message: string, public readonly response?: Response, public readonly body?: any) {
+  constructor(
+    message: string,
+    public readonly request: Request,
+    public readonly response?: Response,
+    public readonly body?: any
+  ) {
     super(message);
 
     // captureStackTrace is only available in the V8 engine, so any browser using

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

@@ -25,6 +25,10 @@ import { readFileSync } from 'fs';
 import { join } from 'path';
 import { setup, SetupTap } from '../../../test_utils/public/http_test_setup';
 
+function delay<T>(duration: number) {
+  return new Promise<T>(r => setTimeout(r, duration));
+}
+
 const setupFakeBasePath: SetupTap = injectedMetadata => {
   injectedMetadata.getBasePath.mockReturnValue('/foo/bar');
 };
@@ -341,169 +345,228 @@ describe('interception', () => {
   });
 
   it('should skip remaining interceptors when controller halts during request', async () => {
-    const order: string[] = [];
+    const usedSpy = jest.fn();
+    const unusedSpy = jest.fn();
 
-    http.intercept({
-      request() {
-        order.push('Request 1');
-      },
-      response() {
-        order.push('Response 1');
-      },
-    });
+    http.intercept({ request: unusedSpy, response: unusedSpy });
     http.intercept({
       request(request, controller) {
         controller.halt();
-        order.push('Request 2');
-      },
-      response() {
-        order.push('Response 2');
       },
+      response: unusedSpy,
     });
     http.intercept({
-      request() {
-        order.push('Request 3');
-      },
-      response() {
-        order.push('Response 3');
-      },
+      request: usedSpy,
+      response: unusedSpy,
     });
 
-    await expect(http.fetch('/my/wat')).rejects.toThrow(/HTTP Intercept Halt/);
+    http.fetch('/my/path').then(unusedSpy, unusedSpy);
+    await delay(1000);
+
+    expect(unusedSpy).toHaveBeenCalledTimes(0);
+    expect(usedSpy).toHaveBeenCalledTimes(1);
     expect(fetchMock.called()).toBe(false);
-    expect(order).toEqual(['Request 3', 'Request 2']);
   });
 
   it('should skip remaining interceptors when controller halts during response', async () => {
-    const order: string[] = [];
+    const usedSpy = jest.fn();
+    const unusedSpy = jest.fn();
 
     http.intercept({
-      request() {
-        order.push('Request 1');
-      },
+      request: usedSpy,
       response(response, controller) {
         controller.halt();
-        order.push('Response 1');
       },
     });
+    http.intercept({ request: usedSpy, response: unusedSpy });
+    http.intercept({ request: usedSpy, response: unusedSpy });
+
+    http.fetch('/my/path').then(unusedSpy, unusedSpy);
+    await delay(1000);
+
+    expect(fetchMock.called()).toBe(true);
+    expect(usedSpy).toHaveBeenCalledTimes(3);
+    expect(unusedSpy).toHaveBeenCalledTimes(0);
+  });
+
+  it('should skip remaining interceptors when controller halts during responseError', async () => {
+    fetchMock.post('*', 401);
+
+    const unusedSpy = jest.fn();
+
+    http.intercept({ response: unusedSpy });
     http.intercept({
-      request() {
-        order.push('Request 2');
-      },
-      response() {
-        order.push('Response 2');
-      },
-    });
-    http.intercept({
-      request() {
-        order.push('Request 3');
-      },
-      response() {
-        order.push('Response 3');
+      responseError(response, controller) {
+        controller.halt();
       },
     });
 
-    await expect(http.fetch('/my/wat')).rejects.toThrow(/HTTP Intercept Halt/);
+    http.post('/my/path').then(unusedSpy, unusedSpy);
+    await delay(1000);
+
     expect(fetchMock.called()).toBe(true);
-    expect(order).toEqual(['Request 3', 'Request 2', 'Request 1', 'Response 1']);
+    expect(unusedSpy).toHaveBeenCalledTimes(0);
   });
 
   it('should not fetch if exception occurs during request interception', async () => {
-    const order: string[] = [];
+    const usedSpy = jest.fn();
+    const unusedSpy = jest.fn();
 
     http.intercept({
-      request() {
-        order.push('Request 1');
-      },
-      requestError() {
-        order.push('RequestError 1');
-      },
-      response() {
-        order.push('Response 1');
-      },
-      responseError() {
-        order.push('ResponseError 1');
-      },
+      request: unusedSpy,
+      requestError: usedSpy,
+      response: unusedSpy,
+      responseError: usedSpy,
     });
     http.intercept({
       request() {
-        order.push('Request 2');
         throw new Error('Interception Error');
       },
-      response() {
-        order.push('Response 2');
-      },
-      responseError() {
-        order.push('ResponseError 2');
-      },
-    });
-    http.intercept({
-      request() {
-        order.push('Request 3');
-      },
-      response() {
-        order.push('Response 3');
-      },
-      responseError() {
-        order.push('ResponseError 3');
-      },
+      response: unusedSpy,
+      responseError: usedSpy,
     });
+    http.intercept({ request: usedSpy, response: unusedSpy, responseError: usedSpy });
 
-    await expect(http.fetch('/my/wat')).rejects.toThrow(/Interception Error/);
+    await expect(http.fetch('/my/path')).rejects.toThrow(/Interception Error/);
     expect(fetchMock.called()).toBe(false);
-    expect(order).toEqual([
-      'Request 3',
-      'Request 2',
-      'RequestError 1',
-      'ResponseError 1',
-      'ResponseError 2',
-      'ResponseError 3',
-    ]);
+    expect(unusedSpy).toHaveBeenCalledTimes(0);
+    expect(usedSpy).toHaveBeenCalledTimes(5);
   });
 
   it('should succeed if request throws but caught by interceptor', async () => {
-    const order: string[] = [];
+    const usedSpy = jest.fn();
+    const unusedSpy = jest.fn();
 
     http.intercept({
-      request() {
-        order.push('Request 1');
-      },
+      request: unusedSpy,
       requestError({ request }) {
-        order.push('RequestError 1');
         return new Request('/my/route', request);
       },
-      response() {
-        order.push('Response 1');
-      },
+      response: usedSpy,
     });
     http.intercept({
       request() {
-        order.push('Request 2');
         throw new Error('Interception Error');
       },
-      response() {
-        order.push('Response 2');
-      },
-    });
-    http.intercept({
-      request() {
-        order.push('Request 3');
-      },
-      response() {
-        order.push('Response 3');
-      },
+      response: usedSpy,
     });
+    http.intercept({ request: usedSpy, response: usedSpy });
 
     await expect(http.fetch('/my/route')).resolves.toEqual({ foo: 'bar' });
     expect(fetchMock.called()).toBe(true);
-    expect(order).toEqual([
-      'Request 3',
-      'Request 2',
-      'RequestError 1',
-      'Response 1',
-      'Response 2',
-      'Response 3',
-    ]);
+    expect(unusedSpy).toHaveBeenCalledTimes(0);
+    expect(usedSpy).toHaveBeenCalledTimes(4);
+  });
+
+  describe('request availability during interception', () => {
+    it('should not be available to responseError when request throws', async () => {
+      expect.assertions(3);
+
+      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();
+    });
+
+    it('should be available to responseError when response throws', async () => {
+      let spiedRequest: Request | undefined;
+
+      http.intercept({
+        response() {
+          throw new Error('Internal Server Error');
+        },
+      });
+      http.intercept({
+        responseError({ request }) {
+          spiedRequest = request;
+        },
+      });
+
+      await expect(http.fetch('/my/path')).rejects.toThrow();
+      expect(fetchMock.called()).toBe(true);
+      expect(spiedRequest).toBeDefined();
+    });
+  });
+
+  describe('response availability during interception', () => {
+    it('should be available to responseError when network request fails', async () => {
+      fetchMock.restore();
+      fetchMock.get('*', { status: 500 });
+
+      let spiedResponse: Response | undefined;
+
+      http.intercept({
+        responseError({ response }) {
+          spiedResponse = response;
+        },
+      });
+
+      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 () => {
+    const unusedSpy = jest.fn();
+
+    http.intercept({ request: unusedSpy });
+    http.intercept({
+      request(request, controller) {
+        controller.halt();
+      },
+    });
+
+    http.fetch('/my/path');
+    await delay(500);
+
+    expect(unusedSpy).toHaveBeenCalledTimes(0);
+  });
+
+  it('should recover from failing request interception via request error interceptor', async () => {
+    const usedSpy = jest.fn();
+
+    http.intercept({
+      requestError(httpErrorRequest) {
+        return httpErrorRequest.request;
+      },
+      response: usedSpy,
+    });
+
+    http.intercept({
+      request(request, controller) {
+        throw new Error('Request Error');
+      },
+      response: usedSpy,
+    });
+
+    await expect(http.fetch('/my/path')).resolves.toEqual({ foo: 'bar' });
+    expect(usedSpy).toHaveBeenCalledTimes(2);
   });
 });
 

src/core/public/http/http_setup.ts

@@ -40,6 +40,14 @@ import { BasePath } from './base_path_service';
 const JSON_CONTENT = /^(application\/(json|x-javascript)|text\/(x-)?javascript|x-json)(;.*)?$/;
 const NDJSON_CONTENT = /^(application\/ndjson)(;.*)?$/;
 
+function checkHalt(controller: HttpInterceptController, error?: Error) {
+  if (error instanceof HttpInterceptHaltError) {
+    throw error;
+  } else if (controller.halted) {
+    throw new HttpInterceptHaltError();
+  }
+}
+
 export const setup = (
   injectedMetadata: InjectedMetadataSetup,
   fatalErrors: FatalErrorsSetup | null
@@ -102,9 +110,7 @@ export const setup = (
       (promise, interceptor) =>
         promise.then(
           async (current: Request) => {
-            if (controller.halted) {
-              throw new HttpInterceptHaltError();
-            }
+            checkHalt(controller);
 
             if (!interceptor.request) {
               return current;
@@ -115,11 +121,7 @@ export const setup = (
             return next;
           },
           async error => {
-            if (error instanceof HttpInterceptHaltError) {
-              throw error;
-            } else if (controller.halted) {
-              throw new HttpInterceptHaltError();
-            }
+            checkHalt(controller, error);
 
             if (!interceptor.requestError) {
               throw error;
@@ -147,15 +149,13 @@ export const setup = (
     responsePromise: Promise<HttpResponse>,
     controller: HttpInterceptController
   ) {
-    let current: HttpResponse;
+    let current: HttpResponse | undefined;
 
     const finalHttpResponse = await [...interceptors].reduce(
       (promise, interceptor) =>
         promise.then(
           async httpResponse => {
-            if (controller.halted) {
-              throw new HttpInterceptHaltError();
-            }
+            checkHalt(controller);
 
             if (!interceptor.response) {
               return httpResponse;
@@ -166,26 +166,40 @@ export const setup = (
             return current;
           },
           async error => {
-            if (error instanceof HttpInterceptHaltError) {
-              throw error;
-            } else if (controller.halted) {
-              throw new HttpInterceptHaltError();
-            }
+            checkHalt(controller, error);
 
             if (!interceptor.responseError) {
               throw error;
             }
 
-            const next = await interceptor.responseError({ ...current, error }, controller);
+            try {
+              const next = await interceptor.responseError(
+                {
+                  error,
+                  request: error.request || (current && current.request),
+                  response: error.response || (current && current.response),
+                  body: error.body || (current && current.body),
+                },
+                controller
+              );
 
-            if (!next) {
-              throw error;
+              checkHalt(controller, error);
+
+              if (!next) {
+                throw error;
+              }
+
+              return next;
+            } catch (err) {
+              checkHalt(controller, err);
+              throw err;
             }
-
-            return next;
           }
         ),
-      responsePromise
+      responsePromise.then(httpResponse => {
+        current = httpResponse;
+        return httpResponse;
+      })
     );
 
     return finalHttpResponse.body;
@@ -198,7 +212,7 @@ export const setup = (
     try {
       response = await window.fetch(request);
     } catch (err) {
-      throw new HttpFetchError(err.message);
+      throw new HttpFetchError(err.message, request);
     }
 
     const contentType = response.headers.get('Content-Type') || '';
@@ -218,24 +232,36 @@ export const setup = (
         }
       }
     } catch (err) {
-      throw new HttpFetchError(err.message, response, body);
+      throw new HttpFetchError(err.message, request, response, body);
     }
 
     if (!response.ok) {
-      throw new HttpFetchError(response.statusText, response, body);
+      throw new HttpFetchError(response.statusText, request, response, body);
     }
 
     return { response, body, request };
   }
 
-  function fetch(path: string, options: HttpFetchOptions = {}) {
+  async function fetch(path: string, options: HttpFetchOptions = {}) {
     const controller = new HttpInterceptController();
     const initialRequest = createRequest(path, options);
 
-    return interceptResponse(
-      interceptRequest(initialRequest, controller).then(fetcher),
-      controller
-    );
+    // 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) {
+        if (!(err instanceof HttpInterceptHaltError)) {
+          reject(err);
+        }
+      }
+    });
   }
 
   function shorthand(method: string) {

src/core/public/http/types.ts

@@ -89,17 +89,20 @@ export type HttpHandler = (path: string, options?: HttpFetchOptions) => Promise<
 export type HttpBody = BodyInit | null | any;
 /** @public */
 export interface HttpResponse {
-  request: Request;
+  request?: Request;
   response?: Response;
   body?: HttpBody;
 }
 /** @public */
-export interface HttpErrorResponse extends HttpResponse {
+export interface HttpErrorResponse {
   error: Error | HttpFetchError;
+  request?: Request;
+  response?: Response;
+  body?: HttpBody;
 }
 /** @public */
 export interface HttpErrorRequest {
-  request?: Request;
+  request: Request;
   error: Error;
 }
 /** @public */

src/core/public/index.ts

@@ -106,6 +106,7 @@ export {
   HttpResponse,
   HttpHandler,
   HttpBody,
+  HttpInterceptController,
 } from './http';
 
 export {

src/core/public/public.api.md

@@ -397,15 +397,21 @@ export interface HttpErrorRequest {
     // (undocumented)
     error: Error;
     // (undocumented)
-    request?: Request;
+    request: Request;
 }
 
 // @public (undocumented)
-export interface HttpErrorResponse extends HttpResponse {
+export interface HttpErrorResponse {
+    // (undocumented)
+    body?: HttpBody;
     // Warning: (ae-forgotten-export) The symbol "HttpFetchError" needs to be exported by the entry point index.d.ts
     // 
     // (undocumented)
     error: Error | HttpFetchError;
+    // (undocumented)
+    request?: Request;
+    // (undocumented)
+    response?: Response;
 }
 
 // @public (undocumented)
@@ -433,10 +439,18 @@ export interface HttpHeadersInit {
     [name: string]: any;
 }
 
+// Warning: (ae-missing-release-tag) "HttpInterceptController" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal)
+// 
+// @public (undocumented)
+export class HttpInterceptController {
+    // (undocumented)
+    halt(): void;
+    // (undocumented)
+    readonly halted: boolean;
+    }
+
 // @public (undocumented)
 export interface HttpInterceptor {
-    // Warning: (ae-forgotten-export) The symbol "HttpInterceptController" needs to be exported by the entry point index.d.ts
-    // 
     // (undocumented)
     request?(request: Request, controller: HttpInterceptController): Promise<Request> | Request | void;
     // (undocumented)
@@ -482,7 +496,7 @@ export interface HttpResponse {
     // (undocumented)
     body?: HttpBody;
     // (undocumented)
-    request: Request;
+    request?: Request;
     // (undocumented)
     response?: Response;
 }