try/catch on isExportable call and add exclusion reason

elastic · Jun 15, 2021 · a8ad0b6 · a8ad0b6
1 parent 55f03c2
commit a8ad0b6
Show file tree

Hide file tree

Showing 2 changed files with 71 additions and 17 deletions.
diff --git a/src/core/server/saved_objects/export/collect_exported_objects.test.ts b/src/core/server/saved_objects/export/collect_exported_objects.test.ts
@@ -12,7 +12,7 @@ import { httpServerMock } from '../../http/http_server.mocks';
 import { SavedObject, SavedObjectError } from '../../../types';
 import { SavedObjectTypeRegistry } from '../saved_objects_type_registry';
 import type { SavedObjectsExportTransform } from './types';
-import { collectExportedObjects } from './collect_exported_objects';
+import { collectExportedObjects, ExclusionReason } from './collect_exported_objects';
 import { SavedObjectsExportablePredicate } from '../types';
 
 const createObject = (parts: Partial<SavedObject>): SavedObject => ({
@@ -31,6 +31,11 @@ const createError = (parts: Partial<SavedObjectError> = {}): SavedObjectError =>
 });
 
 const toIdTuple = (obj: SavedObject) => ({ type: obj.type, id: obj.id });
+const toExcludedObject = (obj: SavedObject, reason: ExclusionReason = 'excluded') => ({
+  type: obj.type,
+  id: obj.id,
+  reason,
+});
 
 const toMap = <V>(record: Record<string, V>): Map<string, V> => new Map(Object.entries(record));
 
@@ -209,7 +214,45 @@ describe('collectExportedObjects', () => {
       });
 
       expect(objects).toEqual([foo1, bar3]);
-      expect(excludedObjects).toEqual([foo2].map(toIdTuple));
+      expect(excludedObjects).toEqual([foo2].map((obj) => toExcludedObject(obj)));
+    });
+
+    it('excludes objects when the predicate throws', async () => {
+      const foo1 = createObject({
+        type: 'foo',
+        id: '1',
+      });
+      const foo2 = createObject({
+        type: 'foo',
+        id: '2',
+      });
+      const bar3 = createObject({
+        type: 'bar',
+        id: '3',
+      });
+
+      registerType('foo', {
+        isExportable: (obj) => {
+          if (obj.id === '1') {
+            throw new Error('reason');
+          }
+          return true;
+        },
+      });
+      registerType('bar', { isExportable: () => true });
+
+      const { objects, excludedObjects } = await collectExportedObjects({
+        objects: [foo1, foo2, bar3],
+        savedObjectsClient,
+        request,
+        typeRegistry,
+        includeReferences: true,
+      });
+
+      expect(objects).toEqual([foo2, bar3]);
+      expect(excludedObjects).toEqual(
+        [foo1].map((obj) => toExcludedObject(obj, 'predicate_error'))
+      );
     });
 
     it('excludes references filtered by the `isExportable` predicate', async () => {
@@ -255,7 +298,7 @@ describe('collectExportedObjects', () => {
       });
 
       expect(objects).toEqual([foo1, bar2]);
-      expect(excludedObjects).toEqual([excluded1].map(toIdTuple));
+      expect(excludedObjects).toEqual([excluded1].map((obj) => toExcludedObject(obj)));
     });
 
     it('excludes additional objects filtered by the `isExportable` predicate', async () => {
@@ -291,7 +334,7 @@ describe('collectExportedObjects', () => {
       });
 
       expect(objects).toEqual([foo1, bar2]);
-      expect(excludedObjects).toEqual([excluded1].map(toIdTuple));
+      expect(excludedObjects).toEqual([excluded1].map((obj) => toExcludedObject(obj)));
     });
 
     it('returns the missing references', async () => {
@@ -729,7 +772,7 @@ describe('collectExportedObjects', () => {
       });
 
       expect(objects).toEqual([foo1, bar2, dolly3]);
-      expect(excludedObjects).toEqual([baz4].map(toIdTuple));
+      expect(excludedObjects).toEqual([baz4].map((obj) => toExcludedObject(obj)));
     });
   });
 

diff --git a/src/core/server/saved_objects/export/collect_exported_objects.ts b/src/core/server/saved_objects/export/collect_exported_objects.ts
@@ -35,9 +35,11 @@ interface CollectExportedObjectResult {
 interface ExcludedObject {
   id: string;
   type: string;
-  reason?: string;
+  reason: ExclusionReason;
 }
 
+export type ExclusionReason = 'predicate_error' | 'excluded';
+
 export const collectExportedObjects = async ({
   objects,
   includeReferences = true,
@@ -51,7 +53,7 @@ export const collectExportedObjects = async ({
 
   const collectedObjects: SavedObject[] = [];
   const collectedMissingRefs: CollectedReference[] = [];
-  const collectedNonExportableObjects: SavedObject[] = [];
+  const collectedNonExportableObjects: ExcludedObject[] = [];
   const alreadyProcessed: Set<string> = new Set();
 
   let currentObjects = objects;
@@ -110,10 +112,7 @@ export const collectExportedObjects = async ({
 
   return {
     objects: collectedObjects,
-    excludedObjects: collectedNonExportableObjects.map((obj) => ({
-      type: obj.type,
-      id: obj.id,
-    })),
+    excludedObjects: collectedNonExportableObjects,
     missingRefs: collectedMissingRefs,
   };
 };
@@ -195,14 +194,26 @@ const splitByExportability = (
   isExportable: SavedObjectsExportablePredicate<any>
 ) => {
   const exportableObjects: SavedObject[] = [];
-  const nonExportableObjects: SavedObject[] = [];
+  const nonExportableObjects: ExcludedObject[] = [];
 
   objects.forEach((obj) => {
-    const exportable = isExportable(obj);
-    if (exportable) {
-      exportableObjects.push(obj);
-    } else {
-      nonExportableObjects.push(obj);
+    try {
+      const exportable = isExportable(obj);
+      if (exportable) {
+        exportableObjects.push(obj);
+      } else {
+        nonExportableObjects.push({
+          id: obj.id,
+          type: obj.type,
+          reason: 'excluded',
+        });
+      }
+    } catch (e) {
+      nonExportableObjects.push({
+        id: obj.id,
+        type: obj.type,
+        reason: 'predicate_error',
+      });
     }
   });