Release: worker@1.22.1 cli@1.30.3

packages/cli/CHANGELOG.md

@@ -1,5 +1,13 @@
 # @openfn/cli
 
+## 1.30.3
+
+### Patch Changes
+
+- c687ef5: Improve messaging and position reporting in lazy state compilation errors
+- Updated dependencies [c687ef5]
+  - @openfn/compiler@1.2.3
+
 ## 1.30.2
 
 ### Patch Changes

packages/cli/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@openfn/cli",
-  "version": "1.30.2",
+  "version": "1.30.3",
   "description": "CLI devtools for the OpenFn toolchain",
   "engines": {
     "node": ">=18",

packages/cli/src/util/abort.ts

@@ -7,18 +7,35 @@ 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);
   logger.break();
   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);

packages/compiler/CHANGELOG.md

@@ -1,5 +1,11 @@
 # @openfn/compiler
 
+## 1.2.3
+
+### Patch Changes
+
+- c687ef5: Improve messaging and position reporting in lazy state compilation errors
+
 ## 1.2.2
 
 ### Patch Changes

packages/compiler/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@openfn/compiler",
-  "version": "1.2.2",
+  "version": "1.2.3",
   "description": "Compiler and language tooling for openfn jobs.",
   "author": "Open Function Group <admin@openfn.org>",
   "license": "ISC",

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<n.MemberExpression>) => {
   let root = path;
@@ -23,9 +49,12 @@ const ensureParentArrow = (path: NodePath<n.MemberExpression>) => {
     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<n.MemberExpression>) => {
     }
   } 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:

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,
   });
 });
 

packages/engine-multi/CHANGELOG.md

@@ -1,5 +1,16 @@
 # engine-multi
 
+## 1.10.5
+
+### Patch Changes
+
+- 6fd3942: Emit compilation failure log before workflow-error event. Previously the error
+  event arrived first, causing the worker to tear down the channel before the
+  log could be delivered.
+- 32b43cb: When reporting compilation errors, prefer the step name to the id
+- Updated dependencies [c687ef5]
+  - @openfn/compiler@1.2.3
+
 ## 1.10.4
 
 ### Patch Changes

packages/engine-multi/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@openfn/engine-multi",
-  "version": "1.10.4",
+  "version": "1.10.5",
   "description": "Multi-process runtime engine",
   "main": "dist/index.js",
   "type": "module",

packages/engine-multi/src/api/execute.ts

@@ -131,6 +131,19 @@ const execute = async (context: ExecutionContext) => {
       },
       [workerEvents.ERROR]: (evt: workerEvents.ErrorEvent) => {
         didError = true;
+        if (compileStatus === 'started') {
+          log(context, {
+            type: workerEvents.LOG,
+            workflowId: state.plan.id!,
+            threadId: evt.threadId || '-',
+            log: {
+              level: 'info',
+              message: ['Error occurred during compilation'],
+              name: 'RTE',
+              time: timestamp().toString(),
+            },
+          });
+        }
         error(context, {
           workflowId: state.plan.id,
           error: evt.error,
@@ -155,21 +168,6 @@ const execute = async (context: ExecutionContext) => {
       events,
       workerOptions
     ).catch(async (e: any) => {
-      if (compileStatus === 'started') {
-        // Try and alert users that the error occurred at compile-time
-        // Not super keen on adding this down in the engine but it may help app users
-        await log(context, {
-          type: workerEvents.LOG,
-          workflowId: state.plan.id!,
-          threadId: '-',
-          log: {
-            level: 'info',
-            message: [`Error occurred during compilation`],
-            name: 'RTE',
-            time: timestamp().toString(),
-          },
-        });
-      }
       // An error should:
       // a) emit an error event (and so be handled by the error() function
       // b) reject the task in the pool

packages/engine-multi/src/worker/thread/compile.ts

@@ -25,7 +25,7 @@ export default async (
           job.sourceMap = result.map;
           job.expression = result.code;
         } catch (e) {
-          throw new CompileError(e, job.id!);
+          throw new CompileError(e, job.name ?? job.id!);
         }
       }
     }

packages/engine-multi/src/worker/thread/mock-run.ts

@@ -44,11 +44,13 @@ function mockRun(plan: MockExecutionPlan, input: State, _options = {}) {
   const workflowId = plan.id;
 
   // simulate compilation
+  publish(workerEvents.COMPILE_START, { workflowId });
   try {
     eval(job.expression!);
   } catch (e: any) {
     throw new CompileError(e, job.id!);
   }
+  publish(workerEvents.COMPILE_COMPLETE, { workflowId, duration: 0 });
 
   return new Promise((resolve) => {
     const jobId = job.id || '<job>';

packages/engine-multi/test/api/execute.test.ts

@@ -322,6 +322,38 @@ test.serial('should emit CompileError if compilation fails', async (t) => {
   await execute(context);
 });
 
+test.serial.only(
+  'on compile error, the error log should arrive before the workflow-error event',
+  async (t) => {
+    const state = {
+      id: 'compile-order',
+      plan: {
+        workflow: {
+          steps: [{ id: 'j', expression: 'la la la' }],
+        },
+      },
+    } as WorkflowState;
+
+    const context = createContext({ state, options: {} });
+
+    const orderedEvents: string[] = [];
+
+    context.on(WORKFLOW_LOG, (evt) => {
+      if (/error occurred during compilation/i.test(evt.message)) {
+        orderedEvents.push('log');
+      }
+    });
+
+    context.on(WORKFLOW_ERROR, () => {
+      orderedEvents.push('error');
+    });
+
+    await execute(context);
+
+    t.deepEqual(orderedEvents, ['log', 'error']);
+  }
+);
+
 test.serial('should stringify the whitelist array', async (t) => {
   let passedOptions: any;
 

packages/engine-multi/test/errors.test.ts

@@ -27,6 +27,29 @@ test.before(async () => {
   engine = await createEngine(options);
 });
 
+test.serial('reporting: prefer step name to step id', (t) => {
+  return new Promise((done) => {
+    const plan = {
+      id: 'a',
+      workflow: {
+        steps: [
+          {
+            id: 'x',
+            name: 'My Step',
+            expression: 'a a a',
+          },
+        ],
+      },
+      options: {},
+    };
+
+    engine.execute(plan, {}).on(WORKFLOW_ERROR, (evt) => {
+      t.is(evt.message, 'My Step: Unexpected token (1:2)');
+      done();
+    });
+  });
+});
+
 // This should exit gracefully with a compile error
 test.serial('syntax error: missing bracket', (t) => {
   return new Promise((done) => {

packages/lightning-mock/CHANGELOG.md

@@ -1,5 +1,13 @@
 # @openfn/lightning-mock
 
+## 2.4.7
+
+### Patch Changes
+
+- Updated dependencies [6fd3942]
+- Updated dependencies [32b43cb]
+  - @openfn/engine-multi@1.10.5
+
 ## 2.4.6
 
 ### Patch Changes