More PR review feedback

src/core/server/saved_objects/service/lib/legacy_url_aliases/delete_legacy_url_aliases.test.mock.ts

@@ -0,0 +1,23 @@
+/*
+ * 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 type { getErrorMessage } from '../../../../elasticsearch';
+
+export const mockGetEsErrorMessage = jest.fn() as jest.MockedFunction<typeof getErrorMessage>;
+
+jest.mock('../../../../elasticsearch', () => {
+  return { getErrorMessage: mockGetEsErrorMessage };
+});
+
+// Mock these functions to return empty results, as this simplifies test cases and we don't need to exercise alternate code paths for these
+jest.mock('@kbn/es-query', () => {
+  return { nodeTypes: { function: { buildNode: jest.fn() } } };
+});
+jest.mock('../search_dsl', () => {
+  return { getSearchDsl: jest.fn() };
+});

src/core/server/saved_objects/service/lib/legacy_url_aliases/delete_legacy_url_aliases.test.ts

@@ -6,13 +6,7 @@
  * Side Public License, v 1.
  */
 
-// Mock these functions to return empty results, as this simplifies test cases and we don't need to exercise alternate code paths for these
-jest.mock('@kbn/es-query', () => {
-  return { nodeTypes: { function: { buildNode: jest.fn() } } };
-});
-jest.mock('../search_dsl', () => {
-  return { getSearchDsl: jest.fn() };
-});
+import { mockGetEsErrorMessage } from './delete_legacy_url_aliases.test.mock'; // Note: importing this file applies default mocks for other functions too
 
 import { errors as EsErrors } from '@elastic/elasticsearch';
 
@@ -45,7 +39,7 @@ describe('deleteLegacyUrlAliases', () => {
     const namespaces = [ALL_NAMESPACES_STRING];
     const params = setup({ type, id, namespaces, deleteBehavior: 'inclusive' });
 
-    expect(() => deleteLegacyUrlAliases(params)).rejects.toThrowError(
+    await expect(() => deleteLegacyUrlAliases(params)).rejects.toThrowError(
       `Failed to delete legacy URL aliases for ${type}/${id}: "namespaces" cannot include the * string`
     );
     expect(params.client.updateByQuery).not.toHaveBeenCalled();
@@ -63,11 +57,15 @@ describe('deleteLegacyUrlAliases', () => {
     params.client.updateByQuery.mockResolvedValueOnce(
       elasticsearchClientMock.createErrorTransportRequestPromise(esError)
     );
+    mockGetEsErrorMessage.mockClear();
+    mockGetEsErrorMessage.mockReturnValue('Oh no!');
 
-    expect(() => deleteLegacyUrlAliases(params)).rejects.toThrowError(
-      `Failed to delete legacy URL aliases for ${type}/${id}: es_type, es_reason`
+    await expect(() => deleteLegacyUrlAliases(params)).rejects.toThrowError(
+      `Failed to delete legacy URL aliases for ${type}/${id}: Oh no!`
     );
     expect(params.client.updateByQuery).toHaveBeenCalledTimes(1);
+    expect(mockGetEsErrorMessage).toHaveBeenCalledTimes(1);
+    expect(mockGetEsErrorMessage).toHaveBeenCalledWith(esError);
   });
 
   describe('deleteBehavior "inclusive"', () => {

src/core/server/saved_objects/service/lib/legacy_url_aliases/delete_legacy_url_aliases.ts

@@ -8,6 +8,7 @@
 
 import * as esKuery from '@kbn/es-query';
 
+import { getErrorMessage as getEsErrorMessage } from '../../../../elasticsearch';
 import type { ISavedObjectTypeRegistry } from '../../../saved_objects_type_registry';
 import type { IndexMapping } from '../../../mappings';
 import { LEGACY_URL_ALIAS_TYPE } from '../../../object_types';
@@ -76,6 +77,10 @@ export async function deleteLegacyUrlAliases(params: DeleteLegacyUrlAliasesParam
         index: getIndexForType(LEGACY_URL_ALIAS_TYPE),
         refresh: false, // This could be called many times in succession, intentionally do not wait for a refresh
         body: {
+          ...getSearchDsl(mappings, registry, {
+            type: LEGACY_URL_ALIAS_TYPE,
+            kueryNode,
+          }),
           script: {
             // Intentionally use one script source with variable params to take advantage of ES script caching
             source: `
@@ -93,17 +98,13 @@ export async function deleteLegacyUrlAliases(params: DeleteLegacyUrlAliasesParam
             lang: 'painless',
           },
           conflicts: 'proceed',
-          ...getSearchDsl(mappings, registry, {
-            type: LEGACY_URL_ALIAS_TYPE,
-            kueryNode,
-          }),
         },
       },
       { ignore: [404] }
     );
   } catch (err) {
-    const { error } = err.body;
-    throwError(type, id, `${error.type}, ${error.reason}`);
+    const errorMessage = getEsErrorMessage(err);
+    throwError(type, id, `${errorMessage}`);
   }
 }