Expand interceptors to handle both errors and successful responses. F…

…ocus interceptors on executing side effects, not mutating responses.

src/plugins/es_ui_shared/public/request/send_request.test.helpers.ts

@@ -17,10 +17,12 @@ import {
 
 export interface SendRequestHelpers {
   getSendRequestSpy: () => sinon.SinonStub;
-  sendSuccessRequest: () => Promise<SendRequestResponse>;
+  sendSuccessRequest: (
+    responseInterceptors?: SendRequestConfig['responseInterceptors']
+  ) => Promise<SendRequestResponse>;
   getSuccessResponse: () => SendRequestResponse;
   sendErrorRequest: (
-    errorInterceptors?: SendRequestConfig['errorInterceptors']
+    responseInterceptors?: SendRequestConfig['responseInterceptors']
   ) => Promise<SendRequestResponse>;
   getErrorResponse: () => SendRequestResponse;
 }
@@ -51,7 +53,8 @@ export const createSendRequestHelpers = (): SendRequestHelpers => {
       })
     )
     .resolves(successResponse);
-  const sendSuccessRequest = () => sendRequest({ ...successRequest });
+  const sendSuccessRequest = (responseInterceptors?: SendRequestConfig['responseInterceptors']) =>
+    sendRequest({ ...successRequest, responseInterceptors });
   const getSuccessResponse = () => ({ data: successResponse.data, error: null });
 
   // Set up failed request helpers.
@@ -64,8 +67,8 @@ export const createSendRequestHelpers = (): SendRequestHelpers => {
       })
     )
     .rejects(errorResponse);
