diff --git a/.changeset/four-lights-see.md b/.changeset/four-lights-see.md new file mode 100644 index 000000000..1bbc89917 --- /dev/null +++ b/.changeset/four-lights-see.md @@ -0,0 +1,6 @@ +--- +'@openfn/compiler': patch +'@openfn/cli': patch +--- + +Improve messaging and position reporting in lazy state compilation errors diff --git a/packages/cli/src/util/abort.ts b/packages/cli/src/util/abort.ts index 23adaede4..83aa44950 100644 --- a/packages/cli/src/util/abort.ts +++ b/packages/cli/src/util/abort.ts @@ -7,11 +7,16 @@ class AbortError extends Error { handled = true; } +interface CLIFriendlyError extends Error { + fix?: string; + details?: string; +} + // This is always the CLI logger, can I trap it? export default ( logger: Logger, reason: string, - error?: Error, + error?: CLIFriendlyError, help?: string ) => { const e = new AbortError(reason); @@ -19,6 +24,18 @@ export default ( logger.error(reason); if (error) { logger.error(error.message); + logger.break(); + + if (error.details) { + logger.error('ERROR DETAILS:'); + logger.error(error.details); + logger.break(); + } + if (error.fix) { + logger.error('FIX HINT:'); + logger.error(error.fix); + logger.break(); + } } if (help) { logger.always(help); diff --git a/packages/compiler/src/transforms/lazy-state.ts b/packages/compiler/src/transforms/lazy-state.ts index e1e578501..f67a31cda 100644 --- a/packages/compiler/src/transforms/lazy-state.ts +++ b/packages/compiler/src/transforms/lazy-state.ts @@ -11,6 +11,32 @@ import type { NodePath } from 'ast-types/lib/node-path'; import type { Transformer } from '../transform'; import IgnoreRules from '../transform-ignore'; +type LazyStateErrorOptions = { + details?: string; + fix?: string; + pos?: any; +}; + +export class LazyStateError extends Error { + fix?: string; + details?: string; + pos?: any; + + constructor( + message: string, + { details, fix, pos }: LazyStateErrorOptions = {} + ) { + const posStr = (pos && `(${pos.start.line}:${pos.start.column})`) ?? ''; + super(`Lazy State Error: ${message} ${posStr}`); + + this.fix = fix; + this.details = details; + const { start, end } = pos; + this.pos = { start, end }; + delete this.stack; + } +} + // Walk up the AST and work out where the parent arrow function should go const ensureParentArrow = (path: NodePath) => { let root = path; @@ -23,9 +49,12 @@ const ensureParentArrow = (path: NodePath) => { root = root.parent; // if this is any kind of statement, we should throw - // TODO we may relax this, see https://github.com/OpenFn/kit/issues/660 if (n.Statement.check(root.node) || n.Declaration.check(root.node)) { - throw new Error(`invalid state operator: must be inside an expression`); + throw new LazyStateError('Must be inside an operation', { + pos: path.node.loc, + details: + 'The Lazy State operation must be used inside a top-level operation, like fn(). It cannot be used inside a regular JavaScript statement because no valid state reference is available.', + }); } } @@ -47,9 +76,9 @@ const ensureParentArrow = (path: NodePath) => { } } else { // Actually I don't think we'll ever get here - throw new Error( - `invalid state operator: must be be passed as an argument to an operator` - ); + throw new LazyStateError('must be passed as an argument to an operator', { + pos: path.node.loc, + }); } }; @@ -66,11 +95,11 @@ const isOpenFunction = (path: NodePath) => { // We already have a valid open function here return true; } - throw new Error( - `invalid state operator: parameter "${name}" should be called "state"` - ); + throw new LazyStateError(`parameter "${name}" should be called "state"`, { + pos: path.node.loc, + }); } - throw new Error('invalid state operator: parent has wrong arity'); + throw new LazyStateError('parent has wrong arity', { pos: path.node.loc }); } // if we get here, then path is: diff --git a/packages/compiler/test/transforms/lazy-state.test.ts b/packages/compiler/test/transforms/lazy-state.test.ts index 9465de30c..392c9df94 100644 --- a/packages/compiler/test/transforms/lazy-state.test.ts +++ b/packages/compiler/test/transforms/lazy-state.test.ts @@ -5,7 +5,7 @@ import { namedTypes, NodePath, builders as b } from 'ast-types'; import parse from '../../src/parse'; import transform from '../../src/transform'; -import visitors from '../../src/transforms/lazy-state'; +import visitors, { LazyStateError } from '../../src/transforms/lazy-state'; test('convert a simple dollar reference', (t) => { const ast = parse('get($.data)'); @@ -86,7 +86,8 @@ test('throw if a $ is already inside a non-compatible arrow (state name)', (t) = const ast = parse(src); t.throws(() => transform(ast, [visitors]), { - message: `invalid state operator: parameter "s" should be called "state"`, + instanceOf: LazyStateError, + message: /parameter "s" should be called "state" \(1:4\)/i, }); }); @@ -96,7 +97,8 @@ test('throw if a $ is already inside a non-compatible arrow (arity)', (t) => { const ast = parse(src); t.throws(() => transform(ast, [visitors]), { - message: 'invalid state operator: parent has wrong arity', + instanceOf: LazyStateError, + message: /parent has wrong arity \(1:4\)/i, }); }); @@ -106,7 +108,8 @@ test('throw if $ is not inside an operation', (t) => { const ast = parse(src); t.throws(() => transform(ast, [visitors]), { - message: 'invalid state operator: must be inside an expression', + instanceOf: LazyStateError, + message: /must be inside an operation \(1:10\)/i, }); }); @@ -116,7 +119,8 @@ test('throw if $ is on the left hand side of an assignment', (t) => { const ast = parse(src); t.throws(() => transform(ast, [visitors]), { - message: 'invalid state operator: must be inside an expression', + instanceOf: LazyStateError, + message: /must be inside an operation \(1:0\)/i, }); }); @@ -126,7 +130,8 @@ test('throw if $ is on the left hand side of a nested assignment', (t) => { const ast = parse(src); t.throws(() => transform(ast, [visitors]), { - message: 'invalid state operator: must be inside an expression', + instanceOf: LazyStateError, + message: /must be inside an operation \(1:11\)/i, }); }); @@ -136,7 +141,8 @@ test('throw if $ is on the left hand side of a multi assignment', (t) => { const ast = parse(src); t.throws(() => transform(ast, [visitors]), { - message: 'invalid state operator: must be inside an expression', + instanceOf: LazyStateError, + message: /must be inside an operation \(1:10\)/i, }); });