do not crash when es client throws any type of error (#99175)

This commit is contained in:
Mikhail Shustov 2021-05-05 14:39:55 +02:00 committed by GitHub
parent 47cebf0ee1
commit 2eb700be92
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 77 additions and 39 deletions

View file

@ -31,16 +31,16 @@ export const catchRetryableEsClientErrors = (
e instanceof EsErrors.ConnectionError ||
e instanceof EsErrors.TimeoutError ||
(e instanceof EsErrors.ResponseError &&
(retryResponseStatuses.includes(e.statusCode) ||
(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'))
e?.body?.error?.type === 'snapshot_in_progress_exception'))
) {
return Either.left({
type: 'retryable_es_client_error' as const,
message: e.message,
message: e?.message,
error: e,
});
} else {

View file

@ -30,6 +30,11 @@ describe('actions', () => {
elasticsearchClientMock.createErrorTransportRequestPromise(retryableError)
);
const nonRetryableError = new Error('crash');
const clientWithNonRetryableError = elasticsearchClientMock.createInternalClient(
elasticsearchClientMock.createErrorTransportRequestPromise(nonRetryableError)
);
describe('fetchIndices', () => {
it('calls catchRetryableEsClientErrors when the promise rejects', async () => {
const task = Actions.fetchIndices(client, ['my_index']);
@ -52,6 +57,11 @@ describe('actions', () => {
}
expect(catchRetryableEsClientErrors).toHaveBeenCalledWith(retryableError);
});
it('re-throws non retry-able errors', async () => {
const task = Actions.setWriteBlock(clientWithNonRetryableError, 'my_index');
await task();
expect(catchRetryableEsClientErrors).toHaveBeenCalledWith(nonRetryableError);
});
});
describe('cloneIndex', () => {
@ -64,6 +74,11 @@ describe('actions', () => {
}
expect(catchRetryableEsClientErrors).toHaveBeenCalledWith(retryableError);
});
it('re-throws non retry-able errors', async () => {
const task = Actions.setWriteBlock(clientWithNonRetryableError, 'my_index');
await task();
expect(catchRetryableEsClientErrors).toHaveBeenCalledWith(nonRetryableError);
});
});
describe('pickupUpdatedMappings', () => {
@ -156,6 +171,11 @@ describe('actions', () => {
expect(catchRetryableEsClientErrors).toHaveBeenCalledWith(retryableError);
});
it('re-throws non retry-able errors', async () => {
const task = Actions.setWriteBlock(clientWithNonRetryableError, 'my_index');
await task();
expect(catchRetryableEsClientErrors).toHaveBeenCalledWith(nonRetryableError);
});
});
describe('waitForPickupUpdatedMappingsTask', () => {
@ -169,6 +189,11 @@ describe('actions', () => {
expect(catchRetryableEsClientErrors).toHaveBeenCalledWith(retryableError);
});
it('re-throws non retry-able errors', async () => {
const task = Actions.setWriteBlock(clientWithNonRetryableError, 'my_index');
await task();
expect(catchRetryableEsClientErrors).toHaveBeenCalledWith(nonRetryableError);
});
});
describe('updateAliases', () => {
@ -182,6 +207,11 @@ describe('actions', () => {
expect(catchRetryableEsClientErrors).toHaveBeenCalledWith(retryableError);
});
it('re-throws non retry-able errors', async () => {
const task = Actions.setWriteBlock(clientWithNonRetryableError, 'my_index');
await task();
expect(catchRetryableEsClientErrors).toHaveBeenCalledWith(nonRetryableError);
});
});
describe('createIndex', () => {
@ -195,6 +225,11 @@ describe('actions', () => {
expect(catchRetryableEsClientErrors).toHaveBeenCalledWith(retryableError);
});
it('re-throws non retry-able errors', async () => {
const task = Actions.setWriteBlock(clientWithNonRetryableError, 'my_index');
await task();
expect(catchRetryableEsClientErrors).toHaveBeenCalledWith(nonRetryableError);
});
});
describe('updateAndPickupMappings', () => {

View file

@ -107,34 +107,37 @@ export const setWriteBlock = (
IndexNotFound | RetryableEsClientError,
'set_write_block_succeeded'
> => () => {
return client.indices
.addBlock<{
acknowledged: boolean;
shards_acknowledged: boolean;
}>(
{
index,
block: 'write',
},
{ maxRetries: 0 /** handle retry ourselves for now */ }
)
.then((res: any) => {
return res.body.acknowledged === true
? Either.right('set_write_block_succeeded' as const)
: Either.left({
type: 'retryable_es_client_error' as const,
message: 'set_write_block_failed',
});
})
.catch((e: ElasticsearchClientError) => {
if (e instanceof EsErrors.ResponseError) {
if (e.message === 'index_not_found_exception') {
return Either.left({ type: 'index_not_found_exception' as const, index });
return (
client.indices
.addBlock<{
acknowledged: boolean;
shards_acknowledged: boolean;
}>(
{
index,
block: 'write',
},
{ maxRetries: 0 /** handle retry ourselves for now */ }
)
// not typed yet
.then((res: any) => {
return res.body.acknowledged === true
? Either.right('set_write_block_succeeded' as const)
: Either.left({
type: 'retryable_es_client_error' as const,
message: 'set_write_block_failed',
});
})
.catch((e: ElasticsearchClientError) => {
if (e instanceof EsErrors.ResponseError) {
if (e.body?.error?.type === 'index_not_found_exception') {
return Either.left({ type: 'index_not_found_exception' as const, index });
}
}
}
throw e;
})
.catch(catchRetryableEsClientErrors);
throw e;
})
.catch(catchRetryableEsClientErrors)
);
};
/**
@ -263,12 +266,12 @@ export const cloneIndex = (
});
})
.catch((error: EsErrors.ResponseError) => {
if (error.body.error.type === 'index_not_found_exception') {
if (error?.body?.error?.type === 'index_not_found_exception') {
return Either.left({
type: 'index_not_found_exception' as const,
index: error.body.error.index,
});
} else if (error.body.error.type === 'resource_already_exists_exception') {
} else if (error?.body?.error?.type === 'resource_already_exists_exception') {
/**
* If the target index already exists it means a previous clone
* operation had already been started. However, we can't be sure
@ -795,22 +798,22 @@ export const updateAliases = (
})
.catch((err: EsErrors.ElasticsearchClientError) => {
if (err instanceof EsErrors.ResponseError) {
if (err.body.error.type === 'index_not_found_exception') {
if (err?.body?.error?.type === 'index_not_found_exception') {
return Either.left({
type: 'index_not_found_exception' as const,
index: err.body.error.index,
});
} else if (
err.body.error.type === 'illegal_argument_exception' &&
err.body.error.reason.match(
err?.body?.error?.type === 'illegal_argument_exception' &&
err?.body?.error?.reason?.match(
/The provided expression \[.+\] matches an alias, specify the corresponding concrete indices instead./
)
) {
return Either.left({ type: 'remove_index_not_a_concrete_index' as const });
} else if (
err.body.error.type === 'aliases_not_found_exception' ||
(err.body.error.type === 'resource_not_found_exception' &&
err.body.error.reason.match(/required alias \[.+\] does not exist/))
err?.body?.error?.type === 'aliases_not_found_exception' ||
(err?.body?.error?.type === 'resource_not_found_exception' &&
err?.body?.error?.reason?.match(/required alias \[.+\] does not exist/))
) {
return Either.left({
type: 'alias_not_found_exception' as const,
@ -903,7 +906,7 @@ export const createIndex = (
});
})
.catch((error) => {
if (error.body.error.type === 'resource_already_exists_exception') {
if (error?.body?.error?.type === 'resource_already_exists_exception') {
/**
* If the target index already exists it means a previous create
* operation had already been started. However, we can't be sure