Expose isRetryableEsClientError

.github/CODEOWNERS

@@ -166,6 +166,7 @@ src/core/packages/elasticsearch/client-server-mocks @elastic/kibana-core
 src/core/packages/elasticsearch/server @elastic/kibana-core
 src/core/packages/elasticsearch/server-internal @elastic/kibana-core
 src/core/packages/elasticsearch/server-mocks @elastic/kibana-core
+src/core/packages/elasticsearch/server-utils @elastic/kibana-core
 src/core/packages/environment/server-internal @elastic/kibana-core
 src/core/packages/environment/server-mocks @elastic/kibana-core
 src/core/packages/execution-context/browser @elastic/kibana-core

package.json

@@ -322,6 +322,7 @@
     "@kbn/core-elasticsearch-client-server-internal": "link:src/core/packages/elasticsearch/client-server-internal",
     "@kbn/core-elasticsearch-server": "link:src/core/packages/elasticsearch/server",
     "@kbn/core-elasticsearch-server-internal": "link:src/core/packages/elasticsearch/server-internal",
+    "@kbn/core-elasticsearch-server-utils": "link:src/core/packages/elasticsearch/server-utils",
     "@kbn/core-environment-server-internal": "link:src/core/packages/environment/server-internal",
     "@kbn/core-execution-context-browser": "link:src/core/packages/execution-context/browser",
     "@kbn/core-execution-context-browser-internal": "link:src/core/packages/execution-context/browser-internal",

src/core/packages/elasticsearch/server-internal/index.ts

@@ -31,5 +31,4 @@ export { CoreElasticsearchRouteHandlerContext } from './src/elasticsearch_route_
 export { retryCallCluster, migrationRetryCallCluster } from './src/retry_call_cluster';
 export { isInlineScriptingEnabled } from './src/is_scripting_enabled';
 export { getCapabilitiesFromClient } from './src/get_capabilities';
-export { isRetryableEsClientError } from './src/retryable_es_client_errors';
 export type { ClusterInfo } from './src/get_cluster_info';

src/core/packages/elasticsearch/server-internal/src/is_scripting_enabled.test.ts

@@ -7,8 +7,8 @@
  * License v3.0 only", or the "Server Side Public License, v 1".
  */
 
-import { isRetryableEsClientErrorMock } from './is_scripting_enabled.test.mocks';
 import type { estypes } from '@elastic/elasticsearch';
+import { errors as esErrors } from '@elastic/elasticsearch';
 import { elasticsearchClientMock } from '@kbn/core-elasticsearch-client-server-mocks';
 import { isInlineScriptingEnabled } from './is_scripting_enabled';
 
