Skip to content

Commit

Permalink
[Enterprise Search] Update enterpriseSearchRequestHandler to manage r…
Browse files Browse the repository at this point in the history
…ange of errors + add handleAPIErrors helper (#77258)

* Update/refactor EnterpriseSearchRequestHandler to manage internal endpoint error behavior

+ pull out / refactor all errors into their own methods instead of relying on a single 'error connecting' message

* Fix http interceptor bug preventing 4xx/5xx responses from being caught

* Add handleAPIError helper

- New and improved from ent-search - this one automatically sets flash messages for you so you don't have to pass in a callback!

* Fix type check

* [Feedback] Add option to queue flash messages to handleAPIError

- Should be hopefully useful for calls that need to queue an error and then redirect to another page

* PR feedback: Add test case for bodies flagged as JSON which are not

* Rename handleAPIError to flashAPIErrors
  • Loading branch information
Constance authored Sep 15, 2020
1 parent 88caa79 commit 98e6475
Show file tree
Hide file tree
Showing 6 changed files with 426 additions and 93 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

jest.mock('./', () => ({
FlashMessagesLogic: {
actions: {
setFlashMessages: jest.fn(),
setQueuedMessages: jest.fn(),
},
},
}));
import { FlashMessagesLogic } from './';

import { flashAPIErrors } from './handle_api_errors';

describe('flashAPIErrors', () => {
const mockHttpError = {
body: {
statusCode: 404,
error: 'Not Found',
message: 'Could not find X,Could not find Y,Something else bad happened',
attributes: {
errors: ['Could not find X', 'Could not find Y', 'Something else bad happened'],
},
},
} as any;

beforeEach(() => {
jest.clearAllMocks();
});

it('converts API errors into flash messages', () => {
flashAPIErrors(mockHttpError);

expect(FlashMessagesLogic.actions.setFlashMessages).toHaveBeenCalledWith([
{ type: 'error', message: 'Could not find X' },
{ type: 'error', message: 'Could not find Y' },
{ type: 'error', message: 'Something else bad happened' },
]);
});

it('queues messages when isQueued is passed', () => {
flashAPIErrors(mockHttpError, { isQueued: true });

expect(FlashMessagesLogic.actions.setQueuedMessages).toHaveBeenCalledWith([
{ type: 'error', message: 'Could not find X' },
{ type: 'error', message: 'Could not find Y' },
{ type: 'error', message: 'Something else bad happened' },
]);
});

it('displays a generic error message and re-throws non-API errors', () => {
try {
flashAPIErrors(Error('whatever') as any);
} catch (e) {
expect(e.message).toEqual('whatever');
expect(FlashMessagesLogic.actions.setFlashMessages).toHaveBeenCalledWith([
{ type: 'error', message: 'An unexpected error occurred' },
]);
}
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import { HttpResponse } from 'src/core/public';

import { FlashMessagesLogic, IFlashMessage } from './';

/**
* The API errors we are handling can come from one of two ways:
* - When our http calls recieve a response containing an error code, such as a 404 or 500
* - Our own JS while handling a successful response
*
* In the first case, if it is a purposeful error (like a 404) we will receive an
* `errors` property in the response's data, which will contain messages we can
* display to the user.
*/
interface IErrorResponse {
statusCode: number;
error: string;
message: string;
attributes: {
errors: string[];
};
}
interface IOptions {
isQueued?: boolean;
}

/**
* Converts API/HTTP errors into user-facing Flash Messages
*/
export const flashAPIErrors = (
error: HttpResponse<IErrorResponse>,
{ isQueued }: IOptions = {}
) => {
const defaultErrorMessage = 'An unexpected error occurred';

const errorFlashMessages: IFlashMessage[] = Array.isArray(error?.body?.attributes?.errors)
? error.body!.attributes.errors.map((message) => ({ type: 'error', message }))
: [{ type: 'error', message: defaultErrorMessage }];

if (isQueued) {
FlashMessagesLogic.actions.setQueuedMessages(errorFlashMessages);
} else {
FlashMessagesLogic.actions.setFlashMessages(errorFlashMessages);
}

// If this was a programming error or a failed request (such as a CORS) error,
// we rethrow the error so it shows up in the developer console
if (!error?.body?.message) {
throw error;
}
};
Original file line number Diff line number Diff line change
Expand Up @@ -74,21 +74,24 @@ describe('HttpLogic', () => {
describe('errorConnectingInterceptor', () => {
it('handles errors connecting to Enterprise Search', async () => {
const { responseError } = mockHttp.intercept.mock.calls[0][0] as any;
await responseError({ response: { url: '/api/app_search/engines', status: 502 } });
const httpResponse = { response: { url: '/api/app_search/engines', status: 502 } };
await expect(responseError(httpResponse)).rejects.toEqual(httpResponse);

expect(HttpLogic.actions.setErrorConnecting).toHaveBeenCalled();
});

it('does not handle non-502 Enterprise Search errors', async () => {
const { responseError } = mockHttp.intercept.mock.calls[0][0] as any;
await responseError({ response: { url: '/api/workplace_search/overview', status: 404 } });
const httpResponse = { response: { url: '/api/workplace_search/overview', status: 404 } };
await expect(responseError(httpResponse)).rejects.toEqual(httpResponse);

expect(HttpLogic.actions.setErrorConnecting).not.toHaveBeenCalled();
});

it('does not handle errors for unrelated calls', async () => {
const { responseError } = mockHttp.intercept.mock.calls[0][0] as any;
await responseError({ response: { url: '/api/some_other_plugin/', status: 502 } });
const httpResponse = { response: { url: '/api/some_other_plugin/', status: 502 } };
await expect(responseError(httpResponse)).rejects.toEqual(httpResponse);

expect(HttpLogic.actions.setErrorConnecting).not.toHaveBeenCalled();
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

import { kea, MakeLogicType } from 'kea';

import { HttpSetup } from 'src/core/public';
import { HttpSetup, HttpInterceptorResponseError } from 'src/core/public';

export interface IHttpValues {
http: HttpSetup;
Expand Down Expand Up @@ -68,7 +68,9 @@ export const HttpLogic = kea<MakeLogicType<IHttpValues, IHttpActions>>({
if (isApiResponse && hasErrorConnecting) {
actions.setErrorConnecting(true);
}
return httpResponse;

// Re-throw error so that downstream catches work as expected
return Promise.reject(httpResponse) as Promise<HttpInterceptorResponseError>;
},
});
httpInterceptors.push(errorConnectingInterceptor);
Expand Down
Loading

0 comments on commit 98e6475

Please sign in to comment.