diff --git a/integration-tests/execute/src/execute.ts b/integration-tests/execute/src/execute.ts index f19ba57cb..0e251effe 100644 --- a/integration-tests/execute/src/execute.ts +++ b/integration-tests/execute/src/execute.ts @@ -22,6 +22,7 @@ const execute = async ( }; const compiled = compiler(job, options); const result = await run(compiled.code, state, { + defaultStepId: 'src', sourceMap: compiled.map, // preload the linker with some locally installed modules linker: { diff --git a/integration-tests/execute/test/errors.test.ts b/integration-tests/execute/test/errors.test.ts index 511ae644f..4abfdf994 100644 --- a/integration-tests/execute/test/errors.test.ts +++ b/integration-tests/execute/test/errors.test.ts @@ -23,7 +23,7 @@ test.serial('should throw a reference error', async (t) => { t.is(err.message, 'ReferenceError: x is not defined'); t.is(err.severity, 'crash'); - t.is(err.step, 'job-1'); // this name is auto-generated btw + t.is(err.step, 'src'); // this name is auto-generated btw t.deepEqual(err.pos, { column: 11, line: 1, @@ -40,12 +40,12 @@ test.serial('should throw a type error', async (t) => { const ignore = ['x']; const result = await execute(job, state, 'common', ignore); - const err = result.errors['job-1']; + const err = result.errors['src']; t.log(err.pos); t.is(err.message, 'TypeError: s is not a function'); t.is(err.severity, 'fail'); - t.is(err.step, 'job-1'); + t.is(err.step, 'src'); t.deepEqual(err.pos, { column: 11, line: 1, @@ -65,7 +65,7 @@ test.serial.skip('should throw an adaptor error', async (t) => { get("www")`; const result = await execute(job, state, 'http'); - const err = result.errors['job-1']; + const err = result.errors['src']; t.log(err); t.is(err.message, 'AdaptorError: INVALID_URL'); diff --git a/integration-tests/execute/test/execute.test.ts b/integration-tests/execute/test/execute.test.ts index 10ba76e2c..108cf32bb 100644 --- a/integration-tests/execute/test/execute.test.ts +++ b/integration-tests/execute/test/execute.test.ts @@ -117,8 +117,8 @@ test.serial('catch an error and re-throw it', async (t) => { }).catch(e => { throw e })`; const result = await execute(job, state); - t.is(result.errors['job-1'].name, 'JobError'); - t.is(result.errors['job-1'].message, 'err'); + t.is(result.errors.src.name, 'JobError'); + t.is(result.errors.src.message, 'err'); }); test.serial('catch an error and return state', async (t) => { diff --git a/packages/cli/CHANGELOG.md b/packages/cli/CHANGELOG.md index 4ceb95d83..24118df44 100644 --- a/packages/cli/CHANGELOG.md +++ b/packages/cli/CHANGELOG.md @@ -1,5 +1,18 @@ # @openfn/cli +## 1.20.3 + +### Patch Changes + +- a030ebd: Fix an issue where error positions are not properly reported for named steps in a workflow +- Updated dependencies [a1bfdc1] +- Updated dependencies [c70369b] + - @openfn/runtime@1.7.7 + - @openfn/logger@1.1.1 + - @openfn/compiler@1.2.2 + - @openfn/deploy@0.11.5 + - @openfn/project@0.9.3 + ## 1.20.2 ### Patch Changes diff --git a/packages/cli/package.json b/packages/cli/package.json index 5919b8a95..5ead44d0d 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -1,6 +1,6 @@ { "name": "@openfn/cli", - "version": "1.20.2", + "version": "1.20.3", "description": "CLI devtools for the OpenFn toolchain", "engines": { "node": ">=18", diff --git a/packages/cli/src/compile/compile.ts b/packages/cli/src/compile/compile.ts index a8fecdb0d..ea5071ffb 100644 --- a/packages/cli/src/compile/compile.ts +++ b/packages/cli/src/compile/compile.ts @@ -57,6 +57,9 @@ const compileJob = async ( ): Promise => { try { const compilerOptions: Options = await loadTransformOptions(opts, log); + if (jobName) { + compilerOptions.name = jobName; + } return compile(job, compilerOptions); } catch (e: any) { abort( @@ -91,7 +94,7 @@ const compileWorkflow = async ( job.expression as string, jobOpts, log, - job.id + job.name ?? job.id ); job.expression = code; job.sourceMap = map; diff --git a/packages/cli/test/execute/execute.test.ts b/packages/cli/test/execute/execute.test.ts index 66acbe26b..a1e801661 100644 --- a/packages/cli/test/execute/execute.test.ts +++ b/packages/cli/test/execute/execute.test.ts @@ -8,7 +8,6 @@ import { ExecuteOptions } from '../../src/execute/command'; import handler from '../../src/execute/handler'; import { mockFs, resetMockFs } from '../util'; -// Why is this logging everywhere? const logger = createMockLogger(undefined, { level: 'none' }); // These options are designed to minimise i/o @@ -20,12 +19,8 @@ const defaultOptions = { repoDir: '/repo', path: '.', log: { - // TODO if I pass a mock logger into the handler, the handler then - // goes and creates a real logger and passes it to the runtinme, which - // causes all sorts of logging - // Why isn't that affecting other tests? - // Why do I need this special handling here? - default: 'none', + // This special flag even hides errors + default: 'none-really', } as any, // TODO some weird typing here logJson: true, } as Partial; @@ -129,6 +124,104 @@ test.serial('run a workflow with state', async (t) => { t.is(result.data.count, 4); }); +test.serial( + 'run a workflow with errors and positions with anonymous steps', + async (t) => { + const workflow = { + workflow: { + steps: [ + { + expression: `${fn}fn((state) => state.x.length)`, + }, + ], + }, + }; + + mockFs({ + '/workflow.json': JSON.stringify(workflow), + }); + + const options = { + ...defaultOptions, + workflowPath: '/workflow.json', + }; + const result = await handler(options, logger); + + t.truthy(result.errors); + const err = result.errors['job-1']; + t.regex(err.message, /typeerror: cannot read properties of undefined/i); + t.is(err.pos.line, 2); + t.is(err.pos.column, 23); + } +); + +test.serial( + "run a workflow with errors and positions with id'd steps", + async (t) => { + const workflow = { + workflow: { + steps: [ + { + id: 'x', + expression: `${fn}fn((state) => state.x.length)`, + }, + ], + }, + }; + + mockFs({ + '/workflow.json': JSON.stringify(workflow), + }); + + const options = { + ...defaultOptions, + workflowPath: '/workflow.json', + }; + const result = await handler(options, logger); + t.truthy(result.errors); + t.regex( + result.errors.x.message, + /typeerror: cannot read properties of undefined/i + ); + t.is(result.errors.x.pos.line, 2); + t.is(result.errors.x.pos.column, 23); + } +); + +test.serial( + 'run a workflow with errors and positions and named steps', + async (t) => { + const workflow = { + workflow: { + steps: [ + { + id: 'x', + name: 'My Step', + expression: `${fn}fn((state) => state.x.length)`, + }, + ], + }, + }; + + mockFs({ + '/workflow.json': JSON.stringify(workflow), + }); + + const options = { + ...defaultOptions, + workflowPath: '/workflow.json', + }; + const result = await handler(options, logger); + t.truthy(result.errors); + t.regex( + result.errors.x.message, + /typeerror: cannot read properties of undefined/i + ); + t.is(result.errors.x.pos.line, 2); + t.is(result.errors.x.pos.column, 23); + } +); + test.serial('run a workflow with cached steps', async (t) => { const workflow = { workflow: { diff --git a/packages/compiler/CHANGELOG.md b/packages/compiler/CHANGELOG.md index c0ea273d3..c86edeaa6 100644 --- a/packages/compiler/CHANGELOG.md +++ b/packages/compiler/CHANGELOG.md @@ -1,5 +1,12 @@ # @openfn/compiler +## 1.2.2 + +### Patch Changes + +- Updated dependencies [c70369b] + - @openfn/logger@1.1.1 + ## 1.2.1 ### Patch Changes diff --git a/packages/compiler/package.json b/packages/compiler/package.json index 57a3af868..33008222b 100644 --- a/packages/compiler/package.json +++ b/packages/compiler/package.json @@ -1,6 +1,6 @@ { "name": "@openfn/compiler", - "version": "1.2.1", + "version": "1.2.2", "description": "Compiler and language tooling for openfn jobs.", "author": "Open Function Group ", "license": "ISC", diff --git a/packages/deploy/CHANGELOG.md b/packages/deploy/CHANGELOG.md index ab6f4f884..555dc2f2e 100644 --- a/packages/deploy/CHANGELOG.md +++ b/packages/deploy/CHANGELOG.md @@ -1,5 +1,12 @@ # @openfn/deploy +## 0.11.5 + +### Patch Changes + +- Updated dependencies [c70369b] + - @openfn/logger@1.1.1 + ## 0.11.4 ### Patch Changes diff --git a/packages/deploy/package.json b/packages/deploy/package.json index cabeb6261..6c26b10a8 100644 --- a/packages/deploy/package.json +++ b/packages/deploy/package.json @@ -1,6 +1,6 @@ { "name": "@openfn/deploy", - "version": "0.11.4", + "version": "0.11.5", "description": "Deploy projects to Lightning instances", "type": "module", "exports": { diff --git a/packages/engine-multi/CHANGELOG.md b/packages/engine-multi/CHANGELOG.md index f698d7712..8b5785c28 100644 --- a/packages/engine-multi/CHANGELOG.md +++ b/packages/engine-multi/CHANGELOG.md @@ -1,5 +1,15 @@ # engine-multi +## 1.9.1 + +### Patch Changes + +- Updated dependencies [a1bfdc1] +- Updated dependencies [c70369b] + - @openfn/runtime@1.7.7 + - @openfn/logger@1.1.1 + - @openfn/compiler@1.2.2 + ## 1.9.0 ### Minor Changes diff --git a/packages/engine-multi/package.json b/packages/engine-multi/package.json index e117e73f1..9b6af17f1 100644 --- a/packages/engine-multi/package.json +++ b/packages/engine-multi/package.json @@ -1,6 +1,6 @@ { "name": "@openfn/engine-multi", - "version": "1.9.0", + "version": "1.9.1", "description": "Multi-process runtime engine", "main": "dist/index.js", "type": "module", diff --git a/packages/lightning-mock/CHANGELOG.md b/packages/lightning-mock/CHANGELOG.md index 925c10f31..371833a90 100644 --- a/packages/lightning-mock/CHANGELOG.md +++ b/packages/lightning-mock/CHANGELOG.md @@ -1,5 +1,15 @@ # @openfn/lightning-mock +## 2.3.10 + +### Patch Changes + +- Updated dependencies [a1bfdc1] +- Updated dependencies [c70369b] + - @openfn/runtime@1.7.7 + - @openfn/logger@1.1.1 + - @openfn/engine-multi@1.9.1 + ## 2.3.9 ### Patch Changes diff --git a/packages/lightning-mock/package.json b/packages/lightning-mock/package.json index 19478fb14..997fee8f8 100644 --- a/packages/lightning-mock/package.json +++ b/packages/lightning-mock/package.json @@ -1,6 +1,6 @@ { "name": "@openfn/lightning-mock", - "version": "2.3.9", + "version": "2.3.10", "private": true, "description": "A mock Lightning server", "main": "dist/index.js", diff --git a/packages/logger/CHANGELOG.md b/packages/logger/CHANGELOG.md index 790077b79..f12d0d832 100644 --- a/packages/logger/CHANGELOG.md +++ b/packages/logger/CHANGELOG.md @@ -1,5 +1,11 @@ # @openfn/logger +## 1.1.1 + +### Patch Changes + +- c70369b: Add secret "none-really" log level that even suppresses error logs + ## 1.1.0 ### Minor Changes diff --git a/packages/logger/package.json b/packages/logger/package.json index 7c83626b3..92cd7ed40 100644 --- a/packages/logger/package.json +++ b/packages/logger/package.json @@ -1,6 +1,6 @@ { "name": "@openfn/logger", - "version": "1.1.0", + "version": "1.1.1", "description": "Cross-package logging utility", "module": "dist/index.js", "author": "Open Function Group ", diff --git a/packages/logger/src/logger.ts b/packages/logger/src/logger.ts index ce8fbd06d..ab064efa1 100644 --- a/packages/logger/src/logger.ts +++ b/packages/logger/src/logger.ts @@ -12,6 +12,9 @@ import ensureOptions, { LogOptions, LogLevel } from './options'; // (Note that this doesn't stop a component printing to stdout! It just disables the logger) export const NONE = 'none'; +// Special debug log level - this even hides errors +export const REALLY_NONE = 'none-really'; + // Defaults for all the family. Prints what the user absolutely has to know. // Top-level completions, errors and warnings export const SUCCESS = 'success'; // aka default @@ -113,6 +116,8 @@ const priority: Record = { [SUCCESS]: 2, [NONE]: 9, [ERROR]: 100, // errors ALWAYS log + + [REALLY_NONE]: 1000, }; // // TODO I'd quite like each package to have its own colour, I think diff --git a/packages/logger/src/options.ts b/packages/logger/src/options.ts index 6cfa7f7a7..9afe3fdca 100644 --- a/packages/logger/src/options.ts +++ b/packages/logger/src/options.ts @@ -2,7 +2,7 @@ import { SanitizePolicies } from './sanitize'; import defaultEmitter from './util/default-emitter'; import jsonEmitter from './util/json-emitter'; -export type LogLevel = 'debug' | 'info' | 'default' | 'none'; +export type LogLevel = 'debug' | 'info' | 'default' | 'none' | 'none-really'; export type LogEmitter = typeof console & { success: typeof console.log; diff --git a/packages/project/CHANGELOG.md b/packages/project/CHANGELOG.md index 010b62790..44d014c79 100644 --- a/packages/project/CHANGELOG.md +++ b/packages/project/CHANGELOG.md @@ -1,5 +1,12 @@ # @openfn/project +## 0.9.3 + +### Patch Changes + +- Updated dependencies [c70369b] + - @openfn/logger@1.1.1 + ## 0.9.2 ### Patch Changes diff --git a/packages/project/package.json b/packages/project/package.json index 33b7120ab..035a64ae0 100644 --- a/packages/project/package.json +++ b/packages/project/package.json @@ -1,6 +1,6 @@ { "name": "@openfn/project", - "version": "0.9.2", + "version": "0.9.3", "description": "Read, serialize, replicate and sync OpenFn projects", "scripts": { "test": "pnpm ava", diff --git a/packages/runtime/CHANGELOG.md b/packages/runtime/CHANGELOG.md index 154dc0d8a..f38f7500d 100644 --- a/packages/runtime/CHANGELOG.md +++ b/packages/runtime/CHANGELOG.md @@ -1,5 +1,13 @@ # @openfn/runtime +## 1.7.7 + +### Patch Changes + +- a1bfdc1: Fix an issue where an error can be thrown while trying to source map errors +- Updated dependencies [c70369b] + - @openfn/logger@1.1.1 + ## 1.7.6 ### Patch Changes diff --git a/packages/runtime/package.json b/packages/runtime/package.json index fe796e244..3bcad74bc 100644 --- a/packages/runtime/package.json +++ b/packages/runtime/package.json @@ -1,6 +1,6 @@ { "name": "@openfn/runtime", - "version": "1.7.6", + "version": "1.7.7", "description": "Job processing runtime.", "type": "module", "exports": { diff --git a/packages/runtime/src/runtime.ts b/packages/runtime/src/runtime.ts index fd6e87b8e..11b60f6c0 100644 --- a/packages/runtime/src/runtime.ts +++ b/packages/runtime/src/runtime.ts @@ -35,6 +35,9 @@ export type Options = { /** Number of ms to wait before polling memory */ profilePollInterval?: number; + + /** Optional name for the expression (if passed a string) */ + defaultStepId?: string; }; type RawOptions = Omit & { @@ -49,7 +52,8 @@ const defaultLogger = createMockLogger(); const loadPlanFromString = ( expression: string, logger: Logger, - sourceMap?: SourceMapWithOperations + sourceMap?: SourceMapWithOperations, + id?: string ) => { const plan: ExecutionPlan = { workflow: { @@ -58,6 +62,7 @@ const loadPlanFromString = ( { expression, sourceMap, + id, }, ], }, @@ -78,7 +83,12 @@ const run = ( const logger = opts.logger || defaultLogger; if (typeof xplan === 'string') { - xplan = loadPlanFromString(xplan, logger, opts.sourceMap); + xplan = loadPlanFromString( + xplan, + logger, + opts.sourceMap, + opts.defaultStepId + ); } if (!xplan.options) { diff --git a/packages/runtime/src/util/sourcemap-errors.ts b/packages/runtime/src/util/sourcemap-errors.ts index 7c91ade65..511dc1c8f 100644 --- a/packages/runtime/src/util/sourcemap-errors.ts +++ b/packages/runtime/src/util/sourcemap-errors.ts @@ -6,30 +6,40 @@ import type { Job } from '@openfn/lexicon'; // This function takes an error and a job and updates the error with sourcemapped metadata export default async (job: Job, error: RTError) => { if (job.sourceMap) { - const smc = await new SourceMapConsumer(job.sourceMap); + try { + const smc = await new SourceMapConsumer(job.sourceMap); - if (error.pos && error.pos.line && error.pos.column) { - error.pos = pick( - smc.originalPositionFor({ - line: error.pos.line, - column: error.pos.column, - }) as any, - 'line', - 'column' - ); + if (error.pos && error.pos.line && error.pos.column) { + error.pos = pick( + smc.originalPositionFor({ + line: error.pos.line, + column: error.pos.column, + }) as any, + 'line', + 'column' + ); - error.step = job.name || job.id; - } + error.step = job.name || job.id; + } - if (error.stack) { - error.stack = await mapStackTrace(smc, error.stack); - } + if (error.stack) { + error.stack = await mapStackTrace(smc, error.stack); + } - if (error.pos && !isNaN(error.pos.line!)) { - const fileName = job.name ? `${job.name}.js` : 'src.js'; - const src = smc.sourceContentFor(fileName)!.split('\n'); - const line = src[error.pos.line! - 1]; - error.pos.src = line; + if (error.pos && !isNaN(error.pos.line!)) { + const fileName = `${job.name || job.id || 'src'}.js`; + const src = smc.sourceContentFor(fileName)!.split('\n'); + const line = src[error.pos.line! - 1]; + error.pos.src = line; + } + } catch (e) { + // error while trying to map + // (we must not throw here or we'll create more problems) + console.warn( + 'Error occurred trying to resolve sourcemap for ', + job.name || job.id + ); + console.warn(e); } } }; diff --git a/packages/runtime/test/errors.test.ts b/packages/runtime/test/errors.test.ts index 2153e2607..6c915f4ce 100644 --- a/packages/runtime/test/errors.test.ts +++ b/packages/runtime/test/errors.test.ts @@ -176,7 +176,7 @@ fn((s) => z)`; t.log(code); let error: any; try { - await run(code, {}, { sourceMap: map }); + await run(code, {}, { defaultStepId: 'src', sourceMap: map }); } catch (e) { error = e; } @@ -443,6 +443,7 @@ test('fail on nested adaptor error and map to a position in the vm', async (t) = code, {}, { + defaultStepId: 'src', linker: { modules: { x: { path: path.resolve('test/__modules__/@openfn/language-test') }, @@ -452,7 +453,7 @@ test('fail on nested adaptor error and map to a position in the vm', async (t) = } ); - const error = result.errors['job-1']; + const error = result.errors.src; t.log(error); t.deepEqual(error, { @@ -464,7 +465,7 @@ test('fail on nested adaptor error and map to a position in the vm', async (t) = name: 'AdaptorError', source: 'runtime', severity: 'fail', - step: 'job-1', + step: 'src', pos: { column: 20, line: 4, @@ -560,3 +561,71 @@ test('AdaptorError: ensure that error objects are safely serialised', (t) => { name: 'AxiosError', }); }); + +test('maps positions for an error on a workflow', async (t) => { + // compile the code so we get a source map + const { code, map } = compile(` + function fn(f) { return f() }; + fn((x) => x.y); + `); + + // Build a workflow with sourcemap included + const wf = { + workflow: { + steps: [ + { + id: 'x', + expression: code, + sourceMap: map, + }, + ], + }, + }; + + const result = await run(wf); + + const error: any = result.errors.x; + t.is(error.severity, 'fail'); + t.is( + error.message, + "TypeError: Cannot read properties of undefined (reading 'y')" + ); + t.is(error.pos.line, 3); + t.is(error.pos.column, 17); +}); + +test('maps positions for an error on a workflow with a step name', async (t) => { + // compile the code so we get a source map + const { code, map } = compile( + ` + function fn(f) { return f() }; + fn((x) => x.y); + `, + { name: 'x' } // fails if this is not set + ); + + // Build a workflow with sourcemap included + const wf = { + workflow: { + steps: [ + { + id: 'x', + name: 'x', + expression: code, + sourceMap: map, + }, + ], + }, + }; + + const result = await run(wf); + + const error: any = result.errors.x; + t.is(error.severity, 'fail'); + t.is( + error.message, + "TypeError: Cannot read properties of undefined (reading 'y')" + ); + t.is(error.pos.line, 3); + t.is(error.pos.column, 17); +}); diff --git a/packages/runtime/test/execute/plan.test.ts b/packages/runtime/test/execute/plan.test.ts index 7f658562d..4666e29c0 100644 --- a/packages/runtime/test/execute/plan.test.ts +++ b/packages/runtime/test/execute/plan.test.ts @@ -914,7 +914,7 @@ test('log appropriately on error', async (t) => { { id: 'job1', state: {}, - expression: 'export default [s => { throw Error("e")}]', + expression: 'export default [s => { throw Error("eee")}]', }, ]); @@ -929,7 +929,7 @@ test('log appropriately on error', async (t) => { t.truthy(logger._find('error', /Check state.errors.job1 for details/i)); const [_level, _icon, errMessage]: any = logger._history.at(-2); - t.deepEqual(errMessage, 'e'); + t.regex(errMessage, /eee/); }); test('steps do not share a local scope', async (t) => { diff --git a/packages/runtime/test/repo/ensure-repo.test.ts b/packages/runtime/test/repo/ensure-repo.test.ts index 77c0d37ec..49e7247b6 100644 --- a/packages/runtime/test/repo/ensure-repo.test.ts +++ b/packages/runtime/test/repo/ensure-repo.test.ts @@ -67,5 +67,5 @@ test.serial('log if a repo is created', async (t) => { mock({}); await ensureRepo('/tmp/repo', logger); const { message } = logger._parse(logger._last); - t.assert((message as string).startsWith('Creating new repo')); + t.regex(message as string, /Creating new repo/); }); diff --git a/packages/ws-worker/CHANGELOG.md b/packages/ws-worker/CHANGELOG.md index 588f129ce..9cf2511b3 100644 --- a/packages/ws-worker/CHANGELOG.md +++ b/packages/ws-worker/CHANGELOG.md @@ -1,5 +1,15 @@ # ws-worker +## 1.20.1 + +### Patch Changes + +- Updated dependencies [a1bfdc1] +- Updated dependencies [c70369b] + - @openfn/runtime@1.7.7 + - @openfn/logger@1.1.1 + - @openfn/engine-multi@1.9.1 + ## 1.20.0 ### Minor Changes diff --git a/packages/ws-worker/package.json b/packages/ws-worker/package.json index e1b9e0ffe..cd675039f 100644 --- a/packages/ws-worker/package.json +++ b/packages/ws-worker/package.json @@ -1,6 +1,6 @@ { "name": "@openfn/ws-worker", - "version": "1.20.0", + "version": "1.20.1", "description": "A Websocket Worker to connect Lightning to a Runtime Engine", "main": "dist/index.js", "type": "module",