-  const sendErrorRequest = (errorInterceptors?: SendRequestConfig['errorInterceptors']) =>
-    sendRequest({ ...errorRequest, errorInterceptors });
+  const sendErrorRequest = (responseInterceptors?: SendRequestConfig['responseInterceptors']) =>
+    sendRequest({ ...errorRequest, responseInterceptors });
   const getErrorResponse = () => ({
     data: null,
     error: errorResponse.response.data,

src/plugins/es_ui_shared/public/request/send_request.test.ts

@@ -29,24 +29,29 @@ describe('sendRequest function', () => {
     const { sendErrorRequest, getSendRequestSpy, getErrorResponse } = helpers;
 
     // For some reason sinon isn't throwing an error on rejection, as an awaited Promise normally would.
-    const error = await sendErrorRequest();
+    const errorResponse = await sendErrorRequest();
     sinon.assert.calledOnce(getSendRequestSpy());
-    expect(error).toEqual(getErrorResponse());
+    expect(errorResponse).toEqual(getErrorResponse());
   });
 
-  it('applies errorInterceptors to errors', async () => {
-    const { sendErrorRequest, getSendRequestSpy } = helpers;
-    const errorInterceptors = [
-      (error: any) => ['Error is:', error.statusText],
-      (interceptedError: string[]) => interceptedError.join(' '),
-    ];
+  it('calls responseInterceptors with successful responses', async () => {
+    const { sendSuccessRequest, getSuccessResponse } = helpers;
+    const successInterceptorSpy = sinon.spy();
+    const successInterceptors = [successInterceptorSpy];
+
+    await sendSuccessRequest(successInterceptors);
+    sinon.assert.calledOnce(successInterceptorSpy);
+    sinon.assert.calledWith(successInterceptorSpy, getSuccessResponse());
+  });
+
+  it('calls responseInterceptors with errors', async () => {
+    const { sendErrorRequest, getErrorResponse } = helpers;
+    const errorInterceptorSpy = sinon.spy();
+    const errorInterceptors = [errorInterceptorSpy];
 
     // For some reason sinon isn't throwing an error on rejection, as an awaited Promise normally would.
-    const error = await sendErrorRequest(errorInterceptors);
-    sinon.assert.calledOnce(getSendRequestSpy());
-    expect(error).toEqual({
-      data: null,
-      error: 'Error is: Error message',
-    });
+    await sendErrorRequest(errorInterceptors);
+    sinon.assert.calledOnce(errorInterceptorSpy);
+    sinon.assert.calledWith(errorInterceptorSpy, getErrorResponse());
   });
 });

src/plugins/es_ui_shared/public/request/send_request.ts

@@ -8,7 +8,7 @@
 
 import { HttpSetup, HttpFetchQuery } from '../../../../../src/core/public';
 
-export type ResponseInterceptor = (response: any) => any;
+export type ResponseInterceptor = ({ data, error }: { data: any; error: any }) => void;
 
 export interface SendRequestConfig {
   path: string;
@@ -20,45 +20,49 @@ export interface SendRequestConfig {
    * HttpFetchOptions#asSystemRequest.
    */
   asSystemRequest?: boolean;
-  errorInterceptors?: ResponseInterceptor[];
+  responseInterceptors?: ResponseInterceptor[];
 }
 
 export interface SendRequestResponse<D = any, E = any> {
   data: D | null;
   error: E | null;
 }
 
-// Pass the response sequentially through each interceptor, providing
-// the output of one interceptor as the input of the next interceptor.
-const applyInterceptors = (response: any, interceptors: ResponseInterceptor[] = []): any => {
-  return interceptors.reduce(
-    (interceptedResponse, interceptor) => interceptor(interceptedResponse),
-    response
-  );
+// Pass the response sequentially through each interceptor, allowing for
+// side effects to be run.
+const updateResponseInterceptors = (
+  response: any,
+  responseInterceptors: ResponseInterceptor[] = []
+) => {
+  responseInterceptors.forEach((interceptor) => interceptor(response));
 };
 
 export const sendRequest = async <D = any, E = any>(
   httpClient: HttpSetup,
-  { path, method, body, query, asSystemRequest, errorInterceptors }: SendRequestConfig
+  { path, method, body, query, asSystemRequest, responseInterceptors }: SendRequestConfig
 ): Promise<SendRequestResponse<D, E>> => {
   try {
     const stringifiedBody = typeof body === 'string' ? body : JSON.stringify(body);
-    const response = await httpClient[method](path, {
+    const rawResponse = await httpClient[method](path, {
       body: stringifiedBody,
       query,
       asSystemRequest,
     });
 
-    return {
-      data: response.data ? response.data : response,
+    const response = {
+      data: rawResponse.data ? rawResponse.data : rawResponse,
       error: null,
     };
+
+    updateResponseInterceptors(response, responseInterceptors);
+    return response;
   } catch (e) {
-    const responseError = e.response?.data ?? e.body;
-    const interceptedError = applyInterceptors(responseError, errorInterceptors);
-    return {
+    const response = {
       data: null,
-      error: interceptedError,
+      error: e.response?.data ?? e.body,
     };
+
+    updateResponseInterceptors(response, responseInterceptors);
+    return response;
   }
 };

src/plugins/es_ui_shared/public/request/use_request.test.ts

@@ -117,18 +117,6 @@ describe('useRequest hook', () => {
         expect(hookResult.error).toBe(getErrorResponse().error);
       });
 
-      it('applies errorInterceptors to errors', async () => {
-        const { setupErrorRequest, completeRequest, hookResult } = helpers;
-        const errorInterceptors = [
-          (error: any) => ['Error is:', error.statusText],
-          (interceptedError: string[]) => interceptedError.join(' '),
-        ];
-
-        setupErrorRequest({ errorInterceptors });
-        await completeRequest();
-        expect(hookResult.error).toBe('Error is: Error message');
-      });
-
       it('surfaces body-shaped errors from requests', async () => {
         const { setupErrorWithBodyRequest, completeRequest, hookResult, getErrorWithBodyResponse } =
           helpers;

src/plugins/es_ui_shared/public/request/use_request.ts

@@ -35,7 +35,7 @@ export const useRequest = <D = any, E = Error>(
     pollIntervalMs,
     initialData,
     deserializer,
-    errorInterceptors,
+    responseInterceptors,
   }: UseRequestConfig
 ): UseRequestResponse<D, E> => {
   const isMounted = useRef(false);
@@ -89,7 +89,7 @@ export const useRequest = <D = any, E = Error>(
       // Any requests that are sent in the background (without user interaction) should be flagged as "system requests". This should not be
       // confused with any terminology in Elasticsearch. This is a Kibana-specific construct that allows the server to differentiate between
       // user-initiated and requests "system"-initiated requests, for purposes like security features.
-      const requestPayload = { ...requestBody, asSystemRequest, errorInterceptors };
+      const requestPayload = { ...requestBody, asSystemRequest, responseInterceptors };
       const response = await sendRequest<D, E>(httpClient, requestPayload);
       const { data: serializedResponseData, error: responseError } = response;
 
@@ -115,7 +115,7 @@ export const useRequest = <D = any, E = Error>(
       // Setting isLoading to false also acts as a signal for scheduling the next poll request.
       setIsLoading(false);
     },
-    [requestBody, httpClient, deserializer, clearPollInterval, errorInterceptors]
+    [requestBody, httpClient, deserializer, clearPollInterval, responseInterceptors]
   );
 
   const scheduleRequest = useCallback(() => {