maxmustermann/kibana
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

[Enterprise Search] Distinguish between error connecting vs. 5xx responses from Enterprise Search in UI (#103555)

Browse source 
* Update Enterprise Search request handler to send back an error connecting header

- vs only distinguishing error connecting issues by 502 status

+ clarify comment where this.handleConnectionError is called - for the most part, auth issues should already be caught by 401s in logic above

* Update HttpLogic to set errorConnecting state based on header

+ update tests etc to match read-only-mode state

* [Tech debt] Gracefully handle invalid HTTP responses

I've noticed this error a few times after Kibana gets shut down (http.response is undefined) so figured I would catch it here

* Fix missing try/catch/flashAPIErrors on engines overview

- This is the only http call I found missing a try/catch across our codebase, so we should be set for all views correctly flashing an API error that receive a 5xx response from ent-search

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
This commit is contained in:
Constance 2021-06-28 15:56:03 -07:00 committed by GitHub
parent 068aef82bc
commit 6feea1a506
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 100 additions and 42 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
x-pack/plugins/enterprise_search/common/constants.ts
View file

@ -80,6 +80,7 @@ export const JSON_HEADER = {
Accept: 'application/json', // Required for Enterprise Search APIs
};
export const ERROR_CONNECTING_HEADER = 'x-ent-search-error-connecting';
export const READ_ONLY_MODE_HEADER = 'x-ent-search-read-only-mode';
export const ENTERPRISE_SEARCH_KIBANA_COOKIE = '_enterprise_search';

20
x-pack/plugins/enterprise_search/public/applications/app_search/components/engines/engines_logic.test.ts
View file

@ -170,6 +170,16 @@ describe('EnginesLogic', () => {
});
expect(EnginesLogic.actions.onEnginesLoad).toHaveBeenCalledWith(MOCK_ENGINES_API_RESPONSE);
});
it('handles errors', async () => {
http.get.mockReturnValueOnce(Promise.reject('error'));
mount();
EnginesLogic.actions.loadEngines();
await nextTick();
expect(flashAPIErrors).toHaveBeenCalledWith('error');
});
});
describe('loadMetaEngines', () => {
@ -192,6 +202,16 @@ describe('EnginesLogic', () => {
MOCK_ENGINES_API_RESPONSE
);
});
it('handles errors', async () => {
http.get.mockReturnValueOnce(Promise.reject('error'));
mount();
EnginesLogic.actions.loadMetaEngines();
await nextTick();
expect(flashAPIErrors).toHaveBeenCalledWith('error');
});
});
describe('onDeleteEngineSuccess', () => {

40
x-pack/plugins/enterprise_search/public/applications/app_search/components/engines/engines_logic.ts
View file

@ -118,27 +118,35 @@ export const EnginesLogic = kea<MakeLogicType<EnginesValues, EnginesActions>>({
const { http } = HttpLogic.values;
const { enginesMeta } = values;
const response = await http.get('/api/app_search/engines', {
query: {
type: 'indexed',
'page[current]': enginesMeta.page.current,
'page[size]': enginesMeta.page.size,
},
});
actions.onEnginesLoad(response);
try {
const response = await http.get('/api/app_search/engines', {
query: {
type: 'indexed',
'page[current]': enginesMeta.page.current,
'page[size]': enginesMeta.page.size,
},
});
actions.onEnginesLoad(response);
} catch (e) {
flashAPIErrors(e);
}
},
loadMetaEngines: async () => {
const { http } = HttpLogic.values;
const { metaEnginesMeta } = values;
const response = await http.get('/api/app_search/engines', {
query: {
type: 'meta',
'page[current]': metaEnginesMeta.page.current,
'page[size]': metaEnginesMeta.page.size,
},
});
actions.onMetaEnginesLoad(response);
try {
const response = await http.get('/api/app_search/engines', {
query: {
type: 'meta',
'page[current]': metaEnginesMeta.page.current,
'page[size]': metaEnginesMeta.page.size,
},
});
actions.onMetaEnginesLoad(response);
} catch (e) {
flashAPIErrors(e);
}
},
onDeleteEngineSuccess: async ({ engine }) => {
flashSuccessToast(DELETE_ENGINE_MESSAGE(engine.name));

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

@ -91,31 +91,42 @@ describe('HttpLogic', () => {
jest.spyOn(HttpLogic.actions, 'setErrorConnecting');
});
it('handles errors connecting to Enterprise Search', async () => {
it('sets errorConnecting to true if the response header is true', async () => {
const httpResponse = {
response: { url: '/api/app_search/engines', status: 502 },
response: { url: '/api/app_search/engines', headers: { get: () => 'true' } },
};
await expect(interceptedResponse(httpResponse)).rejects.toEqual(httpResponse);
expect(HttpLogic.actions.setErrorConnecting).toHaveBeenCalled();
expect(HttpLogic.actions.setErrorConnecting).toHaveBeenCalledWith(true);
});
it('does not handle non-502 Enterprise Search errors', async () => {
it('sets errorConnecting to false if the response header is false', async () => {
const httpResponse = {
response: { url: '/api/workplace_search/overview', status: 404 },
response: { url: '/api/workplace_search/overview', headers: { get: () => 'false' } },
};
await expect(interceptedResponse(httpResponse)).rejects.toEqual(httpResponse);
expect(HttpLogic.actions.setErrorConnecting).not.toHaveBeenCalled();
expect(HttpLogic.actions.setErrorConnecting).toHaveBeenCalledWith(false);
});
it('does not handle errors for non-Enterprise Search API calls', async () => {
const httpResponse = {
response: { url: '/api/some_other_plugin/', status: 502 },
};
await expect(interceptedResponse(httpResponse)).rejects.toEqual(httpResponse);
describe('isEnterpriseSearchApi check', () => {
let httpResponse: any;
expect(HttpLogic.actions.setErrorConnecting).not.toHaveBeenCalled();
afterEach(async () => {
// Should always re-reject the promise and not call setErrorConnecting
await expect(interceptedResponse(httpResponse)).rejects.toEqual(httpResponse);
expect(HttpLogic.actions.setErrorConnecting).not.toHaveBeenCalled();
});
it('does not handle non-Enterprise Search API calls', async () => {
httpResponse = {
response: { url: '/api/some_other_plugin/', headers: { get: () => 'true' } },
};
});
it('does not handle invalid responses', async () => {
httpResponse = {};
});
});
});
@ -145,13 +156,24 @@ describe('HttpLogic', () => {
expect(HttpLogic.actions.setReadOnlyMode).toHaveBeenCalledWith(false);
});
it('does not handle headers for non-Enterprise Search API calls', async () => {
const httpResponse = {
response: { url: '/api/some_other_plugin/', headers: { get: () => 'true' } },
};
await expect(interceptedResponse(httpResponse)).resolves.toEqual(httpResponse);
describe('isEnterpriseSearchApi check', () => {
let httpResponse: any;
expect(HttpLogic.actions.setReadOnlyMode).not.toHaveBeenCalled();
afterEach(async () => {
// Should always resolve the promise and not call setReadOnlyMode
await expect(interceptedResponse(httpResponse)).resolves.toEqual(httpResponse);
expect(HttpLogic.actions.setReadOnlyMode).not.toHaveBeenCalled();
});
it('does not handle non-Enterprise Search API calls', async () => {
httpResponse = {
response: { url: '/api/some_other_plugin/', headers: { get: () => 'true' } },
};
});
it('does not handle invalid responses', async () => {
httpResponse = {};
});
});
});
});

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

@ -9,7 +9,7 @@ import { kea, MakeLogicType } from 'kea';
import { HttpSetup, HttpInterceptorResponseError, HttpResponse } from 'src/core/public';
import { READ_ONLY_MODE_HEADER } from '../../../../common/constants';
import { ERROR_CONNECTING_HEADER, READ_ONLY_MODE_HEADER } from '../../../../common/constants';
interface HttpValues {
http: HttpSetup;
@ -60,11 +60,12 @@ export const HttpLogic = kea<MakeLogicType<HttpValues, HttpActions>>({
const errorConnectingInterceptor = values.http.intercept({
responseError: async (httpResponse) => {
if (isEnterpriseSearchApi(httpResponse)) {
const { status } = httpResponse.response!;
const hasErrorConnecting = status === 502;
const hasErrorConnecting = httpResponse.response!.headers.get(ERROR_CONNECTING_HEADER);
if (hasErrorConnecting) {
if (hasErrorConnecting === 'true') {
actions.setErrorConnecting(true);
} else {
actions.setErrorConnecting(false);
}
}
@ -124,6 +125,8 @@ export const mountHttpLogic = (props: HttpLogicProps) => {
* Small helper that checks whether or not an http call is for an Enterprise Search API
*/
const isEnterpriseSearchApi = (httpResponse: HttpResponse) => {
const { url } = httpResponse.response!;
if (!httpResponse.response) return false; // Typically this means Kibana has stopped working, in which case we short-circuit early to prevent errors
const { url } = httpResponse.response;
return url.includes('/api/app_search/') || url.includes('/api/workplace_search/');
};

3
x-pack/plugins/enterprise_search/server/lib/enterprise_search_request_handler.test.ts
View file

@ -10,6 +10,7 @@ import { mockConfig, mockLogger, mockHttpAgent } from '../__mocks__';
import {
ENTERPRISE_SEARCH_KIBANA_COOKIE,
JSON_HEADER,
ERROR_CONNECTING_HEADER,
READ_ONLY_MODE_HEADER,
} from '../../common/constants';
@ -380,7 +381,7 @@ describe('EnterpriseSearchRequestHandler', () => {
expect(responseMock.customError).toHaveBeenCalledWith({
statusCode: 502,
body: 'Error connecting to Enterprise Search: Failed',
headers: mockExpectedResponseHeaders,
headers: { ...mockExpectedResponseHeaders, [ERROR_CONNECTING_HEADER]: 'true' },
});
expect(mockLogger.error).toHaveBeenCalled();
});

7
x-pack/plugins/enterprise_search/server/lib/enterprise_search_request_handler.ts
View file

@ -21,6 +21,7 @@ import { ConfigType } from '../';
import {
ENTERPRISE_SEARCH_KIBANA_COOKIE,
JSON_HEADER,
ERROR_CONNECTING_HEADER,
READ_ONLY_MODE_HEADER,
} from '../../common/constants';
@ -144,7 +145,7 @@ export class EnterpriseSearchRequestHandler {
body: responseBody,
});
} catch (e) {
// Catch connection/auth errors
// Catch connection errors
return this.handleConnectionError(response, e);
}
};
@ -280,7 +281,9 @@ export class EnterpriseSearchRequestHandler {
this.log.error(errorMessage);
if (e instanceof Error) this.log.debug(e.stack as string);
return response.customError({ statusCode: 502, headers: this.headers, body: errorMessage });
const headers = { ...this.headers, [ERROR_CONNECTING_HEADER]: 'true' };
return response.customError({ statusCode: 502, headers, body: errorMessage });
}
/**