Fixes bugs for product check in update and delete (#109979) (#110280)

* Fixes bugs for product check in update and delete * Apply suggestions from code review Co-authored-by: Christiane (Tina) Heiligers <christiane.heiligers@elastic.co>
elastic · Aug 26, 2021 · e50d565 · e50d565
1 parent 0151a82
commit e50d565
Show file tree

Hide file tree

Showing 4 changed files with 32 additions and 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
@@ -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
@@ -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
@@ -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
@@ -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;