Sourcemapping

.changeset/fifty-seals-allow.md

@@ -0,0 +1,7 @@
+---
+'@openfn/runtime': minor
+'@openfn/cli': minor
+'@openfn/ws-worker': minor
+---
+
+Update errors to include a source-mapped position and more dignostic information

.changeset/popular-islands-double.md

@@ -0,0 +1,5 @@
+---
+'@openfn/compiler': major
+---
+
+Update main compile API to return a sourcemap

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

@@ -2,7 +2,6 @@ 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');
 
@@ -133,3 +132,5 @@ test.serial('invalid end (ambiguous)', async (t) => {
   assertLog(t, stdlogs, /Error: end pattern matched multiple steps/i);
   assertLog(t, stdlogs, /aborting/i);
 });
+
+// TODO lets add some errors here and take a close look at the output

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');
+});

packages/cli/src/compile/compile.ts

@@ -1,36 +1,52 @@
 import compile, { preloadAdaptorExports, Options } from '@openfn/compiler';
-import { getModulePath } from '@openfn/runtime';
+import { getModulePath, SourceMap } from '@openfn/runtime';
 import type { ExecutionPlan, Job } from '@openfn/lexicon';
 
 import createLogger, { COMPILER, Logger } from '../util/logger';
 import abort from '../util/abort';
 import type { CompileOptions } from './command';
 
-// Load and compile a job from a file, then return the result
-// This is designed to be re-used in different CLI steps
-export default async (
-  planOrPath: ExecutionPlan | string,
+export type CompiledJob = { code: string; map?: SourceMap };
+
+export default async function (
+  job: ExecutionPlan,
   opts: CompileOptions,
   log: Logger
-) => {
+): Promise<ExecutionPlan>;
+
+export default async function (
+  plan: string,
+  opts: CompileOptions,
+  log: Logger
+): Promise<CompiledJob>;
+
+export default async function (
+  planOrPath: string | ExecutionPlan,
+  opts: CompileOptions,
+  log: Logger
+): Promise<CompiledJob | ExecutionPlan> {
   if (typeof planOrPath === 'string') {
     const result = await compileJob(planOrPath as string, opts, log);
     log.success(`Compiled expression from ${opts.expressionPath}`);
     return result;
   }
 
-  const compiledPlan = compileWorkflow(planOrPath as ExecutionPlan, opts, log);
+  const compiledPlan = await compileWorkflow(
+    planOrPath as ExecutionPlan,
+    opts,
+    log
+  );
   log.success('Compiled all expressions in workflow');
 
   return compiledPlan;
-};
+}
 
 const compileJob = async (
   job: string,
   opts: CompileOptions,
   log: Logger,
   jobName?: string
-): Promise<string> => {
+): Promise<CompiledJob> => {
   try {
     const compilerOptions: Options = await loadTransformOptions(opts, log);
     return compile(job, compilerOptions);
@@ -41,8 +57,8 @@ const compileJob = async (
       e,
       'Check the syntax of the job expression:\n\n' + job
     );
-    // This will never actully execute
-    return '';
+    // This will never actually execute
+    return { code: job };
   }
 };
 
@@ -59,12 +75,14 @@ const compileWorkflow = async (
       adaptors: job.adaptors ?? opts.adaptors,
     };
     if (job.expression) {
-      job.expression = await compileJob(
+      const { code, map } = await compileJob(
         job.expression as string,
         jobOpts,
         log,
         job.id
       );
+      job.expression = code;
+      job.sourceMap = map;
     }
   }
   return plan;

packages/cli/src/compile/handler.ts

