Map error positions against the original source (sourcemapping) (#848)

* compiler: generate a source map if a job name is passed

* runtime: add a position mapping function

* runtime: map error position

* runtime: update test

* comment

* runtime: properly map positions and stack traces in errors

* compiler: more tests

* cli: refactor to build sourcemaps up properly

* lexicon: updated typings

* runtime: nicely log errors with position and line of code

* runtime: tidy up

* runtime: rewrite sourcemap tests and improve typings

* runtime: fix tests

* runtime: ensure a sourcemap is set when a workflow is generated from a string expression

* tests: add tests for error types

Not sure how useful this is tbh

* runtime: typing

* engine: adjust to new compiler API

* changesets

* runtime: update test

* format

* runtime: refine error output

* tests: added error logging tests

* Sourcemapping adaptor errors (#851)

* runtime: refine error output

* tests: added error logging tests

* compiler: append positional information to top level operations

* compiler: write the operations map to the souce map

* lexicon: add typings for extended source map

* lexicon: tweak sourcemap types

* package lock

* runtime: updat error handling to handle adaptor errors with source mapping

* runtime: better handling of nested adaptor errors

probably

* runtime: update tests

* cli: types

* tidy

* runtime: better handling of nested errors

* Runtime: attempt to clean up error output (#852)

* compiler: append positional information to top level operations

* compiler: write the operations map to the souce map

* lexicon: add typings for extended source map

* lexicon: tweak sourcemap types

* package lock

* runtime: updat error handling to handle adaptor errors with source mapping

* runtime: better handling of nested adaptor errors

probably

* runtime: update tests

* cli: types

* tidy

* runtime: better handling of nested errors

* runtime: improvements to reporting of errors

* changeset

* runtime: improve error details

* runtime: better frame detection for adaptor errors

* runtime: fix tests

* tests: update output logs

* logger: ensure timestamp is added to print logs, so that the worker can handle them properly

* version: [email protected] [email protected]

* tmp: worker to rc1 version

* fix openfnx build

Make sure dist is properly cleaned each time

* runtime: simplify adaptorerror constructor

* tests: fix adaptor versions

* cli: skip flay test

* tests: skip more flaky docs tests

* worker: version to 1.9.0

* cli: update changelog

.gitignore

@@ -8,6 +8,7 @@ public/
 !public/index.html
 tsconfig.tsbuildinfo
 ts.cache
+dist
 
 # Build tools
 .rollup.cache

integration-tests/cli/test/errors.test.ts

@@ -2,18 +2,24 @@ import test from 'ava';
 import path from 'node:path';
 import run from '../src/run';
 import { extractLogs, assertLog } from '../src/util';
-import { stderr } from 'node:process';
 
 const jobsPath = path.resolve('test/fixtures');
 
+const extractErrorLogs = (stdout) => {
+  const stdlogs = extractLogs(stdout);
+  return stdlogs
+    .filter((e) => e.level === 'error')
+    .map((e) => e.message.join(' ').replace(/\(\d+ms\)/, '(SSSms)'))
+    .filter((e) => e.length);
+};
+
 // These are all errors that will stop the CLI from even running
 
 test.serial('expression not found', async (t) => {
   const { stdout, err } = await run('openfn blah.js --log-json');
   t.is(err.code, 1);
 
   const stdlogs = extractLogs(stdout);
-
   assertLog(t, stdlogs, /expression not found/i);
   assertLog(t, stdlogs, /failed to load the expression from blah.js/i);
   assertLog(t, stdlogs, /critical error: aborting command/i);
@@ -133,3 +139,65 @@ test.serial('invalid end (ambiguous)', async (t) => {
   assertLog(t, stdlogs, /Error: end pattern matched multiple steps/i);
   assertLog(t, stdlogs, /aborting/i);
 });
+
+// These test error outputs within valid workflows
+
+test.serial('job with reference error', async (t) => {
+  const { stdout, err } = await run(
+    `openfn ${jobsPath}/errors.json --log-json --start ref --no-cache-steps`
+  );
+
+  const logs = extractErrorLogs(stdout);
+  t.log(logs);
+
+  t.deepEqual(logs, [
+    'ref aborted with error (SSSms)',
+    `TypeError: Cannot read properties of undefined (reading 'y')
+    at vm:module(0):1:23
+    @openfn/language-common_2.1.1/dist/index.cjs:333:12`,
+    'Error occurred at: ref',
+    '1: fn((state) => state.x.y)',
+    '                         ^ ',
+    'Check state.errors.ref for details',
+  ]);
+});
+
+test.serial('job with not a function error', async (t) => {
+  const { stdout, err } = await run(
+    `openfn ${jobsPath}/errors.json --log-json --start not-function --no-cache-steps`
+  );
+
+  const logs = extractErrorLogs(stdout);
+  t.log(logs);
+
+  t.deepEqual(logs, [
+    'not-function aborted with error (SSSms)',
+    `TypeError: state is not a function
+    at vm:module(0):1:15
+    @openfn/language-common_2.1.1/dist/index.cjs:333:12`,
+    'Error occurred at: not-function',
+    '1: fn((state) => state())',
+    '                 ^       ',
+    'Check state.errors.not-function for details',
+  ]);
+});
+
+test.serial('job with assign-to-const error', async (t) => {
+  const { stdout, err } = await run(
+    `openfn ${jobsPath}/errors.json --log-json --start assign-const --no-cache-steps`
+  );
+
+  const logs = extractErrorLogs(stdout);
+  t.log(logs);
+
+  t.deepEqual(logs, [
+    'assign-const aborted with error (SSSms)',
+    `TypeError: Assignment to constant variable.
+    at vm:module(0):1:33
+    @openfn/language-common_2.1.1/dist/index.cjs:333:12`,
+    'Error occurred at: assign-const',
+    '1: fn((state) => {  const x = 10; x = 20; })',
+    '                                   ^        ',
+    'Check state.errors.assign-const for details',
+  ]);
+});

integration-tests/cli/test/fixtures/errors.json

@@ -0,0 +1,21 @@
+{
+  "workflow": {
+    "steps": [
+      {
+        "id": "ref",
+        "adaptor": "[email protected]",
+        "expression": "fn((state) => state.x.y)"
+      },
+      {
+        "id": "not-function",
+        "adaptor": "[email protected]",
+        "expression": "fn((state) => state())"
+      },
+      {
+        "id": "assign-const",
+        "adaptor": "[email protected]",
+        "expression": "fn((state) => {  const x = 10; x = 20; })"
+      }
+    ]
+  }
+}

integration-tests/execute/CHANGELOG.md

@@ -1,5 +1,16 @@
 # @openfn/integration-tests-execute
 
+## 1.0.13
+
+### Patch Changes
+
+- Updated dependencies [4ddd4d6]
+- Updated dependencies [93e9844]
+- Updated dependencies [aaa7e7b]
+- Updated dependencies [4ddd4d6]
+  - @openfn/runtime@1.6.0
+  - @openfn/compiler@1.0.0
+
 ## 1.0.12
 
 ### Patch Changes

integration-tests/execute/package.json

@@ -1,7 +1,7 @@
 {
   "name": "@openfn/integration-tests-execute",
   "private": true,
-  "version": "1.0.12",
+  "version": "1.0.13",
   "description": "Job execution tests",
   "author": "Open Function Group <[email protected]>",
   "license": "ISC",

integration-tests/execute/src/execute.ts

@@ -2,10 +2,16 @@ import path from 'node:path';
 import run from '@openfn/runtime';
 import compiler from '@openfn/compiler';
 
-const execute = async (job: string, state: any, adaptor = 'common') => {
+const execute = async (
+  job: string,
+  state: any,
+  adaptor = 'common',
+  ignore = []
+) => {
   // compile with common and dumb imports
   const options = {
     'add-imports': {
+      ignore,
       adaptors: [
         {
           name: `@openfn/language-${adaptor}`,
@@ -15,9 +21,8 @@ const execute = async (job: string, state: any, adaptor = 'common') => {
     },
   };
   const compiled = compiler(job, options);
-  // console.log(compiled);
-
-  const result = await run(compiled, state, {
+  const result = await run(compiled.code, state, {
+    sourceMap: compiled.map,
     // preload the linker with some locally installed modules
     linker: {
       modules: {

integration-tests/execute/test/errors.test.ts

@@ -0,0 +1,72 @@
+import test from 'ava';
+import execute from '../src/execute';
+
+// This tests the raw errors that come out of runtime
+// (or are written to state)
+// How those errors are serialized, displayed and emitted
+// is a problem for the runtime managers (which can have their own tests)
+
+test.serial('should throw a reference error', async (t) => {
+  const state = { data: { x: 1 } };
+
+  const job = `fn((s) => x)`;
+
+  //  Tell the compiler not to import x from the adaptor
+  const ignore = ['x'];
+
+  let err;
+  try {
+    await execute(job, state, 'common', ignore);
+  } catch (e: any) {
+    err = e;
+  }
+
+  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.deepEqual(err.pos, {
+    column: 11,
+    line: 1,
+    src: 'fn((s) => x)',
+  });
+});
+
+test.serial('should throw a type error', async (t) => {
+  const state = { data: { x: 1 } };
+
+  const job = `fn((s) => s())`;
+
+  //  Tell the compiler not to import x from the adaptor
+  const ignore = ['x'];
+
+  const result = await execute(job, state, 'common', ignore);
+  const err = result.errors['job-1'];
+  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.deepEqual(err.pos, {
+    column: 11,
+    line: 1,
+    src: 'fn((s) => s())',
+  });
+});
+
+// In http 6.4.3 this throws a type error
+// because the start of the error message is TypeError
+// In 6.5 we get a better error out
+// But the real question is: is AdaptorError even a useful error class?
+// I think it confuses people
+test.serial.skip('should throw an adaptor error', async (t) => {
+  const state = { data: { x: 1 } };
+
+  const job = `fn((s) => s)
+get("www")`;
+
+  const result = await execute(job, state, 'http');
+  const err = result.errors['job-1'];
+  t.log(err);
+
+  t.is(err.message, 'AdaptorError: INVALID_URL');
+});

integration-tests/worker/CHANGELOG.md

@@ -1,5 +1,16 @@
 # @openfn/integration-tests-worker
 
+## 1.0.73
+
+### Patch Changes
+
+- Updated dependencies [4ddd4d6]
+- Updated dependencies [6e87156]
+  - @openfn/ws-worker@1.9.0
+  - @openfn/logger@1.0.4
+  - @openfn/engine-multi@1.4.7
+  - @openfn/lightning-mock@2.0.28
+
 ## 1.0.72
 
 ### Patch Changes

integration-tests/worker/package.json

@@ -1,7 +1,7 @@
 {
   "name": "@openfn/integration-tests-worker",
   "private": true,
-  "version": "1.0.72",
+  "version": "1.0.73",
   "description": "Lightning WOrker integration tests",
   "author": "Open Function Group <[email protected]>",
   "license": "ISC",

package.json

@@ -10,7 +10,7 @@
     "export": "sh scripts/export.sh",
     "format": "prettier --write packages/*/src",
     "generate-slack-report": "node ./scripts/slack-publish-message.js",
-    "install:global": "pnpm build && pnpm run pack && node ./build/install-global.js",
+    "install:global": "rimraf dist && pnpm build && pnpm run pack && node ./build/install-global.js",
     "install:openfnx": "pnpm install:global",
     "pack:local": "pnpm run pack && node ./build/pack-local.js",
     "pack": "pnpm -r run pack",

packages/cli/CHANGELOG.md

@@ -1,5 +1,23 @@
 # @openfn/cli
 
+## 1.10.0
+
+### Minor Changes
+
+- Overhaul of error reporting in the CLI. Errors now include positional information and a stack trace, and reporting has generally been cleaned up a little bit.
+
+### Patch Changes
+
+- Updated dependencies [4ddd4d6]
+- Updated dependencies [93e9844]
+- Updated dependencies [6e87156]
+- Updated dependencies [aaa7e7b]
+- Updated dependencies [4ddd4d6]
+  - @openfn/runtime@1.6.0
+  - @openfn/compiler@1.0.0
+  - @openfn/logger@1.0.4
+  - @openfn/deploy@0.8.2
+
 ## 1.9.1
 
 ### Patch Changes

packages/cli/package.json

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