Add context to integrationReader errors

Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
opensearch-project · Mar 5, 2025 · 6f88fd8 · 6f88fd8
1 parent 71bc489
commit 6f88fd8
Show file tree

Hide file tree

Showing 4 changed files with 69 additions and 21 deletions.
diff --git a/.eslintrc.js b/.eslintrc.js
@@ -39,6 +39,13 @@ module.exports = {
         ],
       },
     ],
+    'jest/expect-expect': [
+      'warn',
+      {
+        // Allow using custom expect test helpers as long as the name starts with `expect`.
+        assertFunctionNames: ["expect*"],
+      }
+    ]
   },
   overrides: [
     {

diff --git a/server/adaptors/integrations/__test__/local_fs_repository.test.ts b/server/adaptors/integrations/__test__/local_fs_repository.test.ts
@@ -49,25 +49,40 @@ describe('The local repository', () => {
   });
 });
 
+// Nginx and VPC are specifically used in other tests, so we add dedicated checks for them.
+
 describe('Local Nginx Integration', () => {
   it('Should serialize without errors', async () => {
     const integration = await repository.getIntegration('nginx');
 
-    await expect(integration?.serialize()).resolves.toEqual({
-      ok: true,
-      value: expect.anything(),
-    });
+    expect(integration).not.toBeNull();
+    expectOkResult(await integration!.serialize());
   });
 
-  it('Should serialize to include the config', async () => {
+  it('Should contain its config in its serialized form', async () => {
     const integration = await repository.getIntegration('nginx');
     const config = await integration!.getConfig();
     const serialized = await integration!.serialize();
 
-    expect(serialized).toEqual({
-      ok: true,
-      value: expect.anything(),
-    });
+    expectOkResult(serialized);
+    expect(serialized.value).toMatchObject(config.value!);
+  });
+});
+
+describe('Local VPC Integration', () => {
+  it('Should serialize without errors', async () => {
+    const integration = await repository.getIntegration('amazon_vpc_flow');
+
+    expect(integration).not.toBeNull();
+    expectOkResult(await integration!.serialize());
+  });
+
+  it('Should contain its config in its serialized form', async () => {
+    const integration = await repository.getIntegration('amazon_vpc_flow');
+    const config = await integration!.getConfig();
+    const serialized = await integration!.serialize();
+
+    expectOkResult(serialized);
     expect(serialized.value).toMatchObject(config.value!);
   });
 });
diff --git a/server/adaptors/integrations/repository/__test__/integration_reader.test.ts b/server/adaptors/integrations/repository/__test__/integration_reader.test.ts
@@ -8,6 +8,7 @@ import { IntegrationReader } from '../integration_reader';
 import { Dirent, Stats } from 'fs';
 import * as path from 'path';
 import { TEST_INTEGRATION_CONFIG } from '../../../../../test/constants';
+import { expectOkResult } from '../../__test__/custom_expects';
 
 jest.mock('fs/promises');
 
@@ -75,15 +76,15 @@ describe('Integration', () => {
 
       const result = await integration.getConfig(TEST_INTEGRATION_CONFIG.version);
 
-      expect(result.error?.message).toBe('data/version must be string');
+      expect(result.error?.message).toContain('data/version must be string');
     });
 
     it('should return an error if the config file has syntax errors', async () => {
       jest.spyOn(fs, 'readFile').mockResolvedValue('Invalid JSON');
 
       const result = await integration.getConfig(TEST_INTEGRATION_CONFIG.version);
 
-      expect(result.error?.message).toBe(
+      expect(result.error?.message).toContain(
         "Unable to parse file 'sample-2.0.0.json' as JSON or NDJson"
       );
     });
@@ -114,7 +115,7 @@ describe('Integration', () => {
 
       const result = await integration.getAssets(TEST_INTEGRATION_CONFIG.version);
 
-      expect(result.ok).toBe(true);
+      expectOkResult(result);
       expect((result as { value: Array<{ data: object[] }> }).value[0].data).toEqual([
         { name: 'asset1' },
         { name: 'asset2' },
@@ -136,7 +137,7 @@ describe('Integration', () => {
 
       const result = await integration.getAssets(TEST_INTEGRATION_CONFIG.version);
 
-      expect(result.error?.message).toBe(
+      expect(result.error?.message).toContain(
         "Unable to parse file 'sample-1.0.1.ndjson' as JSON or NDJson"
       );
     });
@@ -178,7 +179,7 @@ describe('Integration', () => {
       );
       const result = await integration.getSchemas();
 
-      expect(result.error?.message).toBe("data must have required property 'name'");
+      expect(result.error?.message).toContain("data must have required property 'name'");
     });
 
     it('should reject with an error if a mapping file is invalid', async () => {
@@ -188,7 +189,7 @@ describe('Integration', () => {
         .mockRejectedValueOnce(new Error('Could not load schema'));
 
       const result = await integration.getSchemas();
-      expect(result.error?.message).toBe('Could not load schema');
+      expect(result.error?.message).toContain('Could not load schema');
     });
   });
 
@@ -200,8 +201,8 @@ describe('Integration', () => {
 
       const result = await integration.getStatic('logo.png');
 
-      expect(result.ok).toBe(true);
-      expect((result as { value: unknown }).value).toStrictEqual(Buffer.from('logo data', 'ascii'));
+      expectOkResult(result);
+      expect(result.value).toStrictEqual(Buffer.from('logo data', 'ascii'));
       expect(readFileMock).toBeCalledWith(path.join('sample', 'static', 'logo.png'));
     });
 
@@ -247,7 +248,7 @@ describe('Integration', () => {
 
       const result = await integration.getSampleData();
 
-      expect(result.ok).toBe(true);
+      expectOkResult(result);
       expect((result as { value: { sampleData: unknown } }).value.sampleData).toBeNull();
     });
 
@@ -266,7 +267,9 @@ describe('Integration', () => {
 
       const result = await integration.getSampleData();
 
-      expect(result.error?.message).toBe("Unable to parse file 'sample.json' as JSON or NDJson");
+      expect(result.error?.message).toContain(
+        "Unable to parse file 'sample.json' as JSON or NDJson"
+      );
     });
   });
 });
diff --git a/server/adaptors/integrations/repository/integration_reader.ts b/server/adaptors/integrations/repository/integration_reader.ts
@@ -10,6 +10,18 @@ import { FileSystemDataAdaptor } from './fs_data_adaptor';
 import { CatalogDataAdaptor, IntegrationPart } from './catalog_data_adaptor';
 import { foldResults, pruneConfig } from './utils';
 
+interface FileParams {
+  filename: string;
+  type?: IntegrationPart;
+}
+
+const formatParams = (fp: FileParams): string => {
+  if (fp.type) {
+    return `\`${fp.filename}\` (type=${fp.type})`;
+  }
+  return `\`${fp.filename}\``;
+};
+
 /**
  * The Integration class represents the data for Integration Templates.
  * It is backed by the repository file system.
@@ -69,15 +81,26 @@ export class IntegrationReader {
           return { ok: true, value: Buffer.from(item.data, 'base64') };
         }
       } catch (error) {
+        error.message = `While parsing integration data for ${formatParams(fileParams)}:\n${
+          error.message
+        }`;
         return { ok: false, error };
       }
     }
 
+    let result: Result<object | object[]>;
     if (format === 'json') {
-      return this.reader.readFile(fileParams.filename, fileParams.type);
+      result = await this.reader.readFile(fileParams.filename, fileParams.type);
     } else {
-      return this.reader.readFileRaw(fileParams.filename, fileParams.type);
+      result = await this.reader.readFileRaw(fileParams.filename, fileParams.type);
+    }
+
+    if (!result.ok) {
+      result.error.message = `While reading integration data for ${formatParams(fileParams)}:\n${
+        result.error.message
+      }`;
     }
+    return result;
   }
 
   private async readAsset(