Skip to content

Commit 18a006e

Browse files
rudolfgsoldevilakibanamachine
authored
[8.19] Expose isRetryableEsClientError (#228315) (#244762)
# Backport This will backport the following commits from `main` to `8.19`: - [Expose isRetryableEsClientError (#228315)](#228315) <!--- Backport version: 10.2.0 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport) <!--BACKPORT [{"author":{"name":"Rudolf Meijering","email":"skaapgif@gmail.com"},"sourceCommit":{"committedDate":"2025-10-15T17:21:24Z","message":"Expose isRetryableEsClientError (#228315)\n\n## Summary\n\nFor plugins to have robust error handling they need to retry transient\nerrors. This PR exposes `isRetryableEsClientError` utility to help\nplugins know which errors are safe to retry.\n\n## Note for reviewers\nCore's isRetryableEsClientError typically retries on more status codes\nthan what most plugins do. Instead of keeping the existing plugin status\ncodes I've adopted the default behavior which means your plugins will be\nretrying on more types of responses after this change. Carefully check\nthat this makes sense.\n\nIn particular the two following status codes get retried that plugins\nweren't previously retrying:\n * - 429 TooManyRequests (ES circuit breaker)\n* - 504 GatewayTimeout (rare, but happens if cloud proxy cannot\nestablish socket connection to ES due to ES thread pool being\noverloaded)\n \n### Checklist\n\nCheck the PR satisfies following conditions. \n\nReviewers should verify this PR satisfies this list as well.\n\n- [ ] Any text added follows [EUI's writing\nguidelines](https://elastic.github.io/eui/#/guidelines/writing), uses\nsentence case text and includes [i18n\nsupport](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md)\n- [ ]\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\nwas added for features that require explanation or tutorials\n- [ ] [Unit or functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere updated or added to match the most common scenarios\n- [ ] If a plugin configuration key changed, check if it needs to be\nallowlisted in the cloud and added to the [docker\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)\n- [ ] This was checked for breaking HTTP API changes, and any breaking\nchanges have been approved by the breaking-change committee. The\n`release_note:breaking` label should be applied in these situations.\n- [ ] [Flaky Test\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was\nused on any tests changed\n- [ ] The PR description includes the appropriate Release Notes section,\nand the correct `release_note:*` label is applied per the\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\n- [ ] Review the [backport\nguidelines](https://docs.google.com/document/d/1VyN5k91e5OVumlc0Gb9RPa3h1ewuPE705nRtioPiTvY/edit?usp=sharing)\nand apply applicable `backport:*` labels.\n\n### Identify risks\n\nDoes this PR introduce any risks? For example, consider risks like hard\nto test bugs, performance regression, potential of data loss.\n\nDescribe the risk, its severity, and mitigation for each identified\nrisk. Invite stakeholders and evaluate how to proceed before merging.\n\n- [ ] [See some risk\nexamples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx)\n- [ ] ...\n\n---------\n\nCo-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>","sha":"c4996849bdd293721ec32a158320af89d79e6fcd","branchLabelMapping":{"^v9.3.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:Core","release_note:skip","backport:skip","Team:actionable-obs","v9.3.0"],"title":"Expose isRetryableEsClientError","number":228315,"url":"https://github.com/elastic/kibana/pull/228315","mergeCommit":{"message":"Expose isRetryableEsClientError (#228315)\n\n## Summary\n\nFor plugins to have robust error handling they need to retry transient\nerrors. This PR exposes `isRetryableEsClientError` utility to help\nplugins know which errors are safe to retry.\n\n## Note for reviewers\nCore's isRetryableEsClientError typically retries on more status codes\nthan what most plugins do. Instead of keeping the existing plugin status\ncodes I've adopted the default behavior which means your plugins will be\nretrying on more types of responses after this change. Carefully check\nthat this makes sense.\n\nIn particular the two following status codes get retried that plugins\nweren't previously retrying:\n * - 429 TooManyRequests (ES circuit breaker)\n* - 504 GatewayTimeout (rare, but happens if cloud proxy cannot\nestablish socket connection to ES due to ES thread pool being\noverloaded)\n \n### Checklist\n\nCheck the PR satisfies following conditions. \n\nReviewers should verify this PR satisfies this list as well.\n\n- [ ] Any text added follows [EUI's writing\nguidelines](https://elastic.github.io/eui/#/guidelines/writing), uses\nsentence case text and includes [i18n\nsupport](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md)\n- [ ]\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\nwas added for features that require explanation or tutorials\n- [ ] [Unit or functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere updated or added to match the most common scenarios\n- [ ] If a plugin configuration key changed, check if it needs to be\nallowlisted in the cloud and added to the [docker\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)\n- [ ] This was checked for breaking HTTP API changes, and any breaking\nchanges have been approved by the breaking-change committee. The\n`release_note:breaking` label should be applied in these situations.\n- [ ] [Flaky Test\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was\nused on any tests changed\n- [ ] The PR description includes the appropriate Release Notes section,\nand the correct `release_note:*` label is applied per the\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\n- [ ] Review the [backport\nguidelines](https://docs.google.com/document/d/1VyN5k91e5OVumlc0Gb9RPa3h1ewuPE705nRtioPiTvY/edit?usp=sharing)\nand apply applicable `backport:*` labels.\n\n### Identify risks\n\nDoes this PR introduce any risks? For example, consider risks like hard\nto test bugs, performance regression, potential of data loss.\n\nDescribe the risk, its severity, and mitigation for each identified\nrisk. Invite stakeholders and evaluate how to proceed before merging.\n\n- [ ] [See some risk\nexamples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx)\n- [ ] ...\n\n---------\n\nCo-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>","sha":"c4996849bdd293721ec32a158320af89d79e6fcd"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.3.0","branchLabelMappingKey":"^v9.3.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/228315","number":228315,"mergeCommit":{"message":"Expose isRetryableEsClientError (#228315)\n\n## Summary\n\nFor plugins to have robust error handling they need to retry transient\nerrors. This PR exposes `isRetryableEsClientError` utility to help\nplugins know which errors are safe to retry.\n\n## Note for reviewers\nCore's isRetryableEsClientError typically retries on more status codes\nthan what most plugins do. Instead of keeping the existing plugin status\ncodes I've adopted the default behavior which means your plugins will be\nretrying on more types of responses after this change. Carefully check\nthat this makes sense.\n\nIn particular the two following status codes get retried that plugins\nweren't previously retrying:\n * - 429 TooManyRequests (ES circuit breaker)\n* - 504 GatewayTimeout (rare, but happens if cloud proxy cannot\nestablish socket connection to ES due to ES thread pool being\noverloaded)\n \n### Checklist\n\nCheck the PR satisfies following conditions. \n\nReviewers should verify this PR satisfies this list as well.\n\n- [ ] Any text added follows [EUI's writing\nguidelines](https://elastic.github.io/eui/#/guidelines/writing), uses\nsentence case text and includes [i18n\nsupport](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md)\n- [ ]\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\nwas added for features that require explanation or tutorials\n- [ ] [Unit or functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere updated or added to match the most common scenarios\n- [ ] If a plugin configuration key changed, check if it needs to be\nallowlisted in the cloud and added to the [docker\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)\n- [ ] This was checked for breaking HTTP API changes, and any breaking\nchanges have been approved by the breaking-change committee. The\n`release_note:breaking` label should be applied in these situations.\n- [ ] [Flaky Test\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was\nused on any tests changed\n- [ ] The PR description includes the appropriate Release Notes section,\nand the correct `release_note:*` label is applied per the\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\n- [ ] Review the [backport\nguidelines](https://docs.google.com/document/d/1VyN5k91e5OVumlc0Gb9RPa3h1ewuPE705nRtioPiTvY/edit?usp=sharing)\nand apply applicable `backport:*` labels.\n\n### Identify risks\n\nDoes this PR introduce any risks? For example, consider risks like hard\nto test bugs, performance regression, potential of data loss.\n\nDescribe the risk, its severity, and mitigation for each identified\nrisk. Invite stakeholders and evaluate how to proceed before merging.\n\n- [ ] [See some risk\nexamples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx)\n- [ ] ...\n\n---------\n\nCo-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>","sha":"c4996849bdd293721ec32a158320af89d79e6fcd"}}]}] BACKPORT--> --------- Co-authored-by: Gerard Soldevila <gerard.soldevila@elastic.co> Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
1 parent 4945bef commit 18a006e

52 files changed

Lines changed: 225 additions & 301 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎.github/CODEOWNERS‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,7 @@ src/core/packages/elasticsearch/client-server-mocks @elastic/kibana-core
186186
src/core/packages/elasticsearch/server @elastic/kibana-core
187187
src/core/packages/elasticsearch/server-internal @elastic/kibana-core
188188
src/core/packages/elasticsearch/server-mocks @elastic/kibana-core
189+
src/core/packages/elasticsearch/server-utils @elastic/kibana-core
189190
src/core/packages/environment/server-internal @elastic/kibana-core
190191
src/core/packages/environment/server-mocks @elastic/kibana-core
191192
src/core/packages/execution-context/browser @elastic/kibana-core

‎package.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,7 @@
301301
"@kbn/core-elasticsearch-client-server-internal": "link:src/core/packages/elasticsearch/client-server-internal",
302302
"@kbn/core-elasticsearch-server": "link:src/core/packages/elasticsearch/server",
303303
"@kbn/core-elasticsearch-server-internal": "link:src/core/packages/elasticsearch/server-internal",
304+
"@kbn/core-elasticsearch-server-utils": "link:src/core/packages/elasticsearch/server-utils",
304305
"@kbn/core-environment-server-internal": "link:src/core/packages/environment/server-internal",
305306
"@kbn/core-execution-context-browser": "link:src/core/packages/execution-context/browser",
306307
"@kbn/core-execution-context-browser-internal": "link:src/core/packages/execution-context/browser-internal",

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,5 +31,4 @@ export { CoreElasticsearchRouteHandlerContext } from './src/elasticsearch_route_
3131
export { retryCallCluster, migrationRetryCallCluster } from './src/retry_call_cluster';
3232
export { isInlineScriptingEnabled } from './src/is_scripting_enabled';
3333
export { getCapabilitiesFromClient } from './src/get_capabilities';
34-
export { isRetryableEsClientError } from './src/retryable_es_client_errors';
3534
export type { ClusterInfo } from './src/get_cluster_info';

‎src/core/packages/elasticsearch/server-internal/moon.yml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ dependsOn:
3131
- '@kbn/core-http-server-internal'
3232
- '@kbn/core-execution-context-server-internal'
3333
- '@kbn/core-elasticsearch-server'
34+
- '@kbn/core-elasticsearch-server-utils'
3435
- '@kbn/core-elasticsearch-client-server-internal'
3536
- '@kbn/core-test-helpers-deprecations-getters'
3637
- '@kbn/config'

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

Lines changed: 21 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@
77
* License v3.0 only", or the "Server Side Public License, v 1".
88
*/
99

10-
import { isRetryableEsClientErrorMock } from './is_scripting_enabled.test.mocks';
11-
import * as estypes from '@elastic/elasticsearch/lib/api/types';
10+
import type { estypes } from '@elastic/elasticsearch';
11+
import { errors as esErrors } from '@elastic/elasticsearch';
1212
import { elasticsearchClientMock } from '@kbn/core-elasticsearch-client-server-mocks';
1313
import { isInlineScriptingEnabled } from './is_scripting_enabled';
1414

@@ -98,55 +98,53 @@ describe('isInlineScriptingEnabled', () => {
9898
});
9999

100100
describe('resiliency', () => {
101-
beforeEach(() => {
102-
isRetryableEsClientErrorMock.mockReset();
103-
});
104-
105101
const mockSuccessOnce = () => {
106102
client.cluster.getSettings.mockResolvedValueOnce({
107103
transient: {},
108104
persistent: {},
109105
defaults: {},
110106
});
111107
};
112-
const mockErrorOnce = () => {
113-
client.cluster.getSettings.mockResponseImplementationOnce(() => {
114-
throw Error('ERR CON REFUSED');
115-
});
108+
109+
const mockRetryableErrorOnce = () => {
110+
client.cluster.getSettings.mockRejectedValueOnce(
111+
new esErrors.ConnectionError(
112+
'Connection failed',
113+
elasticsearchClientMock.createApiResponse()
114+
)
115+
);
116116
};
117117

118-
it('retries the ES api call in case of retryable error', async () => {
119-
isRetryableEsClientErrorMock.mockReturnValue(true);
118+
const mockNonRetryableErrorOnce = () => {
119+
client.cluster.getSettings.mockRejectedValueOnce(new Error('Non-retryable error'));
120+
};
120121

121-
mockErrorOnce();
122+
it('retries the ES api call in case of retryable error', async () => {
123+
mockRetryableErrorOnce();
122124
mockSuccessOnce();
123125

124126
await expect(isInlineScriptingEnabled({ client, maxRetryDelay: 1 })).resolves.toEqual(true);
125127
expect(client.cluster.getSettings).toHaveBeenCalledTimes(2);
126128
});
127129

128130
it('throws in case of non-retryable error', async () => {
129-
isRetryableEsClientErrorMock.mockReturnValue(false);
130-
131-
mockErrorOnce();
131+
mockNonRetryableErrorOnce();
132132
mockSuccessOnce();
133133

134134
await expect(isInlineScriptingEnabled({ client, maxRetryDelay: 0.1 })).rejects.toThrowError(
135-
'ERR CON REFUSED'
135+
'Non-retryable error'
136136
);
137137
});
138138

139139
it('retries up to `maxRetries` times', async () => {
140-
isRetryableEsClientErrorMock.mockReturnValue(true);
141-
142-
mockErrorOnce();
143-
mockErrorOnce();
144-
mockErrorOnce();
140+
mockRetryableErrorOnce();
141+
mockRetryableErrorOnce();
142+
mockRetryableErrorOnce();
145143
mockSuccessOnce();
146144

147145
await expect(
148146
isInlineScriptingEnabled({ client, maxRetryDelay: 0.1, maxRetries: 2 })
149-
).rejects.toThrowError('ERR CON REFUSED');
147+
).rejects.toThrowError('Connection failed');
150148
expect(client.cluster.getSettings).toHaveBeenCalledTimes(3);
151149
});
152150
});

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@
88
*/
99

1010
import { defer, map, retry, timer, firstValueFrom, throwError } from 'rxjs';
11+
import { isRetryableEsClientError } from '@kbn/core-elasticsearch-server-utils';
1112
import type { ElasticsearchClient } from '@kbn/core-elasticsearch-server';
12-
import { isRetryableEsClientError } from './retryable_es_client_errors';
1313

1414
const scriptAllowedTypesKey = 'script.allowed_types';
1515

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
"@kbn/core-http-server-internal",
2525
"@kbn/core-execution-context-server-internal",
2626
"@kbn/core-elasticsearch-server",
27+
"@kbn/core-elasticsearch-server-utils",
2728
"@kbn/core-elasticsearch-client-server-internal",
2829
"@kbn/core-test-helpers-deprecations-getters",
2930
"@kbn/config",

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
# @kbn/core-elasticsearch-server-utils
2+
3+
Utilities for working with Elasticsearch

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

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,4 @@
77
* License v3.0 only", or the "Server Side Public License, v 1".
88
*/
99

10-
export const isRetryableEsClientErrorMock = jest.fn();
11-
12-
jest.doMock('./retryable_es_client_errors', () => {
13-
return {
14-
isRetryableEsClientError: isRetryableEsClientErrorMock,
15-
};
16-
});
10+
export { isRetryableEsClientError } from './src/is_retryable_es_client_error';

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the "Elastic License
4+
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
5+
* Public License v 1"; you may not use this file except in compliance with, at
6+
* your election, the "Elastic License 2.0", the "GNU Affero General Public
7+
* License v3.0 only", or the "Server Side Public License, v 1".
8+
*/
9+
10+
module.exports = {
11+
preset: '@kbn/test/jest_node',
12+
rootDir: '../../../../..',
13+
roots: ['<rootDir>/src/core/packages/elasticsearch/server-utils'],
14+
};

0 commit comments

Comments
 (0)