prevent double-fixing of stack traces (#4041)

* prevent double-fixing of stack traces - fixes #3638

* dont mutate errors

* failing test

* linting etc

* skip test for now

* add link to vite issue

.changeset/angry-pugs-play.md

@@ -0,0 +1,5 @@
+---
+'@sveltejs/kit': patch
+---
+
+Prevent double-fixing of error stack traces in dev mode

packages/kit/src/core/dev/plugin.js

@@ -134,6 +134,24 @@ export async function create_plugin(config, cwd) {
 				};
 			}
 
+			/** @param {Error} error */
+			function fix_stack_trace(error) {
+				// TODO https://github.com/vitejs/vite/issues/7045
+
+				// ideally vite would expose ssrRewriteStacktrace, but
+				// in lieu of that, we can implement it ourselves. we
+				// don't want to mutate the error object, because
+				// the stack trace could be 'fixed' multiple times,
+				// and Vite will fix stack traces before we even
+				// see them if they occur during ssrLoadModule
+				const original = error.stack;
+				vite.ssrFixStacktrace(error);
+				const fixed = error.stack;
+				error.stack = original;
+
+				return fixed;
+			}
+
 			update_manifest();
 
 			vite.watcher.on('add', update_manifest);
@@ -238,13 +256,19 @@ export async function create_plugin(config, cwd) {
 							dev: true,
 							floc: config.kit.floc,
 							get_stack: (error) => {
-								vite.ssrFixStacktrace(error);
-								return error.stack;
+								return fix_stack_trace(error);
 							},
 							handle_error: (error, event) => {
-								vite.ssrFixStacktrace(error);
 								hooks.handleError({
-									error,
+									error: new Proxy(error, {
+										get: (target, property) => {
+											if (property === 'stack') {
+												return fix_stack_trace(error);
+											}
+
+											return Reflect.get(target, property, target);
+										}
+									}),
 									event,
 
 									// TODO remove for 1.0

packages/kit/test/apps/basics/src/routes/errors/endpoint.json.js

@@ -1,4 +1,3 @@
-/** @type {import('@sveltejs/kit').RequestHandler} */
 export function get() {
 	throw new Error('nope');
 }

packages/kit/test/apps/basics/src/routes/errors/init-error-endpoint.js

@@ -0,0 +1,10 @@
+// @ts-expect-error
+thisvariableisnotdefined; // eslint-disable-line
+
+export function get() {
+	return {
+		body: {
+			answer: 42
+		}
+	};
+}

packages/kit/test/apps/basics/test/test.js

@@ -908,7 +908,7 @@ test.describe.parallel('Errors', () => {
 		expect(lines[0]).toMatch('nope');
 
 		if (process.env.DEV) {
-			expect(lines[1]).toMatch('endpoint-shadow');
+			expect(lines[1]).toMatch('endpoint-shadow.js:3:8');
 		}
 
 		expect(res && res.status()).toBe(500);
@@ -917,7 +917,7 @@ test.describe.parallel('Errors', () => {
 		);
 
 		const contents = await page.textContent('#stack');
-		const location = 'endpoint-shadow.js:1:8'; // TODO this is the wrong location, but i'm not going to open the sourcemap can of worms just now
+		const location = 'endpoint-shadow.js:3:8';
 
 		if (process.env.DEV) {
 			expect(contents).toMatch(location);
@@ -964,6 +964,14 @@ test.describe.parallel('Errors', () => {
 		);
 		expect(await page.innerHTML('h1')).toBe('500');
 	});
+
+	// TODO re-enable this if https://github.com/vitejs/vite/issues/7046 is implemented
+	test.skip('error evaluating module', async ({ request }) => {
+		const response = await request.get('/errors/init-error-endpoint');
+
+		expect(response.status()).toBe(500);
+		expect(await response.text()).toMatch('thisvariableisnotdefined is not defined');
+	});
 });
 
 test.describe.parallel('ETags', () => {