From 2cb3911219f3ded895f6b040af6dbbae34a5950a Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Tue, 22 Aug 2023 13:09:00 -0700 Subject: [PATCH] Update error handling for saved query service (#163904) ## Summary Resolves https://github.com/elastic/kibana/issues/153497. Updates the saved query service to properly handle & return errors from the saved object client. Instead of displaying "internal server error" and returning 500, specific error messages occur for corresponding saved object client errors. After: ![image](https://github.com/elastic/kibana/assets/1178348/f8ba7b90-77fe-4db9-8377-0a1f878fe3a0) ### To do - [x] API integration tests --------- Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> (cherry picked from commit 57b7efc3eb3e03bb26e7a185342807b8e083a1a7) --- src/plugins/data/common/index.ts | 1 + src/plugins/data/server/query/routes.ts | 29 ++-- test/api_integration/apis/index.ts | 1 + .../apis/saved_queries/index.js | 13 ++ .../apis/saved_queries/saved_queries.js | 154 ++++++++++++++++++ 5 files changed, 184 insertions(+), 14 deletions(-) create mode 100644 test/api_integration/apis/saved_queries/index.js create mode 100644 test/api_integration/apis/saved_queries/saved_queries.js diff --git a/src/plugins/data/common/index.ts b/src/plugins/data/common/index.ts index 0d2fce23109f0..a7a5263229ddc 100644 --- a/src/plugins/data/common/index.ts +++ b/src/plugins/data/common/index.ts @@ -13,6 +13,7 @@ export { DEFAULT_QUERY_LANGUAGE, KIBANA_USER_QUERY_LANGUAGE_KEY, KQL_TELEMETRY_ROUTE_LATEST_VERSION, + SAVED_QUERY_BASE_URL, SCRIPT_LANGUAGES_ROUTE_LATEST_VERSION, UI_SETTINGS, } from './constants'; diff --git a/src/plugins/data/server/query/routes.ts b/src/plugins/data/server/query/routes.ts index a092a32d321a5..d7779273d9381 100644 --- a/src/plugins/data/server/query/routes.ts +++ b/src/plugins/data/server/query/routes.ts @@ -8,6 +8,7 @@ import { schema } from '@kbn/config-schema'; import { CoreSetup } from '@kbn/core/server'; +import { reportServerError } from '@kbn/kibana-utils-plugin/server'; import { SavedQueryRouteHandlerContext } from './route_handler_context'; import { SavedQueryRestResponse } from './route_types'; import { SAVED_QUERY_BASE_URL } from '../../common/constants'; @@ -57,8 +58,8 @@ export function registerSavedQueryRoutes({ http }: CoreSetup): void { const body: SavedQueryRestResponse = await savedQuery.create(request.body); return response.ok({ body }); } catch (e) { - // TODO: Handle properly - return response.customError(e); + const err = e.output?.payload ?? e; + return reportServerError(response, err); } } ); @@ -85,8 +86,8 @@ export function registerSavedQueryRoutes({ http }: CoreSetup): void { const body: SavedQueryRestResponse = await savedQuery.update(id, request.body); return response.ok({ body }); } catch (e) { - // TODO: Handle properly - return response.customError(e); + const err = e.output?.payload ?? e; + return reportServerError(response, err); } } ); @@ -112,8 +113,8 @@ export function registerSavedQueryRoutes({ http }: CoreSetup): void { const body: SavedQueryRestResponse = await savedQuery.get(id); return response.ok({ body }); } catch (e) { - // TODO: Handle properly - return response.customError(e); + const err = e.output?.payload ?? e; + return reportServerError(response, err); } } ); @@ -136,8 +137,8 @@ export function registerSavedQueryRoutes({ http }: CoreSetup): void { const count: number = await savedQuery.count(); return response.ok({ body: `${count}` }); } catch (e) { - // TODO: Handle properly - return response.customError(e); + const err = e.output?.payload ?? e; + return reportServerError(response, err); } } ); @@ -170,8 +171,8 @@ export function registerSavedQueryRoutes({ http }: CoreSetup): void { await savedQuery.find(request.body); return response.ok({ body }); } catch (e) { - // TODO: Handle properly - return response.customError(e); + const err = e.output?.payload ?? e; + return reportServerError(response, err); } } ); @@ -198,8 +199,8 @@ export function registerSavedQueryRoutes({ http }: CoreSetup): void { await savedQuery.getAll(); return response.ok({ body }); } catch (e) { - // TODO: Handle properly - return response.customError(e); + const err = e.output?.payload ?? e; + return reportServerError(response, err); } } ); @@ -225,8 +226,8 @@ export function registerSavedQueryRoutes({ http }: CoreSetup): void { await savedQuery.delete(id); return response.ok(); } catch (e) { - // TODO: Handle properly - return response.customError(e); + const err = e.output?.payload ?? e; + return reportServerError(response, err); } } ); diff --git a/test/api_integration/apis/index.ts b/test/api_integration/apis/index.ts index 241af3b9a6374..4bc1ea7648ad4 100644 --- a/test/api_integration/apis/index.ts +++ b/test/api_integration/apis/index.ts @@ -20,6 +20,7 @@ export default function ({ loadTestFile }: FtrProviderContext) { loadTestFile(require.resolve('./kql_telemetry')); loadTestFile(require.resolve('./saved_objects_management')); loadTestFile(require.resolve('./saved_objects')); + loadTestFile(require.resolve('./saved_queries')); loadTestFile(require.resolve('./scripts')); loadTestFile(require.resolve('./search')); loadTestFile(require.resolve('./short_url')); diff --git a/test/api_integration/apis/saved_queries/index.js b/test/api_integration/apis/saved_queries/index.js new file mode 100644 index 0000000000000..6f531e8026940 --- /dev/null +++ b/test/api_integration/apis/saved_queries/index.js @@ -0,0 +1,13 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +export default function ({ loadTestFile }) { + describe('Saved queries', () => { + loadTestFile(require.resolve('./saved_queries')); + }); +} diff --git a/test/api_integration/apis/saved_queries/saved_queries.js b/test/api_integration/apis/saved_queries/saved_queries.js new file mode 100644 index 0000000000000..eb3c1465e24de --- /dev/null +++ b/test/api_integration/apis/saved_queries/saved_queries.js @@ -0,0 +1,154 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ +import expect from '@kbn/expect'; +import { ELASTIC_HTTP_VERSION_HEADER } from '@kbn/core-http-common'; +import { SAVED_QUERY_BASE_URL } from '@kbn/data-plugin/common'; + +// node scripts/functional_tests --config test/api_integration/config.js --grep="search session" + +const mockSavedQuery = { + title: 'my title', + description: 'my description', + query: { + query: 'foo: bar', + language: 'kql', + }, + filters: [], +}; + +export default function ({ getService }) { + const esArchiver = getService('esArchiver'); + const supertest = getService('supertest'); + void SAVED_QUERY_BASE_URL; + + describe('Saved queries API', function () { + before(async () => { + await esArchiver.emptyKibanaIndex(); + await esArchiver.loadIfNeeded('test/functional/fixtures/es_archiver/logstash_functional'); + }); + + after(async () => { + await esArchiver.unload('test/functional/fixtures/es_archiver/logstash_functional'); + }); + + it('should return 200 for create saved query', () => + supertest + .post(`${SAVED_QUERY_BASE_URL}/_create`) + .set(ELASTIC_HTTP_VERSION_HEADER, '1') + .send(mockSavedQuery) + .expect(200) + .then(({ body }) => { + expect(body.id).to.have.length(36); + expect(body.attributes.title).to.be('my title'); + expect(body.attributes.description).to.be('my description'); + })); + + it('should return 400 for create invalid saved query', () => + supertest + .post(`${SAVED_QUERY_BASE_URL}/_create`) + .set(ELASTIC_HTTP_VERSION_HEADER, '1') + .send({ description: 'my description' }) + .expect(400)); + + it('should return 200 for update saved query', () => + supertest + .post(`${SAVED_QUERY_BASE_URL}/_create`) + .set(ELASTIC_HTTP_VERSION_HEADER, '1') + .send(mockSavedQuery) + .expect(200) + .then(({ body }) => + supertest + .put(`${SAVED_QUERY_BASE_URL}/${body.id}`) + .set(ELASTIC_HTTP_VERSION_HEADER, '1') + .send({ + ...mockSavedQuery, + title: 'my new title', + }) + .expect(200) + .then((res) => { + expect(res.body.id).to.be(body.id); + expect(res.body.attributes.title).to.be('my new title'); + }) + )); + + it('should return 404 for update non-existent saved query', () => + supertest + .put(`${SAVED_QUERY_BASE_URL}/invalid_id`) + .set(ELASTIC_HTTP_VERSION_HEADER, '1') + .send(mockSavedQuery) + .expect(404)); + + it('should return 200 for get saved query', () => + supertest + .post(`${SAVED_QUERY_BASE_URL}/_create`) + .set(ELASTIC_HTTP_VERSION_HEADER, '1') + .send(mockSavedQuery) + .expect(200) + .then(({ body }) => + supertest + .get(`${SAVED_QUERY_BASE_URL}/${body.id}`) + .set(ELASTIC_HTTP_VERSION_HEADER, '1') + .expect(200) + .then((res) => { + expect(res.body.id).to.be(body.id); + expect(res.body.attributes.title).to.be(body.attributes.title); + }) + )); + + it('should return 404 for get non-existent saved query', () => + supertest + .get(`${SAVED_QUERY_BASE_URL}/invalid_id`) + .set(ELASTIC_HTTP_VERSION_HEADER, '1') + .expect(404)); + + it('should return 200 for saved query count', () => + supertest + .get(`${SAVED_QUERY_BASE_URL}/_count`) + .set(ELASTIC_HTTP_VERSION_HEADER, '1') + .expect(200)); + + it('should return 200 for find saved queries', () => + supertest + .post(`${SAVED_QUERY_BASE_URL}/_find`) + .set(ELASTIC_HTTP_VERSION_HEADER, '1') + .send({}) + .expect(200)); + + it('should return 400 for bad find saved queries request', () => + supertest + .post(`${SAVED_QUERY_BASE_URL}/_find`) + .set(ELASTIC_HTTP_VERSION_HEADER, '1') + .send({ foo: 'bar' }) + .expect(400)); + + it('should return 200 for find all saved queries', () => + supertest + .post(`${SAVED_QUERY_BASE_URL}/_all`) + .set(ELASTIC_HTTP_VERSION_HEADER, '1') + .expect(200)); + + it('should return 200 for delete saved query', () => + supertest + .post(`${SAVED_QUERY_BASE_URL}/_create`) + .set(ELASTIC_HTTP_VERSION_HEADER, '1') + .send(mockSavedQuery) + .expect(200) + .then(({ body }) => + supertest + .delete(`${SAVED_QUERY_BASE_URL}/${body.id}`) + .set(ELASTIC_HTTP_VERSION_HEADER, '1') + .expect(200) + )); + + it('should return 404 for get non-existent saved query', () => + supertest + .delete(`${SAVED_QUERY_BASE_URL}/invalid_id`) + .set(ELASTIC_HTTP_VERSION_HEADER, '1') + .expect(404)); + }); +}