[8.10] Update error handling for saved query service (elastic#163904) (…

…elastic#164511)

# Backport

This will backport the following commits from `main` to `8.10`:
- [Update error handling for saved query service
(elastic#163904)](elastic#163904)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Lukas
Olson","email":"[email protected]"},"sourceCommit":{"committedDate":"2023-08-22T20:09:00Z","message":"Update
error handling for saved query service (elastic#163904)\n\n##
Summary\r\n\r\nResolves
https://github.com/elastic/kibana/issues/153497.\r\n\r\nUpdates the
saved query service to properly handle & return errors from\r\nthe saved
object client. Instead of displaying \"internal server error\"\r\nand
returning 500, specific error messages occur for corresponding
saved\r\nobject client
errors.\r\n\r\nAfter:\r\n\r\n\r\n![image](https://github.com/elastic/kibana/assets/1178348/f8ba7b90-77fe-4db9-8377-0a1f878fe3a0)\r\n\r\n###
To do\r\n\r\n- [x] API integration
tests\r\n\r\n---------\r\n\r\nCo-authored-by: Kibana Machine
<[email protected]>","sha":"57b7efc3eb3e03bb26e7a185342807b8e083a1a7","branchLabelMapping":{"^v8.11.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team:DataDiscovery","v8.10.0","v8.11.0"],"number":163904,"url":"https://github.com/elastic/kibana/pull/163904","mergeCommit":{"message":"Update
error handling for saved query service (elastic#163904)\n\n##
Summary\r\n\r\nResolves
https://github.com/elastic/kibana/issues/153497.\r\n\r\nUpdates the
saved query service to properly handle & return errors from\r\nthe saved
object client. Instead of displaying \"internal server error\"\r\nand
returning 500, specific error messages occur for corresponding
saved\r\nobject client
errors.\r\n\r\nAfter:\r\n\r\n\r\n![image](https://github.com/elastic/kibana/assets/1178348/f8ba7b90-77fe-4db9-8377-0a1f878fe3a0)\r\n\r\n###
To do\r\n\r\n- [x] API integration
tests\r\n\r\n---------\r\n\r\nCo-authored-by: Kibana Machine
<[email protected]>","sha":"57b7efc3eb3e03bb26e7a185342807b8e083a1a7"}},"sourceBranch":"main","suggestedTargetBranches":["8.10"],"targetPullRequestStates":[{"branch":"8.10","label":"v8.10.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.11.0","labelRegex":"^v8.11.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/163904","number":163904,"mergeCommit":{"message":"Update
error handling for saved query service (elastic#163904)\n\n##
Summary\r\n\r\nResolves
https://github.com/elastic/kibana/issues/153497.\r\n\r\nUpdates the
saved query service to properly handle & return errors from\r\nthe saved
object client. Instead of displaying \"internal server error\"\r\nand
returning 500, specific error messages occur for corresponding
saved\r\nobject client
errors.\r\n\r\nAfter:\r\n\r\n\r\n![image](https://github.com/elastic/kibana/assets/1178348/f8ba7b90-77fe-4db9-8377-0a1f878fe3a0)\r\n\r\n###
To do\r\n\r\n- [x] API integration
tests\r\n\r\n---------\r\n\r\nCo-authored-by: Kibana Machine
<[email protected]>","sha":"57b7efc3eb3e03bb26e7a185342807b8e083a1a7"}}]}]
BACKPORT-->

Co-authored-by: Lukas Olson <[email protected]>

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

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

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

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

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