From 39fb47a5009db184720c537de9be986e1de68d02 Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Tue, 17 Mar 2026 14:05:33 +0000 Subject: [PATCH 1/5] compiler: improve error messages for lazy state --- .changeset/four-lights-see.md | 6 ++++++ packages/cli/src/util/abort.ts | 19 ++++++++++++++++- .../compiler/src/transforms/lazy-state.ts | 21 +++++++++++++++++-- .../test/transforms/lazy-state.test.ts | 14 ++++++++----- 4 files changed, 52 insertions(+), 8 deletions(-) create mode 100644 .changeset/four-lights-see.md diff --git a/.changeset/four-lights-see.md b/.changeset/four-lights-see.md new file mode 100644 index 000000000..660909aa7 --- /dev/null +++ b/.changeset/four-lights-see.md @@ -0,0 +1,6 @@ +--- +'@openfn/compiler': patch +'@openfn/cli': patch +--- + +Improve messaging 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..7426b91f2 100644 --- a/packages/compiler/src/transforms/lazy-state.ts +++ b/packages/compiler/src/transforms/lazy-state.ts @@ -11,6 +11,21 @@ import type { NodePath } from 'ast-types/lib/node-path'; import type { Transformer } from '../transform'; import IgnoreRules from '../transform-ignore'; +export class LazyStateError extends Error { + fix?: string; + details?: string; + + constructor( + message: string, + { details, fix }: { details?: string; fix?: string } = {} + ) { + super(`Lazy State Error: ${message}`); + + this.fix = fix; + this.details = details; + } +} + // Walk up the AST and work out where the parent arrow function should go const ensureParentArrow = (path: NodePath) => { let root = path; @@ -23,9 +38,11 @@ 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', { + 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.', + }); } } diff --git a/packages/compiler/test/transforms/lazy-state.test.ts b/packages/compiler/test/transforms/lazy-state.test.ts index 9465de30c..0ca4e8ba0 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)'); @@ -106,7 +106,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/i, }); }); @@ -116,7 +117,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/i, }); }); @@ -126,7 +128,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/i, }); }); @@ -136,7 +139,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/i, }); }); From b32e72dfa8eb6b34ee43d5aaf94026f71686d308 Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Tue, 17 Mar 2026 14:19:16 +0000 Subject: [PATCH 2/5] compiler: report position on lazy state reporting --- packages/compiler/src/transforms/lazy-state.ts | 16 ++++++++++++++-- .../compiler/test/transforms/lazy-state.test.ts | 8 ++++---- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/packages/compiler/src/transforms/lazy-state.ts b/packages/compiler/src/transforms/lazy-state.ts index 7426b91f2..b37287ca4 100644 --- a/packages/compiler/src/transforms/lazy-state.ts +++ b/packages/compiler/src/transforms/lazy-state.ts @@ -11,18 +11,29 @@ 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 }: { details?: string; fix?: string } = {} + { details, fix, pos }: LazyStateErrorOptions = {} ) { - super(`Lazy State Error: ${message}`); + const posStr = (pos && `(${pos.start.line}:${pos.start.column})`) ?? ''; + super(`Lazy State Error: ${message} ${posStr}`); this.fix = fix; this.details = details; + const { start, end, ..._rest } = pos; + this.pos = { start, end }; + delete this.stack; } } @@ -40,6 +51,7 @@ const ensureParentArrow = (path: NodePath) => { // if this is any kind of statement, we should throw if (n.Statement.check(root.node) || n.Declaration.check(root.node)) { 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.', }); diff --git a/packages/compiler/test/transforms/lazy-state.test.ts b/packages/compiler/test/transforms/lazy-state.test.ts index 0ca4e8ba0..3e63045eb 100644 --- a/packages/compiler/test/transforms/lazy-state.test.ts +++ b/packages/compiler/test/transforms/lazy-state.test.ts @@ -107,7 +107,7 @@ test('throw if $ is not inside an operation', (t) => { t.throws(() => transform(ast, [visitors]), { instanceOf: LazyStateError, - message: /must be inside an operation/i, + message: /must be inside an operation \(1:10\)/i, }); }); @@ -118,7 +118,7 @@ test('throw if $ is on the left hand side of an assignment', (t) => { t.throws(() => transform(ast, [visitors]), { instanceOf: LazyStateError, - message: /must be inside an operation/i, + message: /must be inside an operation \(1:0\)/i, }); }); @@ -129,7 +129,7 @@ test('throw if $ is on the left hand side of a nested assignment', (t) => { t.throws(() => transform(ast, [visitors]), { instanceOf: LazyStateError, - message: /must be inside an operation/i, + message: /must be inside an operation \(1:11\)/i, }); }); @@ -140,7 +140,7 @@ test('throw if $ is on the left hand side of a multi assignment', (t) => { t.throws(() => transform(ast, [visitors]), { instanceOf: LazyStateError, - message: /must be inside an operation/i, + message: /must be inside an operation \(1:10\)/i, }); }); From 4b77b280b4e064e6924898040450c8ed80ee2915 Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Tue, 17 Mar 2026 14:28:14 +0000 Subject: [PATCH 3/5] update extra errors --- packages/compiler/src/transforms/lazy-state.ts | 14 +++++++------- .../compiler/test/transforms/lazy-state.test.ts | 6 ++++-- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/packages/compiler/src/transforms/lazy-state.ts b/packages/compiler/src/transforms/lazy-state.ts index b37287ca4..553b498dc 100644 --- a/packages/compiler/src/transforms/lazy-state.ts +++ b/packages/compiler/src/transforms/lazy-state.ts @@ -76,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, + }); } }; @@ -95,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 3e63045eb..392c9df94 100644 --- a/packages/compiler/test/transforms/lazy-state.test.ts +++ b/packages/compiler/test/transforms/lazy-state.test.ts @@ -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, }); }); From ed7fe87bca51d147d5662e83a17b7c6a17d7438b Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Tue, 17 Mar 2026 14:28:39 +0000 Subject: [PATCH 4/5] update changeset --- .changeset/four-lights-see.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/four-lights-see.md b/.changeset/four-lights-see.md index 660909aa7..1bbc89917 100644 --- a/.changeset/four-lights-see.md +++ b/.changeset/four-lights-see.md @@ -3,4 +3,4 @@ '@openfn/cli': patch --- -Improve messaging in lazy state compilation errors +Improve messaging and position reporting in lazy state compilation errors From d92b5eed58f7559cf951c6eda14636f74fe9f968 Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Tue, 17 Mar 2026 15:30:09 +0000 Subject: [PATCH 5/5] types --- packages/compiler/src/transforms/lazy-state.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/compiler/src/transforms/lazy-state.ts b/packages/compiler/src/transforms/lazy-state.ts index 553b498dc..f67a31cda 100644 --- a/packages/compiler/src/transforms/lazy-state.ts +++ b/packages/compiler/src/transforms/lazy-state.ts @@ -31,7 +31,7 @@ export class LazyStateError extends Error { this.fix = fix; this.details = details; - const { start, end, ..._rest } = pos; + const { start, end } = pos; this.pos = { start, end }; delete this.stack; }