Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 34 commits into from
Jan 13, 2025
Merged
Show file tree
Hide file tree
Changes from 28 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
47914cf
compiler: generate a source map if a job name is passed
josephjclark Nov 3, 2024
adf907c
runtime: add a position mapping function
josephjclark Nov 4, 2024
7d607ce
runtime: map error position
josephjclark Dec 19, 2024
2564547
runtime: update test
josephjclark Dec 20, 2024
338e898
comment
josephjclark Dec 20, 2024
3d1b1ea
runtime: properly map positions and stack traces in errors
josephjclark Dec 20, 2024
c50fdaf
compiler: more tests
josephjclark Dec 22, 2024
c5f2149
cli: refactor to build sourcemaps up properly
josephjclark Dec 22, 2024
427f9fe
lexicon: updated typings
josephjclark Dec 22, 2024
b2e4ca5
runtime: nicely log errors with position and line of code
josephjclark Dec 22, 2024
bdf0623
runtime: tidy up
josephjclark Dec 22, 2024
98e5303
runtime: rewrite sourcemap tests and improve typings
josephjclark Dec 23, 2024
64a865e
runtime: fix tests
josephjclark Dec 23, 2024
a47a62b
runtime: ensure a sourcemap is set when a workflow is generated from …
josephjclark Dec 23, 2024
8d17b50
tests: add tests for error types
josephjclark Dec 23, 2024
a06145b
runtime: typing
josephjclark Dec 23, 2024
4eadff4
engine: adjust to new compiler API
josephjclark Dec 23, 2024
4ddd4d6
changesets
josephjclark Dec 23, 2024
e005322
runtime: update test
josephjclark Dec 23, 2024
76784e1
format
josephjclark Dec 23, 2024
11fe511
runtime: refine error output
josephjclark Dec 28, 2024
d62da9a
tests: added error logging tests
josephjclark Dec 28, 2024
93e9844
Sourcemapping adaptor errors (#851)
josephjclark Jan 3, 2025
aaa7e7b
Runtime: attempt to clean up error output (#852)
josephjclark Jan 4, 2025
6e87156
logger: ensure timestamp is added to print logs, so that the worker c…
josephjclark Jan 4, 2025
8320439
version: [email protected] [email protected]
josephjclark Jan 4, 2025
2a62d48
tmp: worker to rc1 version
josephjclark Jan 4, 2025
f056361
fix openfnx build
josephjclark Jan 8, 2025
8882c4c
runtime: simplify adaptorerror constructor
josephjclark Jan 12, 2025
acb57ae
tests: fix adaptor versions
josephjclark Jan 12, 2025
4a349a0
cli: skip flay test
josephjclark Jan 12, 2025
c5ec87c
tests: skip more flaky docs tests
josephjclark Jan 12, 2025
83e6054
worker: version to 1.9.0
josephjclark Jan 13, 2025
8804c1d
cli: update changelog
josephjclark Jan 13, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ public/
!public/index.html
tsconfig.tsbuildinfo
ts.cache
dist

# Build tools
.rollup.cache
Expand Down
1 change: 0 additions & 1 deletion dist/.gitignore

This file was deleted.

72 changes: 70 additions & 2 deletions integration-tests/cli/test/errors.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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',
]);
});
21 changes: 21 additions & 0 deletions integration-tests/cli/test/fixtures/errors.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
{
"workflow": {
"steps": [
{
"id": "ref",
"adaptor": "common",
"expression": "fn((state) => state.x.y)"
},
{
"id": "not-function",
"adaptor": "common",
"expression": "fn((state) => state())"
},
{
"id": "assign-const",
"adaptor": "common",
"expression": "fn((state) => { const x = 10; x = 20; })"
}
]
}
}
11 changes: 11 additions & 0 deletions integration-tests/execute/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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/[email protected]
- @openfn/[email protected]

## 1.0.12

### Patch Changes
Expand Down
2 changes: 1 addition & 1 deletion integration-tests/execute/package.json
Original file line number Diff line number Diff line change
@@ -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",
Expand Down
13 changes: 9 additions & 4 deletions integration-tests/execute/src/execute.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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}`,
Expand All @@ -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: {
Expand Down
72 changes: 72 additions & 0 deletions integration-tests/execute/test/errors.test.ts
Original file line number Diff line number Diff line change
@@ -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');
});
11 changes: 11 additions & 0 deletions integration-tests/worker/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,16 @@
# @openfn/integration-tests-worker

## 1.0.73

### Patch Changes

- Updated dependencies [4ddd4d6]
- Updated dependencies [6e87156]
- @openfn/[email protected]
- @openfn/[email protected]
- @openfn/[email protected]
- @openfn/[email protected]

## 1.0.72

### Patch Changes
Expand Down
2 changes: 1 addition & 1 deletion integration-tests/worker/package.json
Original file line number Diff line number Diff line change
@@ -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",
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
18 changes: 18 additions & 0 deletions packages/cli/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,23 @@
# @openfn/cli

## 1.10.0

### Minor Changes

- 4ddd4d6: Update errors to include a source-mapped position and more dignostic information

### Patch Changes

- Updated dependencies [4ddd4d6]
- Updated dependencies [93e9844]
- Updated dependencies [6e87156]
- Updated dependencies [aaa7e7b]
- Updated dependencies [4ddd4d6]
- @openfn/[email protected]
- @openfn/[email protected]
- @openfn/[email protected]
- @openfn/[email protected]

## 1.9.1

### Patch Changes
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/package.json
Original file line number Diff line number Diff line change
@@ -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",
Expand Down
Loading