fix: catchable transform errors (pvtnbr/tsx#7)

privatenumber · May 1, 2024 · 084dec0 · 084dec0
1 parent 99ba136
commit 084dec0
Show file tree

Hide file tree

Showing 4 changed files with 80 additions and 61 deletions.
diff --git a/src/utils/transform/index.ts b/src/utils/transform/index.ts
@@ -22,21 +22,15 @@ import {
 	patchOptions,
 } from './get-esbuild-options.js';
 
-const handleEsbuildError = (
+const formatEsbuildError = (
 	error: TransformFailure,
 ) => {
-	const [firstError] = error.errors;
-	let errorMessage = `[esbuild Error]: ${firstError.text}`;
-
-	if (firstError.location) {
-		const { file, line, column } = firstError.location;
-		errorMessage += `\n    at ${file}:${line}:${column}`;
-	}
-
-	console.error(errorMessage);
-
-	// eslint-disable-next-line n/no-process-exit
-	process.exit(1);
+	error.name = 'TransformError';
+	// @ts-expect-error deleting non-option property
+	delete error.errors;
+	// @ts-expect-error deleting non-option property
+	delete error.warnings;
+	throw error;
 };
 
 // Used by cjs-loader
@@ -76,19 +70,14 @@ export const transformSync = (
 			[
 				// eslint-disable-next-line @typescript-eslint/no-shadow
 				(_filePath, code) => {
-					const patchResults = patchOptions(esbuildOptions);
+					const patchResult = patchOptions(esbuildOptions);
+					let result;
 					try {
-						return patchResults(
-							esbuildTransformSync(code, esbuildOptions),
-						);
+						result = esbuildTransformSync(code, esbuildOptions);
 					} catch (error) {
-						handleEsbuildError(error as TransformFailure);
-
-						/**
-						 * esbuild warnings are ignored because they're usually caught
-						 * at runtime by Node.js with better errors + stack traces
-						 */
+						throw formatEsbuildError(error as TransformFailure);
 					}
+					return patchResult(result);
 				},
 				transformDynamicImport,
 			],
@@ -128,19 +117,14 @@ export const transform = async (
 			[
 				// eslint-disable-next-line @typescript-eslint/no-shadow
 				async (_filePath, code) => {
-					const patchResults = patchOptions(esbuildOptions);
+					const patchResult = patchOptions(esbuildOptions);
+					let result;
 					try {
-						return patchResults(
-							await esbuildTransform(code, esbuildOptions),
-						);
+						result = await esbuildTransform(code, esbuildOptions);
 					} catch (error) {
-						handleEsbuildError(error as TransformFailure);
-
-						/**
-						 * esbuild warnings are ignored because they're usually caught
-						 * at runtime by Node.js with better errors + stack traces
-						 */
+						throw formatEsbuildError(error as TransformFailure);
 					}
+					return patchResult(result);
 				},
 				transformDynamicImport,
 			],

diff --git a/tests/index.ts b/tests/index.ts
@@ -10,6 +10,7 @@ import { nodeVersions } from './utils/node-versions';
 		for (const nodeVersion of nodeVersions) {
 			const node = await createNode(nodeVersion);
 			await describe(`Node ${node.version}`, async ({ runTestSuite }) => {
+				await runTestSuite(import('./specs/api'), node);
 				await runTestSuite(import('./specs/cli'), node);
 				await runTestSuite(import('./specs/api'), node);
 				await runTestSuite(import('./specs/watch'), node);

diff --git a/tests/specs/api.ts b/tests/specs/api.ts
@@ -58,39 +58,59 @@ export default testSuite(({ describe }, node: NodeApis) => {
 				expect(stdout).toBe('Fails as expected\nfoo bar\nUnregistered');
 			});
 
-			test('tsx.require()', async ({ onTestFinish }) => {
-				const fixture = await createFixture({
-					'require.cjs': `
-					const tsx = require(${JSON.stringify(cjsApiPath)});
-					try {
-						require('./file');
-					} catch {
-						console.log('Fails as expected');
-					}
+			describe('tsx.require()', ({ test }) => {
+				test('loads', async ({ onTestFinish }) => {
+					const fixture = await createFixture({
+						'require.cjs': `
+						const tsx = require(${JSON.stringify(cjsApiPath)});
+						try {
+							require('./file');
+						} catch {
+							console.log('Fails as expected');
+						}
 
-					const loaded = tsx.require('./file', __filename);
-					console.log(loaded.message);
+						const loaded = tsx.require('./file', __filename);
+						console.log(loaded.message);
 
-					// Remove from cache
-					const loadedPath = tsx.require.resolve('./file', __filename);
-					delete require.cache[loadedPath];
+						// Remove from cache
+						const loadedPath = tsx.require.resolve('./file', __filename);
+						delete require.cache[loadedPath];
 
-					try {
-						require('./file');
-					} catch {
-						console.log('Unpolluted global require');
-					}
-					`,
-					...tsFiles,
-				});
-				onTestFinish(async () => await fixture.rm());
+						try {
+							require('./file');
+						} catch {
+							console.log('Unpolluted global require');
+						}
+						`,
+						...tsFiles,
+					});
+					onTestFinish(async () => await fixture.rm());
 
-				const { stdout } = await execaNode(path.join(fixture.path, 'require.cjs'), [], {
-					nodePath: node.path,
-					nodeOptions: [],
+					const { stdout } = await execaNode(path.join(fixture.path, 'require.cjs'), [], {
+						nodePath: node.path,
+						nodeOptions: [],
+					});
+
+					expect(stdout).toBe('Fails as expected\nfoo bar\nUnpolluted global require');
 				});
 
-				expect(stdout).toBe('Fails as expected\nfoo bar\nUnpolluted global require');
+				test('catchable', async ({ onTestFinish }) => {
+					const fixture = await createFixture({
+						'require.cjs': `
+						const tsx = require(${JSON.stringify(cjsApiPath)});
+						try { tsx.require('./file', __filename); } catch {}
+						`,
+						'file.ts': 'if',
+					});
+					onTestFinish(async () => await fixture.rm());
+
+					const { all } = await execaNode(path.join(fixture.path, 'require.cjs'), [], {
+						nodePath: node.path,
+						nodeOptions: [],
+						all: true,
+					});
+					expect(all).toBe('');
+				});
 			});
 		});
 

diff --git a/tests/specs/smoke.ts b/tests/specs/smoke.ts
@@ -202,7 +202,7 @@ const files = {
 				if (!thrown) {
 					return new Error('No error thrown');
 				} else if (!thrown.message.includes(expectedError)) {
-					return new Error(\`Message \${JSON.stringify(expectedError)} not found in \${JSON.stringify(thrown.message)}\`);
+					return new Error(\`Message \${JSON.stringify(expectedError)} not found in \${JSON.stringify(thrown.message)}\n\${thrown.stack}\`);
 				}
 			}),
 		);
@@ -226,6 +226,8 @@ const files = {
 	console.log('imported');
 	`,
 
+	'broken-syntax.ts': 'if',
+
 	node_modules: {
 		'pkg-commonjs': {
 			'package.json': JSON.stringify({
@@ -425,6 +427,12 @@ export default testSuite(async ({ describe }, { tsx }: NodeApis) => {
 									`
 									: ''
 							}
+							${
+								isCommonJs
+									? '[() => require(\'./broken-syntax\'), \'Transform failed\'],'
+									: ''
+							}
+							[() => import('./broken-syntax'), 'Transform failed'],
 						);
 
 						console.log(JSON.stringify({
@@ -608,6 +616,12 @@ export default testSuite(async ({ describe }, { tsx }: NodeApis) => {
 									`
 									: ''
 							}
+							${
+								isCommonJs
+									? '[() => require(\'./broken-syntax\'), \'Transform failed\'],'
+									: ''
+							}
+							[() => import('./broken-syntax'), 'Transform failed'],
 						);
 
 						console.log(JSON.stringify({