From 015b7f260163ac9f2ae3f2430f56f44318920d85 Mon Sep 17 00:00:00 2001 From: josephjclark Date: Tue, 21 May 2024 10:46:05 +0100 Subject: [PATCH] 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: worker@1.1.9 * tests: update error message --- integration-tests/worker/CHANGELOG.md | 9 +++++ integration-tests/worker/package.json | 2 +- .../worker/test/exit-reasons.test.ts | 24 +++++++++++ .../worker/test/integration.test.ts | 5 ++- packages/engine-multi/CHANGELOG.md | 6 +++ packages/engine-multi/package.json | 2 +- packages/engine-multi/src/errors.ts | 12 +++++- .../test/api/preload-credentials.test.ts | 40 +++++++++++++++++-- .../engine-multi/test/integration.test.ts | 2 +- packages/lightning-mock/CHANGELOG.md | 7 ++++ packages/lightning-mock/package.json | 2 +- packages/ws-worker/CHANGELOG.md | 8 ++++ packages/ws-worker/package.json | 2 +- packages/ws-worker/test/reasons.test.ts | 2 +- 14 files changed, 111 insertions(+), 12 deletions(-) diff --git a/integration-tests/worker/CHANGELOG.md b/integration-tests/worker/CHANGELOG.md index 53cf52614..36d2ca2b3 100644 --- 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 index f1134f4bb..5a638dc7b 100644 --- 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 ", "license": "ISC", diff --git a/integration-tests/worker/test/exit-reasons.test.ts b/integration-tests/worker/test/exit-reasons.test.ts index 478d74252..64f6b3a9c 100644 --- 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 index 66ef623ec..1684399a0 100644 --- 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 index 19d7918b3..f67d1773a 100644 --- 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 index 02c830cec..871e0fb09 100644 --- 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 index cae5e1a39..79fe147fc 100644 --- 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 = {}; 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 index 92a240f2c..9786c79b2 100644 --- 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 index 93a489850..0bdeae78a 100644 --- 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 index f76d0ac13..fd5b2b434 100644 --- 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 index 360739909..35b70ddf9 100644 --- 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 index 8fda2d68c..10632071d 100644 --- 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 index 5d245808a..9fe35b760 100644 --- 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 index 514dbd6b8..4c89dbd5b 100644 --- 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) => {