Skip to content

Commit

Permalink
Make loadJS use the same error handling as runJS (#199)
Browse files Browse the repository at this point in the history
  • Loading branch information
calebeby authored Aug 9, 2021
1 parent a335563 commit 8e26bea
Show file tree
Hide file tree
Showing 7 changed files with 205 additions and 130 deletions.
5 changes: 5 additions & 0 deletions .changeset/early-rules-knock.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'pleasantest': minor
---

Make loadJS share error mapping logic with runJS
108 changes: 19 additions & 89 deletions src/index.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import * as puppeteer from 'puppeteer';
import { relative, join, isAbsolute, dirname, posix, sep, resolve } from 'path';
import { relative, join, isAbsolute, dirname } from 'path';
import type { BoundQueries } from './pptr-testing-library';
import { getQueriesForElement } from './pptr-testing-library';
import { connectToBrowser } from './connect-to-browser';
Expand All @@ -11,16 +11,13 @@ import _ansiRegex from 'ansi-regex';
import { fileURLToPath } from 'url';
import type { PleasantestUser } from './user';
import { pleasantestUser } from './user';
import {
assertElementHandle,
printStackLine,
removeFuncFromStackTrace,
} from './utils';
import { assertElementHandle, removeFuncFromStackTrace } from './utils';
import type { ModuleServerOpts } from './module-server';
import { createModuleServer } from './module-server';
import { cleanupClientRuntimeServer } from './module-server/client-runtime-server';
import { Console } from 'console';
import { createBuildStatusTracker } from './module-server/build-status-tracker';
import { sourceMapErrorFromBrowser } from './source-map-error-from-browser';

export { JSHandle, ElementHandle } from 'puppeteer';
koloristOpts.enabled = true;
Expand Down Expand Up @@ -320,7 +317,7 @@ const createTab = async ({
// This uses the testPath as the url so that if there are relative imports
// in the inline code, the relative imports are resolved relative to the test file
const url = `http://localhost:${port}/${testPath}?inline-code=${encodedCode}&build-id=${buildStatus.buildId}`;
const res = (await safeEvaluate(
const res = await safeEvaluate(
runJS,
new Function(
'...args',
Expand All @@ -334,91 +331,13 @@ const createTab = async ({
: e)`,
) as () => any,
...(Array.isArray(args) ? (args as any) : []),
)) as undefined | { message: string; stack: string };
);

const errorsFromBuild = buildStatus.complete();
// It only throws the first one but that is probably OK
if (errorsFromBuild) throw errorsFromBuild[0];

if (res === undefined) return;
if (typeof res !== 'object') throw res;
const { message, stack } = res;
const parsedStack = parseStackTrace(stack);
const modifiedStack = parsedStack.map(async (stackItem) => {
if (stackItem.raw.startsWith(stack.slice(0, stack.indexOf('\n'))))
return null;
if (!stackItem.fileName) return stackItem.raw;
const fileName = stackItem.fileName;
const line = stackItem.line;
const column = stackItem.column;
if (!fileName.startsWith(`http://localhost:${port}`))
return stackItem.raw;
const url = new URL(fileName);
const osPath = url.pathname.slice(1).split(posix.sep).join(sep);
// Absolute file path
const file = resolve(process.cwd(), osPath);
// Rollup-style Unix-normalized path "id":
const id = file.split(sep).join(posix.sep);
const transformResult = requestCache.get(id);
const map = typeof transformResult === 'object' && transformResult.map;
if (!map) {
let p = url.pathname;
const npmPrefix = '/@npm/';
if (p.startsWith(npmPrefix))
p = join(process.cwd(), 'node_modules', p.slice(npmPrefix.length));
return printStackLine(p, line, column, stackItem.name);
}

const { SourceMapConsumer } = await import('source-map');
const consumer = await new SourceMapConsumer(map as any);
const sourceLocation = consumer.originalPositionFor({
line,
column: column - 1, // Source-map uses zero-based column numbers
});
consumer.destroy();
return printStackLine(
join(process.cwd(), url.pathname),
sourceLocation.line ?? line,
sourceLocation.column === null
? column
: // Convert back from zero-based column to 1-based
sourceLocation.column + 1,
stackItem.name,
);
});
const errorName = stack.slice(0, stack.indexOf(':')) || 'Error';
const specializedErrors = {
EvalError,
RangeError,
ReferenceError,
SyntaxError,
TypeError,
URIError,
} as any;
const ErrorConstructor: ErrorConstructor =
specializedErrors[errorName] || Error;
const error = new ErrorConstructor(message);

const finalStack = (await Promise.all(modifiedStack))
.filter(Boolean)
.join('\n');

// If the browser error did not provide a stack, use the stack trace from node
if (finalStack) {
error.stack = `${errorName}: ${message}\n${finalStack}`;
} else {
removeFuncFromStackTrace(error, runJS);
if (error.stack)
error.stack = error.stack
.split('\n')
.filter(
// This was appearing in stack traces and it messed up the Jest output
(line) => !(/runMicrotasks/.test(line) && /<anonymous>/.test(line)),
)
.join('\n');
}

throw error;
await sourceMapErrorFromBrowser(res, requestCache, port, runJS);
};

const injectHTML: PleasantestUtils['injectHTML'] = async (html) => {
Expand Down Expand Up @@ -459,10 +378,21 @@ const createTab = async ({
const fullPath = jsPath.startsWith('.')
? join(dirname(testPath), jsPath)
: jsPath;
await safeEvaluate(
const buildStatus = createBuildStatusTracker();
const url = `http://localhost:${port}/${fullPath}?build-id=${buildStatus.buildId}`;
const res = await safeEvaluate(
loadJS,
`import(${JSON.stringify(`http://localhost:${port}/${fullPath}`)})`,
`import(${JSON.stringify(url)})
.catch(e => e instanceof Error
? { message: e.message, stack: e.stack }
: e)`,
);

const errorsFromBuild = buildStatus.complete();
// It only throws the first one but that is probably OK
if (errorsFromBuild) throw errorsFromBuild[0];

await sourceMapErrorFromBrowser(res, requestCache, port, loadJS);
};

const utils: PleasantestUtils = {
Expand Down
100 changes: 100 additions & 0 deletions src/source-map-error-from-browser.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
import { join, posix, sep, resolve } from 'path';
import { parseStackTrace } from 'errorstacks';
import _ansiRegex from 'ansi-regex';
import { printStackLine, removeFuncFromStackTrace } from './utils';
import type { SourceDescription } from 'rollup';

export const sourceMapErrorFromBrowser = async (
res: unknown,
requestCache: Map<string, SourceDescription>,
port: number,
func: (...args: any[]) => void,
) => {
if (res === undefined) return;
if (
typeof res !== 'object' ||
!res ||
!('message' in res) ||
!('stack' in res)
)
throw res;
const { message, stack } = res as {
message: string;
stack: string;
};
const parsedStack = parseStackTrace(stack);
const modifiedStack = parsedStack.map(async (stackItem) => {
if (stackItem.raw.startsWith(stack.slice(0, stack.indexOf('\n'))))
return null;
if (!stackItem.fileName) return stackItem.raw;
const fileName = stackItem.fileName;
const line = stackItem.line;
const column = stackItem.column;
if (!fileName.startsWith(`http://localhost:${port}`)) return stackItem.raw;
const url = new URL(fileName);
const osPath = url.pathname.slice(1).split(posix.sep).join(sep);
// Absolute file path
const file = resolve(process.cwd(), osPath);
// Rollup-style Unix-normalized path "id":
const id = file.split(sep).join(posix.sep);
const transformResult = requestCache.get(id);
const map = typeof transformResult === 'object' && transformResult.map;
if (!map) {
let p = url.pathname;
const npmPrefix = '/@npm/';
if (p.startsWith(npmPrefix))
p = join(process.cwd(), 'node_modules', p.slice(npmPrefix.length));
return printStackLine(p, line, column, stackItem.name);
}

const { SourceMapConsumer } = await import('source-map');
const consumer = await new SourceMapConsumer(map as any);
const sourceLocation = consumer.originalPositionFor({
line,
column: column - 1, // Source-map uses zero-based column numbers
});
consumer.destroy();
return printStackLine(
join(process.cwd(), url.pathname),
sourceLocation.line ?? line,
sourceLocation.column === null
? column
: // Convert back from zero-based column to 1-based
sourceLocation.column + 1,
stackItem.name,
);
});
const errorName = stack.slice(0, stack.indexOf(':')) || 'Error';
const ErrorConstructor = specializedErrors[errorName] || Error;
const error = new ErrorConstructor(message);

const finalStack = (await Promise.all(modifiedStack))
.filter(Boolean)
.join('\n');

// If the browser error did not provide a stack, use the stack trace from node
if (finalStack) {
error.stack = `${errorName}: ${message}\n${finalStack}`;
} else {
removeFuncFromStackTrace(error, func);
if (error.stack)
error.stack = error.stack
.split('\n')
.filter(
// This was appearing in stack traces and it messed up the Jest output
(line) => !(/runMicrotasks/.test(line) && /<anonymous>/.test(line)),
)
.join('\n');
}

throw error;
};

const specializedErrors: Record<string, ErrorConstructor | undefined> = {
EvalError,
RangeError,
ReferenceError,
SyntaxError,
TypeError,
URIError,
};
25 changes: 25 additions & 0 deletions tests/test-utils.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { parseStackTrace } from 'errorstacks';
import { promises as fs } from 'fs';
import * as path from 'path';
import ansiRegex from 'ansi-regex';

export const printErrorFrames = async (error?: Error) => {
if (!error?.stack) return '';
Expand Down Expand Up @@ -45,3 +46,27 @@ export const printErrorFrames = async (error?: Error) => {
`\n${'-'.repeat(55)}\n`,
);
};

const stripAnsi = (input: string) => input.replace(ansiRegex(), '');

const removeLineNumbers = (input: string) => {
const lineRegex = /^(\s*>?\s*)(\d+)/gm;
const fileRegex = new RegExp(`${process.cwd()}([a-zA-Z/._-]*)[\\d:]*`, 'g');
return (
input
.replace(
lineRegex,
(_match, whitespace, numbers) =>
`${whitespace}${'#'.repeat(numbers.length)}`,
)
// Take out the file paths so the tests will pass on more than 1 person's machine
.replace(fileRegex, '<root>$1:###:###')
);
};

export const formatErrorWithCodeFrame = <T extends any>(input: Promise<T>) =>
input.catch((error) => {
error.message = removeLineNumbers(stripAnsi(error.message));
error.stack = removeLineNumbers(stripAnsi(error.stack));
throw error;
});
3 changes: 3 additions & 0 deletions tests/utils/external-throwing.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
let foo: string;

throw new Error('asdf');
41 changes: 39 additions & 2 deletions tests/utils/loadJS.test.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,40 @@
import { withBrowser } from 'pleasantest';
import { formatErrorWithCodeFrame, printErrorFrames } from '../test-utils';

test.todo('loads from .ts file with transpiling');
test.todo('if the file throws an error the error is source mapped');
test.todo('if the file has a syntax error the location is source mapped');

test(
'if the file throws an error the error is source mapped',
withBrowser(async ({ utils }) => {
const error = await utils
.loadJS('./external-throwing.ts')
.catch((error) => error);
expect(await printErrorFrames(error)).toMatchInlineSnapshot(`
"Error: asdf
-------------------------------------------------------
tests/utils/external-throwing.ts
throw new Error('asdf');
^"
`);
}),
);

test(
'if the file has a syntax error the location is source mapped',
withBrowser(async ({ utils }) => {
const loadPromise = utils.loadJS('./external-with-syntax-error.ts');
await expect(formatErrorWithCodeFrame(loadPromise)).rejects
.toThrowErrorMatchingInlineSnapshot(`
"[esbuild] The constant \\"someVariable\\" must be initialized
<root>/tests/utils/external-with-syntax-error.ts:###:###
# | // @ts-expect-error: this is intentionally invalid
> # | const someVariable: string;
| ^
# |
"
`);
}),
);
Loading

0 comments on commit 8e26bea

Please sign in to comment.