@@ -11,11 +11,12 @@ const compileHandler = async (options: CompileOptions, logger: Logger) => {
 
   let result;
   if (options.expressionPath) {
-    result = await compile(options.expressionPath, options, logger);
+    const { code } = await compile(options.expressionPath, options, logger);
+    result = code;
   } else {
     const plan = await loadPlan(options, logger);
-    result = await compile(plan, options, logger);
-    result = JSON.stringify(result, null, 2);
+    const compiledPlan = await compile(plan, options, logger);
+    result = JSON.stringify(compiledPlan, null, 2);
   }
 
   if (options.outputStdout) {

packages/cli/src/execute/handler.ts

@@ -105,7 +105,7 @@ const executeHandler = async (options: ExecuteOptions, logger: Logger) => {
   const state = await loadState(plan, options, logger, customStart);
 
   if (options.compile) {
-    plan = (await compile(plan, options, logger)) as ExecutionPlan;
+    plan = await compile(plan, options, logger);
   } else {
     logger.info('Skipping compilation as noCompile is set');
   }

packages/cli/test/compile/compile.test.ts

@@ -2,14 +2,16 @@ import test from 'ava';
 import mock from 'mock-fs';
 import path from 'node:path';
 import { createMockLogger } from '@openfn/logger';
+
 import compile, {
   stripVersionSpecifier,
   loadTransformOptions,
   resolveSpecifierPath,
 } from '../../src/compile/compile';
 import { CompileOptions } from '../../src/compile/command';
 import { mockFs, resetMockFs } from '../util';
-import { ExecutionPlan, Job } from '@openfn/lexicon';
+
+import type { ExecutionPlan, Job } from '@openfn/lexicon';
 
 const mockLog = createMockLogger();
 
@@ -27,18 +29,15 @@ type TransformOptionsWithImports = {
   };
 };
 
-// TODO this isn't really used and is a bit of a quirky thing
-// The compiler itself probably doesn't do any path parsing?
-// Just compile a source string and return the result
-test('compile from source string', async (t) => {
+test.serial('compile from source string', async (t) => {
   const job = 'x();';
 
   const opts = {} as CompileOptions;
 
-  const result = await compile(job, opts, mockLog);
+  const result = await compile(job, opts, mockLog)
 
   const expected = 'export default [x()];';
-  t.is(result, expected);
+  t.is(result.code, expected);
 });
 
 test.serial('compile from path', async (t) => {
@@ -54,10 +53,10 @@ test.serial('compile from path', async (t) => {
   const result = await compile(expressionPath, opts, mockLog);
 
   const expected = 'export default [x()];';
-  t.is(result, expected);
+  t.is(result.code, expected);
 });
 
-test('compile from execution plan', async (t) => {
+test.serial('compile from execution plan', async (t) => {
   const plan = {
     workflow: {
       steps: [
@@ -78,7 +77,7 @@ test('compile from execution plan', async (t) => {
   t.is((b as Job).expression, expected);
 });
 
-test('throw an AbortError if a job is uncompilable', async (t) => {
+test.serial('throw an AbortError if a job is uncompilable', async (t) => {
   const job = 'a b';
 
   const opts = {} as CompileOptions;
@@ -93,7 +92,7 @@ test('throw an AbortError if a job is uncompilable', async (t) => {
   t.assert(logger._find('error', /critical error: aborting command/i));
 });
 
-test('throw an AbortError if an xplan contains an uncompilable job', async (t) => {
+test.serial('throw an AbortError if an xplan contains an uncompilable job', async (t) => {
   const plan: ExecutionPlan = {
     workflow: {
       steps: [{ id: 'a', expression: 'x b' }],
@@ -113,35 +112,35 @@ test('throw an AbortError if an xplan contains an uncompilable job', async (t) =
   t.assert(logger._find('error', /critical error: aborting command/i));
 });
 
-test('stripVersionSpecifier: remove version specifier from @openfn', (t) => {
+test.serial('stripVersionSpecifier: remove version specifier from @openfn', (t) => {
   const specifier = '@openfn/[email protected]';
   const transformed = stripVersionSpecifier(specifier);
   const expected = '@openfn/language-common';
   t.assert(transformed == expected);
 });
 
-test('stripVersionSpecifier: remove version specifier from arbitrary package', (t) => {
+test.serial('stripVersionSpecifier: remove version specifier from arbitrary package', (t) => {
   const specifier = '[email protected]';
   const transformed = stripVersionSpecifier(specifier);
   const expected = 'ava';
   t.assert(transformed == expected);
 });
 
-test('stripVersionSpecifier: remove version specifier from arbitrary namespaced package', (t) => {
+test.serial('stripVersionSpecifier: remove version specifier from arbitrary namespaced package', (t) => {
   const specifier = '@ava/some-pkg@^1';
   const transformed = stripVersionSpecifier(specifier);
   const expected = '@ava/some-pkg';
   t.assert(transformed == expected);
 });
 
-test("stripVersionSpecifier: do nothing if there's no specifier", (t) => {
+test.serial("stripVersionSpecifier: do nothing if there's no specifier", (t) => {
   const specifier = '@openfn/language-common';
   const transformed = stripVersionSpecifier(specifier);
   const expected = '@openfn/language-common';
   t.assert(transformed == expected);
 });
 
-test('loadTransformOptions: do nothing', async (t) => {
+test.serial('loadTransformOptions: do nothing', async (t) => {
   const opts = {};
   const result = loadTransformOptions(opts, mockLog);
   t.assert(JSON.stringify(result) === '{}');
@@ -158,22 +157,22 @@ test.serial(
   }
 );
 
-test('resolveSpecifierPath: return a relative path if passed', async (t) => {
+test.serial('resolveSpecifierPath: return a relative path if passed', async (t) => {
   const path = await resolveSpecifierPath('pkg=./a', '/repo', mockLog);
   t.assert(path === './a');
 });
 
-test('resolveSpecifierPath: return an absolute path if passed', async (t) => {
+test.serial('resolveSpecifierPath: return an absolute path if passed', async (t) => {
   const path = await resolveSpecifierPath('pkg=/a', '/repo', mockLog);
   t.assert(path === '/a');
 });
 
-test('resolveSpecifierPath: return a path if passed', async (t) => {
+test.serial('resolveSpecifierPath: return a path if passed', async (t) => {
   const path = await resolveSpecifierPath('pkg=a/b/c', '/repo', mockLog);
   t.assert(path === 'a/b/c');
 });
 
-test('resolveSpecifierPath: basically return anything after the =', async (t) => {
+test.serial('resolveSpecifierPath: basically return anything after the =', async (t) => {
   const path = await resolveSpecifierPath('pkg=a', '/repo', mockLog);
   t.assert(path === 'a');