Skip to content

Commit

Permalink
prevent double stack trace 'fixes' (#5644)
Browse files Browse the repository at this point in the history
* tidy up

* reset stack traces to avoid double fix - closes #3371

* use options.dev instead of __SVELTEKIT_DEV__ so that testing against src still works

* weird
  • Loading branch information
Rich-Harris authored Jul 21, 2022
1 parent 2045437 commit 68a30cc
Show file tree
Hide file tree
Showing 6 changed files with 39 additions and 6 deletions.
5 changes: 5 additions & 0 deletions .changeset/plenty-radios-turn.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': patch
---

Reset stack traces to avoid double-fix
9 changes: 8 additions & 1 deletion packages/kit/src/runtime/server/page/render.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,9 @@ export async function render_response({
/** @type {import('types').NormalizedLoadOutputCache | undefined} */
let cache;

if (error) {
const stack = error?.stack;

if (options.dev && error) {
error.stack = options.get_stack(error);
}

Expand Down Expand Up @@ -315,6 +317,11 @@ export async function render_response({
}
}

if (options.dev && error) {
// reset stack, otherwise it may be 'fixed' a second time
error.stack = stack;
}

return new Response(html, {
status,
headers
Expand Down
7 changes: 2 additions & 5 deletions packages/kit/src/vite/dev/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -302,9 +302,7 @@ export async function dev(vite, vite_config, svelte_config) {
{
csp: svelte_config.kit.csp,
dev: true,
get_stack: (error) => {
return fix_stack_trace(error);
},
get_stack: (error) => fix_stack_trace(error),
handle_error: (error, event) => {
hooks.handleError({
error: new Proxy(error, {
Expand Down Expand Up @@ -375,9 +373,8 @@ export async function dev(vite, vite_config, svelte_config) {
}
} catch (e) {
const error = coalesce_to_error(e);
vite.ssrFixStacktrace(error);
res.statusCode = 500;
res.end(error.stack);
res.end(fix_stack_trace(error));
}
});
};
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
const bad = foo().toUpperCase();
export default bad;

// @ts-expect-error
/** @returns {string} */
function foo() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<script>
import bad from './_bad.js';
</script>

<h1>{bad}</h1>
13 changes: 13 additions & 0 deletions packages/kit/test/apps/basics/test/server.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,19 @@ test.describe('Errors', () => {
expect(/** @type {Response} */ (response).status()).toBe(400);
}
});

test('stack traces are not fixed twice', async ({ page }) => {
await page.goto('/errors/stack-trace');
expect(await page.textContent('#message')).toBe(
'This is your custom error page saying: "Cannot read properties of undefined (reading \'toUpperCase\')"'
);

// check the stack wasn't mutated
await page.goto('/errors/stack-trace');
expect(await page.textContent('#message')).toBe(
'This is your custom error page saying: "Cannot read properties of undefined (reading \'toUpperCase\')"'
);
});
});

test.describe('Load', () => {
Expand Down

0 comments on commit 68a30cc

Please sign in to comment.