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 2 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
14 changes: 10 additions & 4 deletions packages/compiler/src/compile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ const defaultLogger = createLogger();
// }

export type Options = TransformOptions & {
name?: string;
logger?: Logger;
logCompiledSource?: boolean;
};
Expand All @@ -27,15 +28,20 @@ export default function compile(pathOrSource: string, options: Options = {}) {
} else {
//logger.debug('Starting compilation from string');
}
const ast = parse(source);

const name = options.name ?? 'src';
const ast = parse(source, { name });

const transformedAst = transform(ast, undefined, options);

const compiledSource = print(transformedAst).code;
const { code, map } = print(transformedAst, {
sourceMapName: `${name}.map.js`,
});

if (options.logCompiledSource) {
logger.debug('Compiled source:');
logger.debug(compiledSource); // TODO indent or something
logger.debug(code);
}

return compiledSource;
return { code, map };
}
8 changes: 7 additions & 1 deletion packages/compiler/src/parse.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,17 @@
import recast from 'recast';
import * as acorn from 'acorn';

export default function parse(source: string) {
type Options = {
/** Name of the source job (no file extension). This triggers source map generation */
name?: string;
};

export default function parse(source: string, options: Options = {}) {
// This is copied from v1 but I am unsure the usecase
const escaped = source.replace(/\ $/, '');

const ast = recast.parse(escaped, {
sourceFileName: options.name && `${options.name}.js`,
tolerant: true,
range: true,
parser: {
Expand Down
46 changes: 28 additions & 18 deletions packages/compiler/test/compile.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,53 +3,63 @@ import fs from 'node:fs/promises';
import path from 'node:path';
import compile from '../src/compile';

// Not doing deep testing on this because recast does the heavy lifting
// This is just to ensure the map is actually generated
test('generate a source map if a file name is passed', (t) => {
const source = 'fn();';
const { map } = compile(source, { name: 'job' });
t.truthy(map);
t.deepEqual(map.sources, ['job.js']);
t.is(map.file, 'job.map.js');
});

test('ensure default exports is created', (t) => {
const source = '';
const expected = 'export default [];';
const result = compile(source);
const { code: result } = compile(source);
t.is(result, expected);
});

test('do not add default exports if exports exist', (t) => {
const source = 'export const x = 10;';
const expected = 'export const x = 10;';
const result = compile(source);
const { code: result } = compile(source);
t.is(result, expected);
});

test('compile a single operation', (t) => {
const source = 'fn();';
const expected = 'export default [fn()];';
const result = compile(source);
const { code: result } = compile(source);
t.is(result, expected);
});

test('compile a single namespaced operation', (t) => {
const source = 'http.get();';
const expected = 'export default [http.get()];';
const result = compile(source);
const { code: result } = compile(source);
t.is(result, expected);
});

test('compile a const assignment with single method call', (t) => {
const source = 'const x = dateFns.parse()';
const expected = `const x = dateFns.parse()
export default [];`;
const result = compile(source);
const { code: result } = compile(source);
t.is(result, expected);
});

test('compile a single operation without being fussy about semicolons', (t) => {
const source = 'fn()';
const expected = 'export default [fn()];';
const result = compile(source);
const { code: result } = compile(source);
t.is(result, expected);
});

test('compile multiple operations', (t) => {
const source = 'fn();fn();fn();';
const expected = 'export default [fn(), fn(), fn()];';
const result = compile(source);
const { code: result } = compile(source);
t.is(result, expected);
});

Expand All @@ -66,7 +76,7 @@ test('add imports', (t) => {
};
const source = 'fn();';
const expected = `import { fn } from "@openfn/language-common";\nexport default [fn()];`;
const result = compile(source, options);
const { code: result } = compile(source, options);
t.is(result, expected);
});

Expand All @@ -84,7 +94,7 @@ test('do not add imports', (t) => {
// This example already has the correct imports declared, so add-imports should do nothing
const source = "import { fn } from '@openfn/language-common'; fn();";
const expected = `import { fn } from '@openfn/language-common';\nexport default [fn()];`;
const result = compile(source, options);
const { code: result } = compile(source, options);
t.is(result, expected);
});

Expand All @@ -101,7 +111,7 @@ test('dumbly add imports', (t) => {
// This example already has the correct imports declared, so add-imports should do nothing
const source = "import { jam } from '@openfn/language-common'; jam(state);";
const expected = `import { jam } from '@openfn/language-common';\nexport default [jam(state)];`;
const result = compile(source, options);
const { code: result } = compile(source, options);
t.is(result, expected);
});

Expand All @@ -119,7 +129,7 @@ test('add imports with export all', (t) => {
};
const source = 'fn();';
const expected = `import { fn } from "@openfn/language-common";\nexport * from "@openfn/language-common";\nexport default [fn()];`;
const result = compile(source, options);
const { code: result } = compile(source, options);
t.is(result, expected);
});

Expand All @@ -134,28 +144,28 @@ test('twitter example', async (t) => {
path.resolve('test/jobs/twitter.compiled.js'),
'utf8'
);
const result = compile(source);
const { code: result } = compile(source);
t.deepEqual(result, expected);
});

test('compile with optional chaining', (t) => {
const source = 'fn(a.b?.c);';
const expected = 'export default [fn(a.b?.c)];';
const result = compile(source);
const { code: result } = compile(source);
t.is(result, expected);
});

test('compile with nullish coalescence', (t) => {
const source = 'fn(a ?? b);';
const expected = 'export default [fn(a ?? b)];';
const result = compile(source);
const { code: result } = compile(source);
t.is(result, expected);
});

test('compile a lazy state ($) expression', (t) => {
const source = 'get($.data.endpoint);';
const expected = 'export default [get(state => state.data.endpoint)];';
const result = compile(source);
const { code: result } = compile(source);
t.is(result, expected);
});

Expand All @@ -175,7 +185,7 @@ test('compile a lazy state ($) expression with dumb imports', (t) => {
export * from "@openfn/language-common";
export default [get(state => state.data.endpoint)];`;

const result = compile(source, options);
const { code: result } = compile(source, options);
t.is(result, expected);
});

Expand All @@ -190,7 +200,7 @@ export default [_defer(
p => p.then((s => { console.log(s.data); return state;} ))
)];`;

const result = compile(source);
const { code: result } = compile(source);
t.is(result, expected);
});

Expand All @@ -207,6 +217,6 @@ export default [each(
_defer(post("/upsert", (state) => state.data), p => p.then((s) => s))
)];`;

const result = compile(source);
const { code: result } = compile(source);
t.is(result, expected);
});
3 changes: 3 additions & 0 deletions packages/compiler/test/parse.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ import parse from '../src/parse';

import { loadAst } from './util';

// Note that these tests do not include source mappings
// just because the ast changes and develops circular structures

test('parse a simple statement', (t) => {
const source = 'const x = 10;';

Expand Down
6 changes: 5 additions & 1 deletion packages/runtime/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,16 @@
"author": "Open Function Group <[email protected]>",
"license": "ISC",
"devDependencies": {
"@openfn/compiler": "workspace:^",
"@openfn/language-common": "2.0.0-rc3",
"@openfn/lexicon": "workspace:^",
"@types/mock-fs": "^4.13.1",
"@types/node": "^18.15.13",
"@types/semver": "^7.5.0",
"ava": "5.3.1",
"mock-fs": "^5.4.1",
"recast": "^0.21.5",
"ts-node": "^10.9.1",
"tslib": "^2.4.0",
"tsup": "^7.2.0",
"typescript": "^5.1.6"
Expand All @@ -45,6 +48,7 @@
"dependencies": {
"@openfn/logger": "workspace:*",
"fast-safe-stringify": "^2.1.1",
"semver": "^7.5.4"
"semver": "^7.5.4",
"source-map": "^0.7.4"
}
}
11 changes: 4 additions & 7 deletions packages/runtime/src/runtime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,29 +5,26 @@ import type { LinkerOptions } from './modules/linker';
import executePlan from './execute/plan';
import { defaultState, parseRegex, clone } from './util/index';

// TODO we should be able to get a proper typing for this from somewherewhere
type SourceMap = any;

export type Options = {
logger?: Logger;
jobLogger?: Logger;

// Treat state as immutable (likely to break in legacy jobs)
immutableState?: boolean;

// TODO currently unused
// Ensure that all incoming jobs are sandboxed / loaded as text
// In practice this means throwing if someone tries to pass live js
forceSandbox?: boolean;

linker?: LinkerOptions;

callbacks?: ExecutionCallbacks;

// inject globals into the environment
// TODO leaving this here for now, but maybe its actually on the xplan?
globals?: any;

statePropsToRemove?: string[];

defaultRunTimeoutMs?: number;
sourceMap?: SourceMap;
};

type RawOptions = Omit<Options, 'linker'> & {
Expand Down
18 changes: 18 additions & 0 deletions packages/runtime/src/util/get-mapped-position.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// TODO rename the file to source mapping
// 1 function to get the position
// 1 function to extract the line from the source

import { SourceMapConsumer } from 'source-map';

const getMappedPosition = async (map, line, column) => {
const smc = await new SourceMapConsumer(map);
const pos = smc.originalPositionFor({
line,
column,
});

//return { line, col, src };
return pos;
};

export default getMappedPosition;
10 changes: 8 additions & 2 deletions packages/runtime/test/errors.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import test from 'ava';
import path from 'node:path';
import type { WorkflowOptions } from '@openfn/lexicon';
import compile from '@openfn/compiler';

import run from '../src/runtime';

Expand Down Expand Up @@ -47,13 +48,18 @@ test('crash on runtime error with SyntaxError', async (t) => {
t.is(error.message, 'SyntaxError: Invalid or unexpected token');
});

test('crash on runtime error with ReferenceError', async (t) => {
test.only('crash on runtime error with ReferenceError', async (t) => {
doc-han marked this conversation as resolved.
Show resolved Hide resolved
const expression = 'export default [(s) => x]';

// compile the code so we get a source map
const { code, map } = compile(expression, { name: 'src' });
console.log({ code, map });

let error: any;
try {
await run(expression);
await run(code, { map });
} catch (e) {
console.log(e);
error = e;
}
t.log(error);
Expand Down
43 changes: 43 additions & 0 deletions packages/runtime/test/util/get-mapped-position.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import test from 'ava';
import recast from 'recast';
import getMappedPosition from '../../src/util/get-mapped-position';

const b = recast.types.builders;

// compile an expression into a function
const compile = (src: string) => {
const ast = recast.parse(src, {
sourceFileName: 'src.js',
});

// take the expression and wrap it in a function declaration
const [{ expression }] = ast.program.body;
const fn = b.functionDeclaration(
b.identifier('fn'),
[],
b.blockStatement([b.returnStatement(expression)])
);

ast.program.body.push(fn);

return recast.print(fn, {
sourceMapName: 'src.map.js',
});
};

test('should return a sourcemapped position', async (t) => {
// Compile this simple expression into a function
const { code, map } = compile('x + 1');

// Now work out where x is
const lines = code.split('\n');
const line = 2; // line is 1 based
const column = lines[1].indexOf('x');

// now get the uncompiled position for x
const result = await getMappedPosition(map, line, column);

// now check the position
t.is(result.line, 1);
t.is(result.column, 0);
});
Loading