github-mirrors/kibana
1
0
Fork 0
mirror of https://github.com/elastic/kibana.git synced 2025-04-23 09:19:04 -04:00
Code Issues Projects Releases Packages Wiki Activity

Update error handling for saved query service (#163904)

Browse source 
## 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](f8ba7b90-77fe-4db9-8377-0a1f878fe3a0)

### To do

- [x] API integration tests

---------

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
This commit is contained in:
Lukas Olson 2023-08-22 13:09:00 -07:00 • committed by GitHub
parent 1caac7d28a
commit 57b7efc3eb
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 184 additions and 14 deletions
Download patch file Download diff file Expand all files Collapse all files

1
src/plugins/data/common/index.ts
View file

@ -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';

29
src/plugins/data/server/query/routes.ts
View file

@ -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);
}
}
);

1
test/api_integration/apis/index.ts
View file

@ -21,6 +21,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'));

13
test/api_integration/apis/saved_queries/index.js Normal file
View file

@ -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'));
});
}

154
test/api_integration/apis/saved_queries/saved_queries.js Normal file
View file

@ -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));
});
}