@@ -98,55 +98,53 @@ describe('isInlineScriptingEnabled', () => {
   });
 
   describe('resiliency', () => {
-    beforeEach(() => {
-      isRetryableEsClientErrorMock.mockReset();
-    });
-
     const mockSuccessOnce = () => {
       client.cluster.getSettings.mockResolvedValueOnce({
         transient: {},
         persistent: {},
         defaults: {},
       });
     };
-    const mockErrorOnce = () => {
-      client.cluster.getSettings.mockResponseImplementationOnce(() => {
-        throw Error('ERR CON REFUSED');
-      });
+
+    const mockRetryableErrorOnce = () => {
+      client.cluster.getSettings.mockRejectedValueOnce(
+        new esErrors.ConnectionError(
+          'Connection failed',
+          elasticsearchClientMock.createApiResponse()
+        )
+      );
     };
 
-    it('retries the ES api call in case of retryable error', async () => {
-      isRetryableEsClientErrorMock.mockReturnValue(true);
+    const mockNonRetryableErrorOnce = () => {
+      client.cluster.getSettings.mockRejectedValueOnce(new Error('Non-retryable error'));
+    };
 
-      mockErrorOnce();
+    it('retries the ES api call in case of retryable error', async () => {
+      mockRetryableErrorOnce();
       mockSuccessOnce();
 
       await expect(isInlineScriptingEnabled({ client, maxRetryDelay: 1 })).resolves.toEqual(true);
       expect(client.cluster.getSettings).toHaveBeenCalledTimes(2);
     });
 
     it('throws in case of non-retryable error', async () => {
-      isRetryableEsClientErrorMock.mockReturnValue(false);
-
-      mockErrorOnce();
+      mockNonRetryableErrorOnce();
       mockSuccessOnce();
 
       await expect(isInlineScriptingEnabled({ client, maxRetryDelay: 0.1 })).rejects.toThrowError(
-        'ERR CON REFUSED'
+        'Non-retryable error'
       );
     });
 
     it('retries up to `maxRetries` times', async () => {
-      isRetryableEsClientErrorMock.mockReturnValue(true);
-
-      mockErrorOnce();
-      mockErrorOnce();
-      mockErrorOnce();
+      mockRetryableErrorOnce();
+      mockRetryableErrorOnce();
+      mockRetryableErrorOnce();
       mockSuccessOnce();
 
       await expect(
         isInlineScriptingEnabled({ client, maxRetryDelay: 0.1, maxRetries: 2 })
-      ).rejects.toThrowError('ERR CON REFUSED');
+      ).rejects.toThrowError('Connection failed');
       expect(client.cluster.getSettings).toHaveBeenCalledTimes(3);
     });
   });

src/core/packages/elasticsearch/server-internal/src/is_scripting_enabled.ts

@@ -8,8 +8,8 @@
  */
 
 import { defer, map, retry, timer, firstValueFrom, throwError } from 'rxjs';
+import { isRetryableEsClientError } from '@kbn/core-elasticsearch-server-utils';
 import type { ElasticsearchClient } from '@kbn/core-elasticsearch-server';
-import { isRetryableEsClientError } from './retryable_es_client_errors';
 
 const scriptAllowedTypesKey = 'script.allowed_types';
 

src/core/packages/elasticsearch/server-internal/tsconfig.json

@@ -24,6 +24,7 @@
     "@kbn/core-http-server-internal",
     "@kbn/core-execution-context-server-internal",
     "@kbn/core-elasticsearch-server",
+    "@kbn/core-elasticsearch-server-utils",
     "@kbn/core-elasticsearch-client-server-internal",
     "@kbn/core-test-helpers-deprecations-getters",
     "@kbn/config",

src/core/packages/elasticsearch/server-utils/README.md

@@ -0,0 +1,3 @@
+# @kbn/core-elasticsearch-server-utils
+
+Utilities for working with Elasticsearch

src/core/packages/elasticsearch/server-internal/src/is_scripting_enabled.test.mocks.ts → src/core/packages/elasticsearch/server-utils/index.ts

@@ -7,10 +7,4 @@
  * License v3.0 only", or the "Server Side Public License, v 1".
  */
 
-export const isRetryableEsClientErrorMock = jest.fn();
-
-jest.doMock('./retryable_es_client_errors', () => {
-  return {
-    isRetryableEsClientError: isRetryableEsClientErrorMock,
-  };
-});
+export { isRetryableEsClientError } from './src/is_retryable_es_client_error';

src/core/packages/elasticsearch/server-utils/jest.config.js

@@ -0,0 +1,14 @@
+/*
+ * 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", the "GNU Affero General Public License v3.0 only", 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", the "GNU Affero General Public
+ * License v3.0 only", or the "Server Side Public License, v 1".
+ */
+
+module.exports = {
+  preset: '@kbn/test/jest_node',
+  rootDir: '../../../../..',
+  roots: ['<rootDir>/src/core/packages/elasticsearch/server-utils'],
+};

src/core/packages/elasticsearch/server-utils/kibana.jsonc

@@ -0,0 +1,7 @@
+{
+  "type": "shared-common",
+  "id": "@kbn/core-elasticsearch-server-utils",
+  "owner": "@elastic/kibana-core",
+  "group": "platform",
+  "visibility": "shared"
+}

src/core/packages/elasticsearch/server-utils/package.json

@@ -0,0 +1,6 @@
+{
+  "name": "@kbn/core-elasticsearch-server-utils",
+  "private": true,
+  "version": "1.0.0",
+  "license": "Elastic License 2.0 OR AGPL-3.0-only OR SSPL-1.0"
+}

src/core/packages/elasticsearch/server-internal/src/retryable_es_client_errors.test.ts → src/core/packages/elasticsearch/server-utils/src/is_retryable_es_client_error.test.ts

@@ -9,7 +9,7 @@
 
 import { errors as esErrors } from '@elastic/elasticsearch';
 import { elasticsearchClientMock } from '@kbn/core-elasticsearch-client-server-mocks';
-import { isRetryableEsClientError } from './retryable_es_client_errors';
+import { isRetryableEsClientError } from './is_retryable_es_client_error';
 
 describe('isRetryableEsClientError', () => {
   describe('returns `false` for', () => {
@@ -51,27 +51,34 @@ describe('isRetryableEsClientError', () => {
       expect(isRetryableEsClientError(error)).toEqual(true);
     });
 
-    it('ResponseError of type snapshot_in_progress_exception', () => {
+    it.each([503, 504, 408, 410, 429])('ResponseError with %p status code', (statusCode) => {
       const error = new esErrors.ResponseError(
         elasticsearchClientMock.createApiResponse({
-          body: { error: { type: 'snapshot_in_progress_exception' } },
+          statusCode,
+          body: { error: { type: 'reason' } },
         })
       );
+
       expect(isRetryableEsClientError(error)).toEqual(true);
     });
 
-    it.each([503, 504, 401, 403, 408, 410, 429])(
-      'ResponseError with %p status code',
-      (statusCode) => {
-        const error = new esErrors.ResponseError(
-          elasticsearchClientMock.createApiResponse({
-            statusCode,
-            body: { error: { type: 'reason' } },
-          })
-        );
+    it('custom response status codes', () => {
+      const retryableError = new esErrors.ResponseError(
+        elasticsearchClientMock.createApiResponse({
+          statusCode: 418, // I'm a retryable teapot
+          body: { error: { type: 'reason' } },
+        })
+      );
 
-        expect(isRetryableEsClientError(error)).toEqual(true);
-      }
-    );
+      const nonRetryableError = new esErrors.ResponseError(
+        elasticsearchClientMock.createApiResponse({
+          statusCode: 503, // 503 is retryable by default but not in our custom retry codes
+          body: { error: { type: 'reason' } },
+        })
+      );
+
+      expect(isRetryableEsClientError(retryableError, [418])).toEqual(true);
+      expect(isRetryableEsClientError(nonRetryableError, [418])).toEqual(false);
+    });
   });
 });

src/core/packages/elasticsearch/server-internal/src/retryable_es_client_errors.ts → src/core/packages/elasticsearch/server-utils/src/is_retryable_es_client_error.ts

@@ -9,9 +9,7 @@
 
 import { errors as EsErrors } from '@elastic/elasticsearch';
 
-const retryResponseStatuses = [
-  401, // AuthorizationException
-  403, // AuthenticationException
+const DEFAULT_RETRY_STATUS_CODES = [
   408, // RequestTimeout
   410, // Gone
   429, // TooManyRequests -> ES circuit breaker
@@ -21,20 +19,32 @@ const retryResponseStatuses = [
 
 /**
  * Returns true if the given elasticsearch error should be retried
- * by retry-based resiliency systems such as the SO migration, false otherwise.
+ *
+ * Retryable errors include:
+ * - NoLivingConnectionsError
+ * - ConnectionError
+ * - TimeoutError
+ * - ResponseError with status codes:
+ *   - 408 RequestTimeout
+ *   - 410 Gone
+ *   - 429 TooManyRequests (ES circuit breaker)
+ *   - 503 ServiceUnavailable
+ *   - 504 GatewayTimeout
+ *   - OR custom status codes if provided
+ * @param e The error to check
+ * @param customRetryStatusCodes Custom response status codes to consider as retryable
+ * @returns true if the error is retryable, false otherwise
  */
-export const isRetryableEsClientError = (e: EsErrors.ElasticsearchClientError): boolean => {
+export const isRetryableEsClientError = (
+  e: EsErrors.ElasticsearchClientError,
+  customRetryStatusCodes?: number[]
+): boolean => {
   if (
     e instanceof EsErrors.NoLivingConnectionsError ||
     e instanceof EsErrors.ConnectionError ||
     e instanceof EsErrors.TimeoutError ||
     (e instanceof EsErrors.ResponseError &&
-      (retryResponseStatuses.includes(e?.statusCode!) ||
-        // ES returns a 400 Bad Request when trying to close or delete an
-        // index while snapshots are in progress. This should have been a 503
-        // so once https://github.com/elastic/elasticsearch/issues/65883 is
-        // fixed we can remove this.
-        e?.body?.error?.type === 'snapshot_in_progress_exception'))
+      (customRetryStatusCodes ?? DEFAULT_RETRY_STATUS_CODES).includes(e?.statusCode!))
   ) {
     return true;
   }

src/core/packages/elasticsearch/server-utils/tsconfig.json

@@ -0,0 +1,19 @@
+{
+  "extends": "../../../../../tsconfig.base.json",
+  "compilerOptions": {
+    "outDir": "target/types",
+    "types": [
+      "jest",
+      "node"
+    ]
+  },
+  "include": [
+    "**/*.ts"
+  ],
+  "kbn_references": [
+    "@kbn/core-elasticsearch-client-server-mocks"
+  ],
+  "exclude": [
+    "target/**/*",
+  ]
+}

src/core/packages/saved-objects/migration-server-internal/src/actions/catch_retryable_es_client_errors.test.ts

@@ -62,19 +62,6 @@ describe('catchRetryableEsClientErrors', () => {
         type: 'retryable_es_client_error',
       });
     });
-    it('ResponseError of type snapshot_in_progress_exception', async () => {
-      const error = new esErrors.ResponseError(
-        elasticsearchClientMock.createApiResponse({
-          body: { error: { type: 'snapshot_in_progress_exception' } },
-        })
-      );
-      expect(
-        ((await Promise.reject(error).catch(catchRetryableEsClientErrors)) as any).left
-      ).toMatchObject({
-        message: 'snapshot_in_progress_exception',
-        type: 'retryable_es_client_error',
-      });
-    });
     it.each([503, 401, 403, 408, 410, 429])(
       'ResponseError with retryable status code (%d)',
       async (status) => {

src/core/packages/saved-objects/migration-server-internal/src/actions/catch_retryable_es_client_errors.ts

@@ -9,18 +9,30 @@
 
 import * as Either from 'fp-ts/Either';
 import type { errors as EsErrors } from '@elastic/elasticsearch';
-import { isRetryableEsClientError } from '@kbn/core-elasticsearch-server-internal';
+import { isRetryableEsClientError } from '@kbn/core-elasticsearch-server-utils';
 
 export interface RetryableEsClientError {
   type: 'retryable_es_client_error';
   message: string;
   error?: Error;
 }
 
+// Migrations also retry on Auth exceptions as this is a common failure for newly created
+// clusters that might have misconfigured credentials.
+const retryResponseStatuses = [
+  401, // AuthorizationException
+  403, // AuthenticationException
+  408, // RequestTimeout
+  410, // Gone
+  429, // TooManyRequests -> ES circuit breaker
+  503, // ServiceUnavailable
+  504, // GatewayTimeout
+];
+
 export const catchRetryableEsClientErrors = (
   e: EsErrors.ElasticsearchClientError
 ): Either.Either<RetryableEsClientError, never> => {
-  if (isRetryableEsClientError(e)) {
+  if (isRetryableEsClientError(e, retryResponseStatuses)) {
     return Either.left({
       type: 'retryable_es_client_error' as const,
       message: e?.message,

src/core/packages/saved-objects/migration-server-internal/tsconfig.json

@@ -16,6 +16,7 @@
     "@kbn/std",
     "@kbn/core-doc-links-server",
     "@kbn/core-elasticsearch-server",
+    "@kbn/core-elasticsearch-server-utils",
     "@kbn/core-elasticsearch-client-server-internal",
     "@kbn/core-saved-objects-common",
     "@kbn/core-saved-objects-server",

tsconfig.base.json

@@ -412,6 +412,8 @@
       "@kbn/core-elasticsearch-server-internal/*": ["src/core/packages/elasticsearch/server-internal/*"],
       "@kbn/core-elasticsearch-server-mocks": ["src/core/packages/elasticsearch/server-mocks"],
       "@kbn/core-elasticsearch-server-mocks/*": ["src/core/packages/elasticsearch/server-mocks/*"],
+      "@kbn/core-elasticsearch-server-utils": ["src/core/packages/elasticsearch/server-utils"],
+      "@kbn/core-elasticsearch-server-utils/*": ["src/core/packages/elasticsearch/server-utils/*"],
       "@kbn/core-environment-server-internal": ["src/core/packages/environment/server-internal"],
       "@kbn/core-environment-server-internal/*": ["src/core/packages/environment/server-internal/*"],
       "@kbn/core-environment-server-mocks": ["src/core/packages/environment/server-mocks"],