Make loadJS use the same error handling as runJS (#199)

Author: calebeby

.changeset/early-rules-knock.md

@@ -0,0 +1,5 @@
+---
+'pleasantest': minor
+---
+
+Make loadJS share error mapping logic with runJS

src/index.ts

@@ -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';
@@ -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;
@@ -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',
@@ -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) => {
@@ -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 = {

src/source-map-error-from-browser.ts

@@ -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,
+};

tests/test-utils.ts

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

tests/utils/external-throwing.ts

@@ -0,0 +1,3 @@
+let foo: string;
+
+throw new Error('asdf');

tests/utils/loadJS.test.ts

@@ -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;
+            |       ^
+          # | 
+        "
+      `);
+  }),
+);