Skip to content

Commit 657ddfb

Browse files
Constancecee-chen
authored andcommitted
[Enterprise Search] Update enterpriseSearchRequestHandler to manage range 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
1 parent e88a4cd commit 657ddfb

File tree

6 files changed

+426
-93
lines changed

6 files changed

+426
-93
lines changed
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License;
4+
* you may not use this file except in compliance with the Elastic License.
5+
*/
6+
7+
jest.mock('./', () => ({
8+
FlashMessagesLogic: {
9+
actions: {
10+
setFlashMessages: jest.fn(),
11+
setQueuedMessages: jest.fn(),
12+
},
13+
},
14+
}));
15+
import { FlashMessagesLogic } from './';
16+
17+
import { flashAPIErrors } from './handle_api_errors';
18+
19+
describe('flashAPIErrors', () => {
20+
const mockHttpError = {
21+
body: {
22+
statusCode: 404,
23+
error: 'Not Found',
24+
message: 'Could not find X,Could not find Y,Something else bad happened',
25+
attributes: {
26+
errors: ['Could not find X', 'Could not find Y', 'Something else bad happened'],
27+
},
28+
},
29+
} as any;
30+
31+
beforeEach(() => {
32+
jest.clearAllMocks();
33+
});
34+
35+
it('converts API errors into flash messages', () => {
36+
flashAPIErrors(mockHttpError);
37+
38+
expect(FlashMessagesLogic.actions.setFlashMessages).toHaveBeenCalledWith([
39+
{ type: 'error', message: 'Could not find X' },
40+
{ type: 'error', message: 'Could not find Y' },
41+
{ type: 'error', message: 'Something else bad happened' },
42+
]);
43+
});
44+
45+
it('queues messages when isQueued is passed', () => {
46+
flashAPIErrors(mockHttpError, { isQueued: true });
47+
48+
expect(FlashMessagesLogic.actions.setQueuedMessages).toHaveBeenCalledWith([
49+
{ type: 'error', message: 'Could not find X' },
50+
{ type: 'error', message: 'Could not find Y' },
51+
{ type: 'error', message: 'Something else bad happened' },
52+
]);
53+
});
54+
55+
it('displays a generic error message and re-throws non-API errors', () => {
56+
try {
57+
flashAPIErrors(Error('whatever') as any);
58+
} catch (e) {
59+
expect(e.message).toEqual('whatever');
60+
expect(FlashMessagesLogic.actions.setFlashMessages).toHaveBeenCalledWith([
61+
{ type: 'error', message: 'An unexpected error occurred' },
62+
]);
63+
}
64+
});
65+
});
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License;
4+
* you may not use this file except in compliance with the Elastic License.
5+
*/
6+
7+
import { HttpResponse } from 'src/core/public';
8+
9+
import { FlashMessagesLogic, IFlashMessage } from './';
10+
11+
/**
12+
* The API errors we are handling can come from one of two ways:
13+
* - When our http calls recieve a response containing an error code, such as a 404 or 500
14+
* - Our own JS while handling a successful response
15+
*
16+
* In the first case, if it is a purposeful error (like a 404) we will receive an
17+
* `errors` property in the response's data, which will contain messages we can
18+
* display to the user.
19+
*/
20+
interface IErrorResponse {
21+
statusCode: number;
22+
error: string;
23+
message: string;
24+
attributes: {
25+
errors: string[];
26+
};
27+
}
28+
interface IOptions {
29+
isQueued?: boolean;
30+
}
31+
32+
/**
33+
* Converts API/HTTP errors into user-facing Flash Messages
34+
*/
35+
export const flashAPIErrors = (
36+
error: HttpResponse<IErrorResponse>,
37+
{ isQueued }: IOptions = {}
38+
) => {
39+
const defaultErrorMessage = 'An unexpected error occurred';
40+
41+
const errorFlashMessages: IFlashMessage[] = Array.isArray(error?.body?.attributes?.errors)
42+
? error.body!.attributes.errors.map((message) => ({ type: 'error', message }))
43+
: [{ type: 'error', message: defaultErrorMessage }];
44+
45+
if (isQueued) {
46+
FlashMessagesLogic.actions.setQueuedMessages(errorFlashMessages);
47+
} else {
48+
FlashMessagesLogic.actions.setFlashMessages(errorFlashMessages);
49+
}
50+
51+
// If this was a programming error or a failed request (such as a CORS) error,
52+
// we rethrow the error so it shows up in the developer console
53+
if (!error?.body?.message) {
54+
throw error;
55+
}
56+
};

x-pack/plugins/enterprise_search/public/applications/shared/http/http_logic.test.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -74,21 +74,24 @@ describe('HttpLogic', () => {
7474
describe('errorConnectingInterceptor', () => {
7575
it('handles errors connecting to Enterprise Search', async () => {
7676
const { responseError } = mockHttp.intercept.mock.calls[0][0] as any;
77-
await responseError({ response: { url: '/api/app_search/engines', status: 502 } });
77+
const httpResponse = { response: { url: '/api/app_search/engines', status: 502 } };
78+
await expect(responseError(httpResponse)).rejects.toEqual(httpResponse);
7879

7980
expect(HttpLogic.actions.setErrorConnecting).toHaveBeenCalled();
8081
});
8182

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

8688
expect(HttpLogic.actions.setErrorConnecting).not.toHaveBeenCalled();
8789
});
8890

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

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

x-pack/plugins/enterprise_search/public/applications/shared/http/http_logic.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
import { kea, MakeLogicType } from 'kea';
88

9-
import { HttpSetup } from 'src/core/public';
9+
import { HttpSetup, HttpInterceptorResponseError } from 'src/core/public';
1010

1111
export interface IHttpValues {
1212
http: HttpSetup;
@@ -68,7 +68,9 @@ export const HttpLogic = kea<MakeLogicType<IHttpValues, IHttpActions>>({
6868
if (isApiResponse && hasErrorConnecting) {
6969
actions.setErrorConnecting(true);
7070
}
71-
return httpResponse;
71+
72+
// Re-throw error so that downstream catches work as expected
73+
return Promise.reject(httpResponse) as Promise<HttpInterceptorResponseError>;
7274
},
7375
});
7476
httpInterceptors.push(errorConnectingInterceptor);

0 commit comments

Comments
 (0)