engine: return slightly better credential errors (#693)

* engine: updated credential error handling to add a message * engine: fix test * worker: update test * tests: add test for bad credential * version: [email protected] * tests: update error message
OpenFn · May 21, 2024 · 015b7f2 · 015b7f2
1 parent 472e0ec
commit 015b7f2
Show file tree

Hide file tree

Showing 14 changed files with 111 additions and 12 deletions.
diff --git a/integration-tests/worker/CHANGELOG.md b/integration-tests/worker/CHANGELOG.md
@@ -1,5 +1,14 @@
 # @openfn/integration-tests-worker
 
+## 1.0.44
+
+### Patch Changes
+
+- Updated dependencies
+  - @openfn/engine-multi@1.1.8
+  - @openfn/ws-worker@1.1.9
+  - @openfn/lightning-mock@2.0.8
+
 ## 1.0.43
 
 ### Patch Changes

diff --git a/integration-tests/worker/package.json b/integration-tests/worker/package.json
@@ -1,7 +1,7 @@
 {
   "name": "@openfn/integration-tests-worker",
   "private": true,
-  "version": "1.0.43",
+  "version": "1.0.44",
   "description": "Lightning WOrker integration tests",
   "author": "Open Function Group <[email protected]>",
   "license": "ISC",

diff --git a/integration-tests/worker/test/exit-reasons.test.ts b/integration-tests/worker/test/exit-reasons.test.ts
@@ -76,6 +76,30 @@ test('exception: autoinstall error', async (t) => {
   );
 });
 
+test('exception: bad credential (not found)', async (t) => {
+  const attempt = {
+    id: crypto.randomUUID(),
+    jobs: [
+      {
+        adaptor: '@openfn/language-common@latest',
+        body: 'fn((s) => s)',
+        credential: 'been-to-the-mountain', // the mock will return not_found
+      },
+    ],
+  };
+
+  const result = await run(attempt);
+
+  const { reason, error_type, error_message } = result;
+
+  t.is(reason, 'exception');
+  t.is(error_type, 'CredentialLoadError');
+  t.is(
+    error_message,
+    'Failed to load credential been-to-the-mountain: not_found'
+  );
+});
+
 test('kill: oom (small, kill worker)', async (t) => {
   const attempt = {
     id: crypto.randomUUID(),

diff --git a/integration-tests/worker/test/integration.test.ts b/integration-tests/worker/test/integration.test.ts
@@ -309,7 +309,10 @@ test.serial('run a job with bad credentials', (t) => {
     lightning.once('run:complete', ({ payload }) => {
       t.is(payload.reason, 'exception');
       t.is(payload.error_type, 'CredentialLoadError');
-      t.regex(payload.error_message, /Failed to load credential zzz for step/);
+      t.regex(
+        payload.error_message,
+        /Failed to load credential zzz: not_found/
+      );
       done();
     });
 

diff --git a/packages/engine-multi/CHANGELOG.md b/packages/engine-multi/CHANGELOG.md
@@ -1,5 +1,11 @@
 # engine-multi
 
+## 1.1.8
+
+### Patch Changes
+
+- Better error reporting for bad credentials
+
 ## 1.1.7
 
 ### Patch Changes

diff --git a/packages/engine-multi/package.json b/packages/engine-multi/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@openfn/engine-multi",
-  "version": "1.1.7",
+  "version": "1.1.8",
   "description": "Multi-process runtime engine",
   "main": "dist/index.js",
   "type": "module",

diff --git a/packages/engine-multi/src/errors.ts b/packages/engine-multi/src/errors.ts
@@ -114,8 +114,18 @@ export class CredentialLoadError extends EngineError {
   original: any; // this is the original error
   constructor(errors: CredentialErrorObj[]) {
     super();
+    const ids: Record<string, true> = {};
     this.message = errors
-      .map((e) => `Failed to load credential ${e.id} for step ${e.step}`)
+      .filter((e) => {
+        if (!ids[e.id]) {
+          ids[e.id] = true;
+          return true;
+        }
+      })
+      .map(
+        (e) =>
+          `Failed to load credential ${e.id}${e.error ? `: ${e.error}` : ''}`
+      )
       .join('\n');
   }
 }
diff --git a/packages/engine-multi/test/api/preload-credentials.test.ts b/packages/engine-multi/test/api/preload-credentials.test.ts
@@ -97,7 +97,7 @@ test('throw if one credential fails to load', async (t) => {
     await preloadCredentials(plan, loader);
   } catch (e: any) {
     t.is(e.name, 'CredentialLoadError');
-    t.is(e.message, `Failed to load credential a for step z`);
+    t.is(e.message, `Failed to load credential a: err`);
   }
 });
 
@@ -118,7 +118,7 @@ test('throw if several credentials fail to load', async (t) => {
         {
           id: 'k',
           expression: '.',
-          configuration: 'a',
+          configuration: 'b',
         },
       ],
     },
@@ -131,8 +131,40 @@ test('throw if several credentials fail to load', async (t) => {
     t.is(e.name, 'CredentialLoadError');
     t.is(
       e.message,
-      `Failed to load credential a for step j
-Failed to load credential a for step k`
+      `Failed to load credential a: err
+Failed to load credential b: err`
     );
   }
 });
+
+test('only report each credential once', async (t) => {
+  const loader = async () => {
+    throw new Error('err');
+  };
+
+  const plan = {
+    id: t.title,
+    workflow: {
+      steps: [
+        {
+          id: 'j',
+          expression: '.',
+          configuration: 'a',
+        },
+        {
+          id: 'k',
+          expression: '.',
+          configuration: 'a',
+        },
+      ],
+    },
+    options: {},
+  } as ExecutionPlan;
+
+  try {
+    await preloadCredentials(plan, loader);
+  } catch (e: any) {
+    t.is(e.name, 'CredentialLoadError');
+    t.is(e.message, `Failed to load credential a: err`);
+  }
+});
diff --git a/packages/engine-multi/test/integration.test.ts b/packages/engine-multi/test/integration.test.ts
@@ -446,7 +446,7 @@ test.serial('send a workflow error if credentials fail to load', (t) => {
 
       t.is(e.type, 'CredentialLoadError');
       t.is(e.severity, 'exception');
-      t.is(e.message, 'Failed to load credential secret for step a');
+      t.is(e.message, 'Failed to load credential secret');
       done();
     });
   });

diff --git a/packages/lightning-mock/CHANGELOG.md b/packages/lightning-mock/CHANGELOG.md
@@ -1,5 +1,12 @@
 # @openfn/lightning-mock
 
+## 2.0.8
+
+### Patch Changes
+
+- Updated dependencies
+  - @openfn/engine-multi@1.1.8
+
 ## 2.0.7
 
 ### Patch Changes

diff --git a/packages/lightning-mock/package.json b/packages/lightning-mock/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@openfn/lightning-mock",
-  "version": "2.0.7",
+  "version": "2.0.8",
   "private": true,
   "description": "A mock Lightning server",
   "main": "dist/index.js",

diff --git a/packages/ws-worker/CHANGELOG.md b/packages/ws-worker/CHANGELOG.md
@@ -1,5 +1,13 @@
 # ws-worker
 
+## 1.1.9
+
+### Patch Changes
+
+- Better error reporting for bad credentials
+- Updated dependencies
+  - @openfn/engine-multi@1.1.8
+
 ## 1.1.8
 
 ### Patch Changes

diff --git a/packages/ws-worker/package.json b/packages/ws-worker/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@openfn/ws-worker",
-  "version": "1.1.8",
+  "version": "1.1.9",
   "description": "A Websocket Worker to connect Lightning to a Runtime Engine",
   "main": "dist/index.js",
   "type": "module",

diff --git a/packages/ws-worker/test/reasons.test.ts b/packages/ws-worker/test/reasons.test.ts
@@ -236,7 +236,7 @@ test('exception: failed to load credential', async (t) => {
 
   t.is(reason.reason, 'exception');
   t.is(reason.error_type, 'CredentialLoadError');
-  t.is(reason.error_message, 'Failed to load credential zzz for step aa');
+  t.is(reason.error_message, 'Failed to load credential zzz: err');
 });
 
 test('kill: timeout', async (t) => {