From b15b3ce19ddcda9e91dd8c3d1369a9daeff64f64 Mon Sep 17 00:00:00 2001 From: "Christiane (Tina) Heiligers" Date: Thu, 26 Aug 2021 09:03:13 -0700 Subject: [PATCH] Fixes bugs for product check in update and delete (#109979) * Fixes bugs for product check in update and delete * Apply suggestions from code review --- .../service/lib/decorate_es_error.test.ts | 12 +++++++++ .../service/lib/decorate_es_error.ts | 6 +++++ .../service/lib/repository.test.js | 22 ++++++---------- .../saved_objects/service/lib/repository.ts | 25 ++++++------------- 4 files changed, 32 insertions(+), 33 deletions(-) diff --git a/src/core/server/saved_objects/service/lib/decorate_es_error.test.ts b/src/core/server/saved_objects/service/lib/decorate_es_error.test.ts index 9dd8959d49293..4e187e85f81a7 100644 --- a/src/core/server/saved_objects/service/lib/decorate_es_error.test.ts +++ b/src/core/server/saved_objects/service/lib/decorate_es_error.test.ts @@ -109,6 +109,18 @@ describe('savedObjectsClient/decorateEsError', () => { expect(SavedObjectsErrorHelpers.isNotFoundError(genericError)).toBe(true); }); + it('makes NotFound errors generic NotFoundEsUnavailableError errors when response is from unsupported server', () => { + const error = new esErrors.ResponseError( + // explicitly override the headers + elasticsearchClientMock.createApiResponse({ statusCode: 404, headers: {} }) + ); + expect(SavedObjectsErrorHelpers.isNotFoundError(error)).toBe(false); + const genericError = decorateEsError(error); + expect(genericError).not.toBe(error); + expect(SavedObjectsErrorHelpers.isNotFoundError(genericError)).toBe(false); + expect(SavedObjectsErrorHelpers.isEsUnavailableError(genericError)).toBe(true); + }); + it('if saved objects index does not exist makes NotFound a SavedObjectsClient/generalError', () => { const error = new esErrors.ResponseError( elasticsearchClientMock.createApiResponse({ diff --git a/src/core/server/saved_objects/service/lib/decorate_es_error.ts b/src/core/server/saved_objects/service/lib/decorate_es_error.ts index e1aa1ab2f956d..016268ccdf9f4 100644 --- a/src/core/server/saved_objects/service/lib/decorate_es_error.ts +++ b/src/core/server/saved_objects/service/lib/decorate_es_error.ts @@ -8,6 +8,7 @@ import { errors as esErrors } from '@elastic/elasticsearch'; import { get } from 'lodash'; +import { isSupportedEsServer } from '../../../elasticsearch'; const responseErrors = { isServiceUnavailable: (statusCode: number) => statusCode === 503, @@ -69,6 +70,11 @@ export function decorateEsError(error: EsErrors) { if (match?.length > 0) { return SavedObjectsErrorHelpers.decorateIndexAliasNotFoundError(error, match[1]); } + // Throw EsUnavailable error if the 404 is not from elasticsearch + // Needed here to verify Product support for any non-ignored 404 responses from calls to ES + if (!isSupportedEsServer(error?.meta?.headers)) { + return SavedObjectsErrorHelpers.createGenericNotFoundEsUnavailableError(); + } return SavedObjectsErrorHelpers.createGenericNotFoundError(); } diff --git a/src/core/server/saved_objects/service/lib/repository.test.js b/src/core/server/saved_objects/service/lib/repository.test.js index d4d794c165fe2..427c28ceb326c 100644 --- a/src/core/server/saved_objects/service/lib/repository.test.js +++ b/src/core/server/saved_objects/service/lib/repository.test.js @@ -2439,27 +2439,19 @@ describe('SavedObjectsRepository', () => { it(`throws when ES is unable to find the document during delete`, async () => { client.delete.mockResolvedValueOnce( - elasticsearchClientMock.createSuccessTransportRequestPromise( - { result: 'not_found' }, - {}, - {} - ) + elasticsearchClientMock.createSuccessTransportRequestPromise({ result: 'not_found' }) ); - await expectNotFoundEsUnavailableError(type, id); + await expectNotFoundError(type, id); expect(client.delete).toHaveBeenCalledTimes(1); }); it(`throws when ES is unable to find the index during delete`, async () => { client.delete.mockResolvedValueOnce( - elasticsearchClientMock.createSuccessTransportRequestPromise( - { - error: { type: 'index_not_found_exception' }, - }, - {}, - {} - ) + elasticsearchClientMock.createSuccessTransportRequestPromise({ + error: { type: 'index_not_found_exception' }, + }) ); - await expectNotFoundEsUnavailableError(type, id); + await expectNotFoundError(type, id); expect(client.delete).toHaveBeenCalledTimes(1); }); @@ -3430,7 +3422,7 @@ describe('SavedObjectsRepository', () => { client.get.mockResolvedValueOnce( elasticsearchClientMock.createSuccessTransportRequestPromise( { found: false }, - undefined, + { statusCode: 404 }, {} ) ); diff --git a/src/core/server/saved_objects/service/lib/repository.ts b/src/core/server/saved_objects/service/lib/repository.ts index c2ae6ebab00e9..7ce3a55d057eb 100644 --- a/src/core/server/saved_objects/service/lib/repository.ts +++ b/src/core/server/saved_objects/service/lib/repository.ts @@ -682,6 +682,10 @@ export class SavedObjectsRepository { { ignore: [404] } ); + if (isNotFoundFromUnsupportedServer({ statusCode, headers })) { + throw SavedObjectsErrorHelpers.createGenericNotFoundEsUnavailableError(type, id); + } + const deleted = body.result === 'deleted'; if (deleted) { return {}; @@ -690,15 +694,9 @@ export class SavedObjectsRepository { const deleteDocNotFound = body.result === 'not_found'; // @ts-expect-error @elastic/elasticsearch doesn't declare error on DeleteResponse const deleteIndexNotFound = body.error && body.error.type === 'index_not_found_exception'; - const esServerSupported = isSupportedEsServer(headers); if (deleteDocNotFound || deleteIndexNotFound) { - if (esServerSupported) { - // see "404s from missing index" above - throw SavedObjectsErrorHelpers.createGenericNotFoundError(type, id); - } else { - // throw if we can't verify the response is from Elasticsearch - throw SavedObjectsErrorHelpers.createGenericNotFoundEsUnavailableError(type, id); - } + // see "404s from missing index" above + throw SavedObjectsErrorHelpers.createGenericNotFoundError(type, id); } throw new Error( @@ -1084,7 +1082,7 @@ export class SavedObjectsRepository { ); const indexNotFound = statusCode === 404; // check if we have the elasticsearch header when index is not found and if we do, ensure it is Elasticsearch - if (!isFoundGetResponse(body) && !isSupportedEsServer(headers)) { + if (indexNotFound && !isSupportedEsServer(headers)) { throw SavedObjectsErrorHelpers.createGenericNotFoundEsUnavailableError(type, id); } if ( @@ -1331,15 +1329,6 @@ export class SavedObjectsRepository { _source_includes: ['namespace', 'namespaces', 'originId'], require_alias: true, }) - .then((res) => { - const indexNotFound = res.statusCode === 404; - const esServerSupported = isSupportedEsServer(res.headers); - // check if we have the elasticsearch header when index is not found and if we do, ensure it is Elasticsearch - if (indexNotFound && !esServerSupported) { - throw SavedObjectsErrorHelpers.createGenericNotFoundEsUnavailableError(type, id); - } - return res; - }) .catch((err) => { if (SavedObjectsErrorHelpers.isEsUnavailableError(err)) { throw err;