From c9c152e6c239707c0d86a8f44c4fbcaf564b694e Mon Sep 17 00:00:00 2001 From: Christopher Carson Date: Tue, 28 Jun 2022 18:00:51 -0500 Subject: [PATCH 01/29] fix and almost working tests --- packages/kit/src/runtime/client/client.js | 6 +++- packages/kit/src/runtime/server/index.js | 25 +++++++++++++ .../errors/endpoint-throws/fetch-throws.js | 3 ++ .../endpoint-throws/fetch-throws.svelte | 17 +++++++++ .../errors/endpoint-throws/index.svelte | 1 + .../page-endpoint-get-throws.js | 3 ++ .../page-endpoint-get-throws.svelte | 3 ++ .../page-endpoint-http-post-throws.js | 3 ++ .../page-endpoint-http-post-throws.svelte | 3 ++ packages/kit/test/apps/basics/test/test.js | 36 +++++++++++++++++++ 10 files changed, 99 insertions(+), 1 deletion(-) create mode 100644 packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/fetch-throws.js create mode 100644 packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/fetch-throws.svelte create mode 100644 packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/index.svelte create mode 100644 packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/page-endpoint-get-throws.js create mode 100644 packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/page-endpoint-get-throws.svelte create mode 100644 packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/page-endpoint-http-post-throws.js create mode 100644 packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/page-endpoint-http-post-throws.svelte diff --git a/packages/kit/src/runtime/client/client.js b/packages/kit/src/runtime/client/client.js index b844f637f464..97746a582116 100644 --- a/packages/kit/src/runtime/client/client.js +++ b/packages/kit/src/runtime/client/client.js @@ -719,7 +719,11 @@ export function create_client({ target, session, base, trailing_slash }) { props = res.status === 204 ? {} : await res.json(); } else { status = res.status; - error = new Error('Failed to load data'); + try { + error = await res.json(); + } catch (e) { + error = new Error('Failed to load data'); + } } } diff --git a/packages/kit/src/runtime/server/index.js b/packages/kit/src/runtime/server/index.js index c10bbacdd937..3f14a6ae3109 100644 --- a/packages/kit/src/runtime/server/index.js +++ b/packages/kit/src/runtime/server/index.js @@ -309,6 +309,31 @@ export async function respond(request, options, state) { const error = coalesce_to_error(e); options.handle_error(error, event); + // start change + /** + * Note: error is guaranteed to be an Error at this point, because coalesce_to_error + * + * We need to check whether the request is a client side fetch. There are two cases: + * - It's a navigation, in which case the const is_data_request, + * defined further up in this function, is true + * - It's some other client side fetch initiated from userland (as opposed to SvelteKit) code. + * In this case we check whether the Accept header starts with application/json. + */ + if ( + is_data_request || + (typeof event.request.headers.get('accept') === 'string' && + event.request.headers.get('accept')?.startsWith('application/json')) + ) { + /** + * Return the error as JSON. For simplicity's sake, boil it down to the + * message. + */ + return new Response(JSON.stringify({ message: error.message }), { + status: 500, + headers: { 'content-type': 'application/json; charset=utf-8' } + }); + } + // end change try { const $session = await options.hooks.getSession(event); diff --git a/packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/fetch-throws.js b/packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/fetch-throws.js new file mode 100644 index 000000000000..af490a514cd8 --- /dev/null +++ b/packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/fetch-throws.js @@ -0,0 +1,3 @@ +export const post = () => { + throw new Error('Some userland error.'); +}; diff --git a/packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/fetch-throws.svelte b/packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/fetch-throws.svelte new file mode 100644 index 000000000000..83520e08cc58 --- /dev/null +++ b/packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/fetch-throws.svelte @@ -0,0 +1,17 @@ + + +
+ {error} +
diff --git a/packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/index.svelte b/packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/index.svelte new file mode 100644 index 000000000000..11f1f89c2333 --- /dev/null +++ b/packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/index.svelte @@ -0,0 +1 @@ +Page endpoint get throws \ No newline at end of file diff --git a/packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/page-endpoint-get-throws.js b/packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/page-endpoint-get-throws.js new file mode 100644 index 000000000000..a8f318161969 --- /dev/null +++ b/packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/page-endpoint-get-throws.js @@ -0,0 +1,3 @@ +export const get = () => { + throw new Error('Some userland error.'); +}; diff --git a/packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/page-endpoint-get-throws.svelte b/packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/page-endpoint-get-throws.svelte new file mode 100644 index 000000000000..887e173a773a --- /dev/null +++ b/packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/page-endpoint-get-throws.svelte @@ -0,0 +1,3 @@ +

+ Should not show up. +

\ No newline at end of file diff --git a/packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/page-endpoint-http-post-throws.js b/packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/page-endpoint-http-post-throws.js new file mode 100644 index 000000000000..af490a514cd8 --- /dev/null +++ b/packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/page-endpoint-http-post-throws.js @@ -0,0 +1,3 @@ +export const post = () => { + throw new Error('Some userland error.'); +}; diff --git a/packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/page-endpoint-http-post-throws.svelte b/packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/page-endpoint-http-post-throws.svelte new file mode 100644 index 000000000000..92405a067d50 --- /dev/null +++ b/packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/page-endpoint-http-post-throws.svelte @@ -0,0 +1,3 @@ +
+ +
\ No newline at end of file diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js index a726480ac740..202baf013fdd 100644 --- a/packages/kit/test/apps/basics/test/test.js +++ b/packages/kit/test/apps/basics/test/test.js @@ -1132,6 +1132,42 @@ test.describe.parallel('Errors', () => { '500: Cannot prerender pages that have endpoints with mutative methods' ); }); + test('Case: "page endpoint" GET method throws', async ({browser}) => { + // The case where we're navigating to a page with the client side router. + // The GET "page endpoint" throws a userland error. + // It should show the __error template with our message. + const page = await browser.newPage(); + await page.goto('/errors/endpoint-throws'); + await page.click('a'); + await page.waitForSelector('#message'); + expect(await page.textContent('#message')).toContain( + 'Some userland error.' + ); + await page.goto('/errors/endpoint-throws'); + await page.click('a'); + await page.waitForSelector('#message'); + expect(await page.textContent('#message')).toContain( + 'Some userland error.' + ); + await page.close(); + }); + test('Case: submitting a form w/o fetch', async ({page}) => { + // The case where we're submitting a POST request via a form. + // It should show the __error template with our message. + await page.goto('/errors/endpoint-throws/page-endpoint-http-post-throws'); + await Promise.all([page.waitForNavigation(), page.click('#submit')]); + expect(await page.textContent('#message')).toContain( + 'Some userland error.' + ); + }); + test('Case: fetch POST with Accept: application/json', async ({page}) => { + // The case where we're fetching a POST with Accept: application/json from javascript. + // It should return JSON with {error: string}, not the __error template HTML. + await page.goto('/errors/endpoint-throws/fetch-throws'); + expect(await page.textContent('#error')).toBe( + 'Some userland error.' + ); + }); }); test.describe.parallel('ETags', () => { From 48ddd8c709c35e5998655756938f538468ec3de3 Mon Sep 17 00:00:00 2001 From: Christopher Carson Date: Tue, 28 Jun 2022 18:05:36 -0500 Subject: [PATCH 02/29] todo note in failing test --- packages/kit/test/apps/basics/test/test.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js index 202baf013fdd..bd7b8579f3f5 100644 --- a/packages/kit/test/apps/basics/test/test.js +++ b/packages/kit/test/apps/basics/test/test.js @@ -1160,9 +1160,11 @@ test.describe.parallel('Errors', () => { 'Some userland error.' ); }); - test('Case: fetch POST with Accept: application/json', async ({page}) => { + test('Case: client side fetch POST with Accept: application/json', async ({page}) => { // The case where we're fetching a POST with Accept: application/json from javascript. // It should return JSON with {error: string}, not the __error template HTML. + // @TODO Note this test fails for the config [dev-js] (which I think disables javascript.) Figure out how to fix. + await page.goto('/errors/endpoint-throws/fetch-throws'); expect(await page.textContent('#error')).toBe( 'Some userland error.' From 5ecfc932c6ee2674bfc805c9ad1ac9407a3067a3 Mon Sep 17 00:00:00 2001 From: Christopher Carson Date: Tue, 28 Jun 2022 18:25:09 -0500 Subject: [PATCH 03/29] tests working --- packages/kit/test/apps/basics/test/test.js | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js index bd7b8579f3f5..31b45dce71b6 100644 --- a/packages/kit/test/apps/basics/test/test.js +++ b/packages/kit/test/apps/basics/test/test.js @@ -1132,7 +1132,7 @@ test.describe.parallel('Errors', () => { '500: Cannot prerender pages that have endpoints with mutative methods' ); }); - test('Case: "page endpoint" GET method throws', async ({browser}) => { + test('Case: a "page endpoint" GET method throws', async ({browser}) => { // The case where we're navigating to a page with the client side router. // The GET "page endpoint" throws a userland error. // It should show the __error template with our message. @@ -1143,6 +1143,7 @@ test.describe.parallel('Errors', () => { expect(await page.textContent('#message')).toContain( 'Some userland error.' ); + // do it all over again just to make sure... await page.goto('/errors/endpoint-throws'); await page.click('a'); await page.waitForSelector('#message'); @@ -1160,15 +1161,17 @@ test.describe.parallel('Errors', () => { 'Some userland error.' ); }); - test('Case: client side fetch POST with Accept: application/json', async ({page}) => { + test('Case: client side fetch POST with Accept: application/json', async ({page, javaScriptEnabled}) => { // The case where we're fetching a POST with Accept: application/json from javascript. // It should return JSON with {error: string}, not the __error template HTML. - // @TODO Note this test fails for the config [dev-js] (which I think disables javascript.) Figure out how to fix. - await page.goto('/errors/endpoint-throws/fetch-throws'); - expect(await page.textContent('#error')).toBe( - 'Some userland error.' - ); + // this case is only relevant if javascript is enabled... + if (javaScriptEnabled) { + expect(await page.textContent('#error')).toBe( + 'Some userland error.' + ); + } + }); }); From bfa3dec127bffbeb0a6c00afa9b5f046004e28dd Mon Sep 17 00:00:00 2001 From: Christopher Carson Date: Tue, 28 Jun 2022 18:29:56 -0500 Subject: [PATCH 04/29] formatting --- packages/kit/src/runtime/server/index.js | 48 +++++++++++----------- packages/kit/test/apps/basics/test/test.js | 26 +++++------- 2 files changed, 34 insertions(+), 40 deletions(-) diff --git a/packages/kit/src/runtime/server/index.js b/packages/kit/src/runtime/server/index.js index 3f14a6ae3109..01ef9eb029d8 100644 --- a/packages/kit/src/runtime/server/index.js +++ b/packages/kit/src/runtime/server/index.js @@ -310,30 +310,30 @@ export async function respond(request, options, state) { options.handle_error(error, event); // start change - /** - * Note: error is guaranteed to be an Error at this point, because coalesce_to_error - * - * We need to check whether the request is a client side fetch. There are two cases: - * - It's a navigation, in which case the const is_data_request, - * defined further up in this function, is true - * - It's some other client side fetch initiated from userland (as opposed to SvelteKit) code. - * In this case we check whether the Accept header starts with application/json. - */ - if ( - is_data_request || - (typeof event.request.headers.get('accept') === 'string' && - event.request.headers.get('accept')?.startsWith('application/json')) - ) { - /** - * Return the error as JSON. For simplicity's sake, boil it down to the - * message. - */ - return new Response(JSON.stringify({ message: error.message }), { - status: 500, - headers: { 'content-type': 'application/json; charset=utf-8' } - }); - } - // end change + /** + * Note: error is guaranteed to be an Error at this point, because coalesce_to_error + * + * We need to check whether the request is a client side fetch. There are two cases: + * - It's a navigation, in which case the const is_data_request, + * defined further up in this function, is true + * - It's some other client side fetch initiated from userland (as opposed to SvelteKit) code. + * In this case we check whether the Accept header starts with application/json. + */ + if ( + is_data_request || + (typeof event.request.headers.get('accept') === 'string' && + event.request.headers.get('accept')?.startsWith('application/json')) + ) { + /** + * Return the error as JSON. For simplicity's sake, boil it down to the + * message. + */ + return new Response(JSON.stringify({ message: error.message }), { + status: 500, + headers: { 'content-type': 'application/json; charset=utf-8' } + }); + } + // end change try { const $session = await options.hooks.getSession(event); diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js index 31b45dce71b6..fa04074f7d5c 100644 --- a/packages/kit/test/apps/basics/test/test.js +++ b/packages/kit/test/apps/basics/test/test.js @@ -1132,7 +1132,7 @@ test.describe.parallel('Errors', () => { '500: Cannot prerender pages that have endpoints with mutative methods' ); }); - test('Case: a "page endpoint" GET method throws', async ({browser}) => { + test('Case: a "page endpoint" GET method throws', async ({ browser }) => { // The case where we're navigating to a page with the client side router. // The GET "page endpoint" throws a userland error. // It should show the __error template with our message. @@ -1140,38 +1140,32 @@ test.describe.parallel('Errors', () => { await page.goto('/errors/endpoint-throws'); await page.click('a'); await page.waitForSelector('#message'); - expect(await page.textContent('#message')).toContain( - 'Some userland error.' - ); + expect(await page.textContent('#message')).toContain('Some userland error.'); // do it all over again just to make sure... await page.goto('/errors/endpoint-throws'); await page.click('a'); await page.waitForSelector('#message'); - expect(await page.textContent('#message')).toContain( - 'Some userland error.' - ); + expect(await page.textContent('#message')).toContain('Some userland error.'); await page.close(); }); - test('Case: submitting a form w/o fetch', async ({page}) => { + test('Case: submitting a form w/o fetch', async ({ page }) => { // The case where we're submitting a POST request via a form. // It should show the __error template with our message. await page.goto('/errors/endpoint-throws/page-endpoint-http-post-throws'); await Promise.all([page.waitForNavigation(), page.click('#submit')]); - expect(await page.textContent('#message')).toContain( - 'Some userland error.' - ); + expect(await page.textContent('#message')).toContain('Some userland error.'); }); - test('Case: client side fetch POST with Accept: application/json', async ({page, javaScriptEnabled}) => { + test('Case: client side fetch POST with Accept: application/json', async ({ + page, + javaScriptEnabled + }) => { // The case where we're fetching a POST with Accept: application/json from javascript. // It should return JSON with {error: string}, not the __error template HTML. await page.goto('/errors/endpoint-throws/fetch-throws'); // this case is only relevant if javascript is enabled... if (javaScriptEnabled) { - expect(await page.textContent('#error')).toBe( - 'Some userland error.' - ); + expect(await page.textContent('#error')).toBe('Some userland error.'); } - }); }); From f72ddd19a669f9cbf2ff3b530af37cb4ebf3ddf9 Mon Sep 17 00:00:00 2001 From: Christopher Carson Date: Wed, 29 Jun 2022 09:48:56 -0500 Subject: [PATCH 05/29] added changeset --- .changeset/swift-dots-cry.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .changeset/swift-dots-cry.md diff --git a/.changeset/swift-dots-cry.md b/.changeset/swift-dots-cry.md new file mode 100644 index 000000000000..ec4ccbe063d1 --- /dev/null +++ b/.changeset/swift-dots-cry.md @@ -0,0 +1,7 @@ +--- +'@sveltejs/kit': patch +'test-basics': patch +--- + +Fixes the error handling issue where an error thrown from an endpoint is returned as HTML (\_\_error.svelte) +regardless of whether or not the request was a client side fetch. From 2e41c78d8013df56a1ace44453710b6b48220a38 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Wed, 6 Jul 2022 15:56:13 -0400 Subject: [PATCH 06/29] make tests more consistent with the rest of the codebase --- packages/kit/test/apps/basics/test/test.js | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js index cc847ea9caec..8a577748f880 100644 --- a/packages/kit/test/apps/basics/test/test.js +++ b/packages/kit/test/apps/basics/test/test.js @@ -1133,30 +1133,22 @@ test.describe('Errors', () => { ); }); - test('Case: a "page endpoint" GET method throws', async ({ browser }) => { - // The case where we're navigating to a page with the client side router. - // The GET "page endpoint" throws a userland error. - // It should show the __error template with our message. - const page = await browser.newPage(); + test('page endpoint GET method throws', async ({ page }) => { await page.goto('/errors/endpoint-throws'); await page.click('a'); await page.waitForSelector('#message'); expect(await page.textContent('#message')).toContain('Some userland error.'); - // do it all over again just to make sure... - await page.goto('/errors/endpoint-throws'); - await page.click('a'); - await page.waitForSelector('#message'); - expect(await page.textContent('#message')).toContain('Some userland error.'); - await page.close(); }); - test('Case: submitting a form w/o fetch', async ({ page }) => { + + test('submitting a form w/o fetch', async ({ page }) => { // The case where we're submitting a POST request via a form. // It should show the __error template with our message. await page.goto('/errors/endpoint-throws/page-endpoint-http-post-throws'); await Promise.all([page.waitForNavigation(), page.click('#submit')]); expect(await page.textContent('#message')).toContain('Some userland error.'); }); - test('Case: client side fetch POST with Accept: application/json', async ({ + + test('client side fetch POST with Accept: application/json', async ({ page, javaScriptEnabled }) => { From 9d0fbcf7eb3d17e9c37be1f69c277b3043807656 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Wed, 6 Jul 2022 16:12:39 -0400 Subject: [PATCH 07/29] simplify tests --- .../errors/endpoint-throws/fetch-throws.js | 3 -- .../endpoint-throws/fetch-throws.svelte | 17 --------- .../src/routes/errors/endpoint-throws/get.js | 3 ++ .../routes/errors/endpoint-throws/get.svelte | 1 + .../errors/endpoint-throws/index.svelte | 6 +++- .../page-endpoint-get-throws.js | 3 -- .../page-endpoint-get-throws.svelte | 3 -- .../page-endpoint-http-post-throws.js | 3 -- .../page-endpoint-http-post-throws.svelte | 3 -- .../src/routes/errors/endpoint-throws/post.js | 3 ++ .../routes/errors/endpoint-throws/post.svelte | 3 ++ packages/kit/test/apps/basics/test/test.js | 35 +++++++++---------- 12 files changed, 32 insertions(+), 51 deletions(-) delete mode 100644 packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/fetch-throws.js delete mode 100644 packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/fetch-throws.svelte create mode 100644 packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/get.js create mode 100644 packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/get.svelte delete mode 100644 packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/page-endpoint-get-throws.js delete mode 100644 packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/page-endpoint-get-throws.svelte delete mode 100644 packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/page-endpoint-http-post-throws.js delete mode 100644 packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/page-endpoint-http-post-throws.svelte create mode 100644 packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/post.js create mode 100644 packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/post.svelte diff --git a/packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/fetch-throws.js b/packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/fetch-throws.js deleted file mode 100644 index af490a514cd8..000000000000 --- a/packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/fetch-throws.js +++ /dev/null @@ -1,3 +0,0 @@ -export const post = () => { - throw new Error('Some userland error.'); -}; diff --git a/packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/fetch-throws.svelte b/packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/fetch-throws.svelte deleted file mode 100644 index 83520e08cc58..000000000000 --- a/packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/fetch-throws.svelte +++ /dev/null @@ -1,17 +0,0 @@ - - -
- {error} -
diff --git a/packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/get.js b/packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/get.js new file mode 100644 index 000000000000..a3f81411957c --- /dev/null +++ b/packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/get.js @@ -0,0 +1,3 @@ +export const get = () => { + throw new Error('oops'); +}; diff --git a/packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/get.svelte b/packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/get.svelte new file mode 100644 index 000000000000..d5e02b47db24 --- /dev/null +++ b/packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/get.svelte @@ -0,0 +1 @@ +

Should not show up.

diff --git a/packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/index.svelte b/packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/index.svelte index 11f1f89c2333..390fba3cb82f 100644 --- a/packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/index.svelte +++ b/packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/index.svelte @@ -1 +1,5 @@ -Page endpoint get throws \ No newline at end of file +GET + +
+ +
diff --git a/packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/page-endpoint-get-throws.js b/packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/page-endpoint-get-throws.js deleted file mode 100644 index a8f318161969..000000000000 --- a/packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/page-endpoint-get-throws.js +++ /dev/null @@ -1,3 +0,0 @@ -export const get = () => { - throw new Error('Some userland error.'); -}; diff --git a/packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/page-endpoint-get-throws.svelte b/packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/page-endpoint-get-throws.svelte deleted file mode 100644 index 887e173a773a..000000000000 --- a/packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/page-endpoint-get-throws.svelte +++ /dev/null @@ -1,3 +0,0 @@ -

- Should not show up. -

\ No newline at end of file diff --git a/packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/page-endpoint-http-post-throws.js b/packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/page-endpoint-http-post-throws.js deleted file mode 100644 index af490a514cd8..000000000000 --- a/packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/page-endpoint-http-post-throws.js +++ /dev/null @@ -1,3 +0,0 @@ -export const post = () => { - throw new Error('Some userland error.'); -}; diff --git a/packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/page-endpoint-http-post-throws.svelte b/packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/page-endpoint-http-post-throws.svelte deleted file mode 100644 index 92405a067d50..000000000000 --- a/packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/page-endpoint-http-post-throws.svelte +++ /dev/null @@ -1,3 +0,0 @@ -
- -
\ No newline at end of file diff --git a/packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/post.js b/packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/post.js new file mode 100644 index 000000000000..4cdd25946513 --- /dev/null +++ b/packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/post.js @@ -0,0 +1,3 @@ +export const post = () => { + throw new Error('oops'); +}; diff --git a/packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/post.svelte b/packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/post.svelte new file mode 100644 index 000000000000..ad00bb6bf699 --- /dev/null +++ b/packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/post.svelte @@ -0,0 +1,3 @@ +
+ +
diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js index 8a577748f880..c79aba44facc 100644 --- a/packages/kit/test/apps/basics/test/test.js +++ b/packages/kit/test/apps/basics/test/test.js @@ -1133,32 +1133,31 @@ test.describe('Errors', () => { ); }); - test('page endpoint GET method throws', async ({ page }) => { + test('page endpoint GET error message is preserved', async ({ page }) => { await page.goto('/errors/endpoint-throws'); - await page.click('a'); + await page.click('#get'); await page.waitForSelector('#message'); - expect(await page.textContent('#message')).toContain('Some userland error.'); + expect(await page.textContent('#message')).toContain('oops'); }); - test('submitting a form w/o fetch', async ({ page }) => { + test('page endpoint POST error message is preserved', async ({ page }) => { // The case where we're submitting a POST request via a form. // It should show the __error template with our message. - await page.goto('/errors/endpoint-throws/page-endpoint-http-post-throws'); - await Promise.all([page.waitForNavigation(), page.click('#submit')]); - expect(await page.textContent('#message')).toContain('Some userland error.'); + await page.goto('/errors/endpoint-throws'); + await Promise.all([page.waitForNavigation(), page.click('#post')]); + expect(await page.textContent('#message')).toContain('oops'); }); - test('client side fetch POST with Accept: application/json', async ({ - page, - javaScriptEnabled - }) => { - // The case where we're fetching a POST with Accept: application/json from javascript. - // It should return JSON with {error: string}, not the __error template HTML. - await page.goto('/errors/endpoint-throws/fetch-throws'); - // this case is only relevant if javascript is enabled... - if (javaScriptEnabled) { - expect(await page.textContent('#error')).toBe('Some userland error.'); - } + test('page endpoint error respects `accept: application/json`', async ({ request }) => { + const response = await request.get('/errors/endpoint-throws/get', { + headers: { + accept: 'application/json' + } + }); + + expect(await response.json()).toEqual({ + message: 'oops' + }); }); test('returns 400 when accessing a malformed URI', async ({ page, javaScriptEnabled }) => { From 5216d42c0525efa8b441ff40d09fb099c43660d0 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Wed, 6 Jul 2022 16:26:21 -0400 Subject: [PATCH 08/29] include stack trace, tweak tests a bit --- packages/kit/src/runtime/server/index.js | 42 +++++++-------- packages/kit/src/runtime/server/page/index.js | 53 +------------------ packages/kit/src/utils/http.js | 52 ++++++++++++++++++ packages/kit/test/apps/basics/test/test.js | 22 ++++++-- 4 files changed, 90 insertions(+), 79 deletions(-) diff --git a/packages/kit/src/runtime/server/index.js b/packages/kit/src/runtime/server/index.js index 947c65fef61f..f560783a2eed 100644 --- a/packages/kit/src/runtime/server/index.js +++ b/packages/kit/src/runtime/server/index.js @@ -6,6 +6,7 @@ import { coalesce_to_error } from '../../utils/error.js'; import { decode_params } from './utils.js'; import { normalize_path } from '../../utils/url.js'; import { exec } from '../../utils/routing.js'; +import { negotiate } from '../../utils/http.js'; const DATA_SUFFIX = '/__data.json'; @@ -314,31 +315,24 @@ export async function respond(request, options, state) { const error = coalesce_to_error(e); options.handle_error(error, event); - // start change - /** - * Note: error is guaranteed to be an Error at this point, because coalesce_to_error - * - * We need to check whether the request is a client side fetch. There are two cases: - * - It's a navigation, in which case the const is_data_request, - * defined further up in this function, is true - * - It's some other client side fetch initiated from userland (as opposed to SvelteKit) code. - * In this case we check whether the Accept header starts with application/json. - */ - if ( - is_data_request || - (typeof event.request.headers.get('accept') === 'string' && - event.request.headers.get('accept')?.startsWith('application/json')) - ) { - /** - * Return the error as JSON. For simplicity's sake, boil it down to the - * message. - */ - return new Response(JSON.stringify({ message: error.message }), { - status: 500, - headers: { 'content-type': 'application/json; charset=utf-8' } - }); + + const type = negotiate(event.request.headers.get('accept') || 'text/html', [ + 'text/html', + 'application/json' + ]); + + if (is_data_request || type === 'application/json') { + return new Response( + JSON.stringify({ + message: error.message, + stack: options.get_stack(error) + }), + { + status: 500, + headers: { 'content-type': 'application/json; charset=utf-8' } + } + ); } - // end change try { const $session = await options.hooks.getSession(event); diff --git a/packages/kit/src/runtime/server/page/index.js b/packages/kit/src/runtime/server/page/index.js index 1900b5ef0f99..df62270b1724 100644 --- a/packages/kit/src/runtime/server/page/index.js +++ b/packages/kit/src/runtime/server/page/index.js @@ -1,3 +1,4 @@ +import { negotiate } from '../../../utils/http.js'; import { render_endpoint } from '../endpoint.js'; import { respond } from './respond.js'; @@ -39,55 +40,3 @@ export async function render_page(event, route, options, state, resolve_opts) { route }); } - -/** - * @param {string} accept - * @param {string[]} types - */ -function negotiate(accept, types) { - const parts = accept - .split(',') - .map((str, i) => { - const match = /([^/]+)\/([^;]+)(?:;q=([0-9.]+))?/.exec(str); - if (match) { - const [, type, subtype, q = '1'] = match; - return { type, subtype, q: +q, i }; - } - - throw new Error(`Invalid Accept header: ${accept}`); - }) - .sort((a, b) => { - if (a.q !== b.q) { - return b.q - a.q; - } - - if ((a.subtype === '*') !== (b.subtype === '*')) { - return a.subtype === '*' ? 1 : -1; - } - - if ((a.type === '*') !== (b.type === '*')) { - return a.type === '*' ? 1 : -1; - } - - return a.i - b.i; - }); - - let accepted; - let min_priority = Infinity; - - for (const mimetype of types) { - const [type, subtype] = mimetype.split('/'); - const priority = parts.findIndex( - (part) => - (part.type === type || part.type === '*') && - (part.subtype === subtype || part.subtype === '*') - ); - - if (priority !== -1 && priority < min_priority) { - accepted = mimetype; - min_priority = priority; - } - } - - return accepted; -} diff --git a/packages/kit/src/utils/http.js b/packages/kit/src/utils/http.js index 8329de9fa392..8ddd7b07a1eb 100644 --- a/packages/kit/src/utils/http.js +++ b/packages/kit/src/utils/http.js @@ -19,3 +19,55 @@ export function to_headers(object) { return headers; } + +/** + * @param {string} accept + * @param {string[]} types + */ +export function negotiate(accept, types) { + const parts = accept + .split(',') + .map((str, i) => { + const match = /([^/]+)\/([^;]+)(?:;q=([0-9.]+))?/.exec(str); + if (match) { + const [, type, subtype, q = '1'] = match; + return { type, subtype, q: +q, i }; + } + + throw new Error(`Invalid Accept header: ${accept}`); + }) + .sort((a, b) => { + if (a.q !== b.q) { + return b.q - a.q; + } + + if ((a.subtype === '*') !== (b.subtype === '*')) { + return a.subtype === '*' ? 1 : -1; + } + + if ((a.type === '*') !== (b.type === '*')) { + return a.type === '*' ? 1 : -1; + } + + return a.i - b.i; + }); + + let accepted; + let min_priority = Infinity; + + for (const mimetype of types) { + const [type, subtype] = mimetype.split('/'); + const priority = parts.findIndex( + (part) => + (part.type === type || part.type === '*') && + (part.subtype === subtype || part.subtype === '*') + ); + + if (priority !== -1 && priority < min_priority) { + accepted = mimetype; + min_priority = priority; + } + } + + return accepted; +} diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js index c79aba44facc..c0d6a212a362 100644 --- a/packages/kit/test/apps/basics/test/test.js +++ b/packages/kit/test/apps/basics/test/test.js @@ -1138,6 +1138,11 @@ test.describe('Errors', () => { await page.click('#get'); await page.waitForSelector('#message'); expect(await page.textContent('#message')).toContain('oops'); + + if (process.env.DEV) { + const lines = (await page.textContent('pre')).split('\n'); + expect(lines[1]).toContain('src/routes/errors/endpoint-throws/get.js:2:8'); + } }); test('page endpoint POST error message is preserved', async ({ page }) => { @@ -1146,6 +1151,11 @@ test.describe('Errors', () => { await page.goto('/errors/endpoint-throws'); await Promise.all([page.waitForNavigation(), page.click('#post')]); expect(await page.textContent('#message')).toContain('oops'); + + if (process.env.DEV) { + const lines = (await page.textContent('pre')).split('\n'); + expect(lines[1]).toContain('src/routes/errors/endpoint-throws/post.js:2:8'); + } }); test('page endpoint error respects `accept: application/json`', async ({ request }) => { @@ -1155,9 +1165,15 @@ test.describe('Errors', () => { } }); - expect(await response.json()).toEqual({ - message: 'oops' - }); + const { message, stack } = await response.json(); + + expect(message).toBe('oops'); + + if (process.env.DEV) { + expect(stack.split('\n').length).toBeGreaterThan(1); + } else { + expect(stack.split('\n').length).toBe(1); + } }); test('returns 400 when accessing a malformed URI', async ({ page, javaScriptEnabled }) => { From f7f9fc254cbdf198ba5723483c590af289c4f191 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Wed, 6 Jul 2022 16:34:59 -0400 Subject: [PATCH 09/29] update changeset --- .changeset/swift-dots-cry.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.changeset/swift-dots-cry.md b/.changeset/swift-dots-cry.md index ec4ccbe063d1..428204437a82 100644 --- a/.changeset/swift-dots-cry.md +++ b/.changeset/swift-dots-cry.md @@ -3,5 +3,4 @@ 'test-basics': patch --- -Fixes the error handling issue where an error thrown from an endpoint is returned as HTML (\_\_error.svelte) -regardless of whether or not the request was a client side fetch. +Returns errors from page endpoints as JSON where appropriate From 1852bbeb24c25bd603edf2c8ae6108d8fc87d8fe Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Wed, 6 Jul 2022 16:50:29 -0400 Subject: [PATCH 10/29] oh do fuck off windows --- packages/kit/test/apps/basics/test/test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js index c0d6a212a362..71d87a426a0b 100644 --- a/packages/kit/test/apps/basics/test/test.js +++ b/packages/kit/test/apps/basics/test/test.js @@ -1141,7 +1141,7 @@ test.describe('Errors', () => { if (process.env.DEV) { const lines = (await page.textContent('pre')).split('\n'); - expect(lines[1]).toContain('src/routes/errors/endpoint-throws/get.js:2:8'); + expect(lines[1]).toContain('get.js:2:8'); } }); @@ -1154,7 +1154,7 @@ test.describe('Errors', () => { if (process.env.DEV) { const lines = (await page.textContent('pre')).split('\n'); - expect(lines[1]).toContain('src/routes/errors/endpoint-throws/post.js:2:8'); + expect(lines[1]).toContain('post.js:2:8'); } }); From 618e16e1f0d6c5e5dc9f1a1aea05b00396a82190 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Wed, 6 Jul 2022 17:51:27 -0400 Subject: [PATCH 11/29] shuffle things around a bit, ensure handleError is called --- .../errors/endpoint-throws/index.svelte | 5 ---- .../get.js => page-endpoint/get-implicit.js} | 0 .../get-implicit.svelte} | 0 .../routes/errors/page-endpoint/index.svelte | 5 ++++ .../post-implicit.js} | 0 .../post-implicit.svelte} | 0 packages/kit/test/apps/basics/test/test.js | 26 ++++++++++++------- 7 files changed, 21 insertions(+), 15 deletions(-) delete mode 100644 packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/index.svelte rename packages/kit/test/apps/basics/src/routes/errors/{endpoint-throws/get.js => page-endpoint/get-implicit.js} (100%) rename packages/kit/test/apps/basics/src/routes/errors/{endpoint-throws/get.svelte => page-endpoint/get-implicit.svelte} (100%) create mode 100644 packages/kit/test/apps/basics/src/routes/errors/page-endpoint/index.svelte rename packages/kit/test/apps/basics/src/routes/errors/{endpoint-throws/post.js => page-endpoint/post-implicit.js} (100%) rename packages/kit/test/apps/basics/src/routes/errors/{endpoint-throws/post.svelte => page-endpoint/post-implicit.svelte} (100%) diff --git a/packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/index.svelte b/packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/index.svelte deleted file mode 100644 index 390fba3cb82f..000000000000 --- a/packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/index.svelte +++ /dev/null @@ -1,5 +0,0 @@ -GET - -
- -
diff --git a/packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/get.js b/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/get-implicit.js similarity index 100% rename from packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/get.js rename to packages/kit/test/apps/basics/src/routes/errors/page-endpoint/get-implicit.js diff --git a/packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/get.svelte b/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/get-implicit.svelte similarity index 100% rename from packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/get.svelte rename to packages/kit/test/apps/basics/src/routes/errors/page-endpoint/get-implicit.svelte diff --git a/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/index.svelte b/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/index.svelte new file mode 100644 index 000000000000..4c220bc7c85c --- /dev/null +++ b/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/index.svelte @@ -0,0 +1,5 @@ +GET (implicit) + +
+ +
diff --git a/packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/post.js b/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/post-implicit.js similarity index 100% rename from packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/post.js rename to packages/kit/test/apps/basics/src/routes/errors/page-endpoint/post-implicit.js diff --git a/packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/post.svelte b/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/post-implicit.svelte similarity index 100% rename from packages/kit/test/apps/basics/src/routes/errors/endpoint-throws/post.svelte rename to packages/kit/test/apps/basics/src/routes/errors/page-endpoint/post-implicit.svelte diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js index 71d87a426a0b..4aed569b3d32 100644 --- a/packages/kit/test/apps/basics/test/test.js +++ b/packages/kit/test/apps/basics/test/test.js @@ -1133,33 +1133,39 @@ test.describe('Errors', () => { ); }); - test('page endpoint GET error message is preserved', async ({ page }) => { - await page.goto('/errors/endpoint-throws'); - await page.click('#get'); + test('page endpoint GET thrown error message is preserved', async ({ page, read_errors }) => { + await page.goto('/errors/page-endpoint'); + await page.click('#get-implicit'); await page.waitForSelector('#message'); expect(await page.textContent('#message')).toContain('oops'); if (process.env.DEV) { const lines = (await page.textContent('pre')).split('\n'); - expect(lines[1]).toContain('get.js:2:8'); + expect(lines[1]).toContain('get-implicit.js:2:8'); } + + const error = read_errors('/errors/page-endpoint/get-implicit'); + expect(error).toContain('oops'); }); - test('page endpoint POST error message is preserved', async ({ page }) => { + test('page endpoint POST thrown error message is preserved', async ({ page, read_errors }) => { // The case where we're submitting a POST request via a form. // It should show the __error template with our message. - await page.goto('/errors/endpoint-throws'); - await Promise.all([page.waitForNavigation(), page.click('#post')]); + await page.goto('/errors/page-endpoint'); + await Promise.all([page.waitForNavigation(), page.click('#post-implicit')]); expect(await page.textContent('#message')).toContain('oops'); if (process.env.DEV) { const lines = (await page.textContent('pre')).split('\n'); - expect(lines[1]).toContain('post.js:2:8'); + expect(lines[1]).toContain('post-implicit.js:2:8'); } + + const error = read_errors('/errors/page-endpoint/post-implicit'); + expect(error).toContain('oops'); }); - test('page endpoint error respects `accept: application/json`', async ({ request }) => { - const response = await request.get('/errors/endpoint-throws/get', { + test('page endpoint thrown error respects `accept: application/json`', async ({ request }) => { + const response = await request.get('/errors/page-endpoint/get-implicit', { headers: { accept: 'application/json' } From e4f886b22cbcd88e1624aca24e954d8770550a7a Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 7 Jul 2022 07:35:53 -0400 Subject: [PATCH 12/29] add (failing) tests for explicit errors --- .../errors/page-endpoint/__error.svelte | 13 +++ .../errors/page-endpoint/get-explicit.js | 6 ++ .../errors/page-endpoint/get-explicit.svelte | 1 + .../errors/page-endpoint/get-implicit.svelte | 2 +- .../routes/errors/page-endpoint/index.svelte | 5 + .../errors/page-endpoint/post-explicit.js | 6 ++ .../errors/page-endpoint/post-explicit.svelte | 1 + .../errors/page-endpoint/post-implicit.svelte | 4 +- packages/kit/test/apps/basics/test/test.js | 101 ++++++++++++++++-- 9 files changed, 126 insertions(+), 13 deletions(-) create mode 100644 packages/kit/test/apps/basics/src/routes/errors/page-endpoint/__error.svelte create mode 100644 packages/kit/test/apps/basics/src/routes/errors/page-endpoint/get-explicit.js create mode 100644 packages/kit/test/apps/basics/src/routes/errors/page-endpoint/get-explicit.svelte create mode 100644 packages/kit/test/apps/basics/src/routes/errors/page-endpoint/post-explicit.js create mode 100644 packages/kit/test/apps/basics/src/routes/errors/page-endpoint/post-explicit.svelte diff --git a/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/__error.svelte b/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/__error.svelte new file mode 100644 index 000000000000..9dde61c6fe9c --- /dev/null +++ b/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/__error.svelte @@ -0,0 +1,13 @@ + + +
{JSON.stringify(
+		{
+			status: $page.status,
+			message: $page.error.message,
+			stack: $page.error.stack
+		},
+		null,
+		'  '
+	)}
diff --git a/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/get-explicit.js b/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/get-explicit.js new file mode 100644 index 000000000000..409a33fb5714 --- /dev/null +++ b/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/get-explicit.js @@ -0,0 +1,6 @@ +export const get = () => { + return { + status: 400, + body: new Error('oops') + }; +}; diff --git a/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/get-explicit.svelte b/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/get-explicit.svelte new file mode 100644 index 000000000000..52cd0974062b --- /dev/null +++ b/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/get-explicit.svelte @@ -0,0 +1 @@ +

if you see this something went wrong

diff --git a/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/get-implicit.svelte b/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/get-implicit.svelte index d5e02b47db24..52cd0974062b 100644 --- a/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/get-implicit.svelte +++ b/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/get-implicit.svelte @@ -1 +1 @@ -

Should not show up.

+

if you see this something went wrong

diff --git a/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/index.svelte b/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/index.svelte index 4c220bc7c85c..b2158ab412f8 100644 --- a/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/index.svelte +++ b/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/index.svelte @@ -1,5 +1,10 @@ GET (implicit) +GET (explicit)
+ +
+ +
diff --git a/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/post-explicit.js b/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/post-explicit.js new file mode 100644 index 000000000000..409a33fb5714 --- /dev/null +++ b/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/post-explicit.js @@ -0,0 +1,6 @@ +export const get = () => { + return { + status: 400, + body: new Error('oops') + }; +}; diff --git a/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/post-explicit.svelte b/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/post-explicit.svelte new file mode 100644 index 000000000000..52cd0974062b --- /dev/null +++ b/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/post-explicit.svelte @@ -0,0 +1 @@ +

if you see this something went wrong

diff --git a/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/post-implicit.svelte b/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/post-implicit.svelte index ad00bb6bf699..52cd0974062b 100644 --- a/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/post-implicit.svelte +++ b/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/post-implicit.svelte @@ -1,3 +1 @@ -
- -
+

if you see this something went wrong

diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js index 4aed569b3d32..6b02406d19c1 100644 --- a/packages/kit/test/apps/basics/test/test.js +++ b/packages/kit/test/apps/basics/test/test.js @@ -1133,14 +1133,21 @@ test.describe('Errors', () => { ); }); - test('page endpoint GET thrown error message is preserved', async ({ page, read_errors }) => { + test.only('page endpoint GET thrown error message is preserved', async ({ + page, + clicknav, + read_errors + }) => { await page.goto('/errors/page-endpoint'); - await page.click('#get-implicit'); - await page.waitForSelector('#message'); - expect(await page.textContent('#message')).toContain('oops'); + await clicknav('#get-implicit'); + const json = await page.textContent('pre'); + const { status, message, stack } = JSON.parse(json); + + expect(status).toBe(500); + expect(message).toBe('oops'); if (process.env.DEV) { - const lines = (await page.textContent('pre')).split('\n'); + const lines = stack.split('\n'); expect(lines[1]).toContain('get-implicit.js:2:8'); } @@ -1148,15 +1155,44 @@ test.describe('Errors', () => { expect(error).toContain('oops'); }); - test('page endpoint POST thrown error message is preserved', async ({ page, read_errors }) => { + test.only('page endpoint GET returned error message is preserved', async ({ + page, + clicknav, + read_errors + }) => { + await page.goto('/errors/page-endpoint'); + await clicknav('#get-explicit'); + const json = await page.textContent('pre'); + const { status, message, stack } = JSON.parse(json); + + expect(status).toBe(400); + expect(message).toBe('oops'); + + if (process.env.DEV) { + const lines = stack.split('\n'); + expect(lines[1]).toContain('get-explicit.js:4:9'); + } + + const error = read_errors('/errors/page-endpoint/get-explicit'); + expect(error).toContain('oops'); + }); + + test.only('page endpoint POST thrown error message is preserved', async ({ + page, + read_errors + }) => { // The case where we're submitting a POST request via a form. // It should show the __error template with our message. await page.goto('/errors/page-endpoint'); await Promise.all([page.waitForNavigation(), page.click('#post-implicit')]); - expect(await page.textContent('#message')).toContain('oops'); + const json = await page.textContent('pre'); + const { status, message, stack } = JSON.parse(json); + + expect(status).toBe(500); + expect(message).toBe('oops'); if (process.env.DEV) { - const lines = (await page.textContent('pre')).split('\n'); + const lines = stack.split('\n'); expect(lines[1]).toContain('post-implicit.js:2:8'); } @@ -1164,7 +1200,32 @@ test.describe('Errors', () => { expect(error).toContain('oops'); }); - test('page endpoint thrown error respects `accept: application/json`', async ({ request }) => { + test.only('page endpoint POST returned error message is preserved', async ({ + page, + read_errors + }) => { + // The case where we're submitting a POST request via a form. + // It should show the __error template with our message. + await page.goto('/errors/page-endpoint'); + await Promise.all([page.waitForNavigation(), page.click('#post-explicit')]); + const json = await page.textContent('pre'); + const { status, message, stack } = JSON.parse(json); + + expect(status).toBe(400); + expect(message).toBe('oops'); + + if (process.env.DEV) { + const lines = stack.split('\n'); + expect(lines[1]).toContain('post-explicit.js:4:9'); + } + + const error = read_errors('/errors/page-endpoint/post-explicit'); + expect(error).toContain('oops'); + }); + + test.only('page endpoint thrown error respects `accept: application/json`', async ({ + request + }) => { const response = await request.get('/errors/page-endpoint/get-implicit', { headers: { accept: 'application/json' @@ -1173,6 +1234,28 @@ test.describe('Errors', () => { const { message, stack } = await response.json(); + expect(response.status()).toBe(500); + expect(message).toBe('oops'); + + if (process.env.DEV) { + expect(stack.split('\n').length).toBeGreaterThan(1); + } else { + expect(stack.split('\n').length).toBe(1); + } + }); + + test.only('page endpoint returned error respects `accept: application/json`', async ({ + request + }) => { + const response = await request.get('/errors/page-endpoint/get-explicit', { + headers: { + accept: 'application/json' + } + }); + + const { message, stack } = await response.json(); + + expect(response.status()).toBe(400); expect(message).toBe('oops'); if (process.env.DEV) { From 30fd42f7a89fff9a5037bb848f8aef754f3b9cfc Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 7 Jul 2022 11:47:29 -0400 Subject: [PATCH 13/29] update tests --- packages/kit/src/runtime/server/endpoint.js | 19 +++++++++++++++++-- packages/kit/src/runtime/server/index.js | 4 ++-- packages/kit/src/runtime/server/page/index.js | 2 +- .../kit/src/runtime/server/page/load_node.js | 4 +++- .../errors/page-endpoint/get-explicit.js | 10 ++++------ .../errors/page-endpoint/post-explicit.js | 10 ++++------ packages/kit/test/apps/basics/test/test.js | 8 ++++---- 7 files changed, 35 insertions(+), 22 deletions(-) diff --git a/packages/kit/src/runtime/server/endpoint.js b/packages/kit/src/runtime/server/endpoint.js index 8ab8d2dd312a..d437b64e9fd4 100644 --- a/packages/kit/src/runtime/server/endpoint.js +++ b/packages/kit/src/runtime/server/endpoint.js @@ -39,9 +39,10 @@ export function is_text(content_type) { /** * @param {import('types').RequestEvent} event * @param {{ [method: string]: import('types').RequestHandler }} mod + * @param {import('types').SSROptions} options * @returns {Promise} */ -export async function render_endpoint(event, mod) { +export async function render_endpoint(event, mod, options) { const method = normalize_request_method(event); /** @type {import('types').RequestHandler} */ @@ -111,7 +112,7 @@ export async function render_endpoint(event, mod) { if (is_pojo(body) && (!type || type.startsWith('application/json'))) { headers.set('content-type', 'application/json; charset=utf-8'); - normalized_body = JSON.stringify(body); + normalized_body = body instanceof Error ? serialize_error(body, options) : JSON.stringify(body); } else { normalized_body = /** @type {import('types').StrictBody} */ (body); } @@ -134,3 +135,17 @@ export async function render_endpoint(event, mod) { } ); } + +/** + * @param {Error} error + * @param {import('types').SSROptions} options + */ +function serialize_error(error, options) { + const object = { + name: error.name, + message: error.message, + stack: options.get_stack(error) + }; + + return JSON.stringify(object); +} diff --git a/packages/kit/src/runtime/server/index.js b/packages/kit/src/runtime/server/index.js index f560783a2eed..8aa18fa3d0bf 100644 --- a/packages/kit/src/runtime/server/index.js +++ b/packages/kit/src/runtime/server/index.js @@ -210,7 +210,7 @@ export async function respond(request, options, state) { let response; if (is_data_request && route.type === 'page' && route.shadow) { - response = await render_endpoint(event, await route.shadow()); + response = await render_endpoint(event, await route.shadow(), options); // loading data for a client-side transition is a special case if (request.headers.has('x-sveltekit-load')) { @@ -232,7 +232,7 @@ export async function respond(request, options, state) { } else { response = route.type === 'endpoint' - ? await render_endpoint(event, await route.load()) + ? await render_endpoint(event, await route.load(), options) : await render_page(event, route, options, state, resolve_opts); } diff --git a/packages/kit/src/runtime/server/page/index.js b/packages/kit/src/runtime/server/page/index.js index df62270b1724..c56e97b05270 100644 --- a/packages/kit/src/runtime/server/page/index.js +++ b/packages/kit/src/runtime/server/page/index.js @@ -25,7 +25,7 @@ export async function render_page(event, route, options, state, resolve_opts) { ]); if (type === 'application/json') { - return render_endpoint(event, await route.shadow()); + return render_endpoint(event, await route.shadow(), options); } } diff --git a/packages/kit/src/runtime/server/page/load_node.js b/packages/kit/src/runtime/server/page/load_node.js index 5ce5abd4d614..4bb1bd5358ef 100644 --- a/packages/kit/src/runtime/server/page/load_node.js +++ b/packages/kit/src/runtime/server/page/load_node.js @@ -541,7 +541,9 @@ function validate_shadow_output(result) { } if (!is_pojo(body)) { - throw new Error('Body returned from endpoint request handler must be a plain object'); + throw new Error( + 'Body returned from endpoint request handler must be a plain object or an Error' + ); } return { status, headers, body }; diff --git a/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/get-explicit.js b/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/get-explicit.js index 409a33fb5714..ecd85beed882 100644 --- a/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/get-explicit.js +++ b/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/get-explicit.js @@ -1,6 +1,4 @@ -export const get = () => { - return { - status: 400, - body: new Error('oops') - }; -}; +export const get = () => ({ + status: 400, + body: new Error('oops') +}); diff --git a/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/post-explicit.js b/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/post-explicit.js index 409a33fb5714..13a1d4c54bde 100644 --- a/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/post-explicit.js +++ b/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/post-explicit.js @@ -1,6 +1,4 @@ -export const get = () => { - return { - status: 400, - body: new Error('oops') - }; -}; +export const post = () => ({ + status: 400, + body: new Error('oops') +}); diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js index 6b02406d19c1..986bfefab0ee 100644 --- a/packages/kit/test/apps/basics/test/test.js +++ b/packages/kit/test/apps/basics/test/test.js @@ -1170,11 +1170,11 @@ test.describe('Errors', () => { if (process.env.DEV) { const lines = stack.split('\n'); - expect(lines[1]).toContain('get-explicit.js:4:9'); + expect(lines[1]).toContain('get-explicit.js:3:8'); } const error = read_errors('/errors/page-endpoint/get-explicit'); - expect(error).toContain('oops'); + expect(error).toBe(undefined); }); test.only('page endpoint POST thrown error message is preserved', async ({ @@ -1216,11 +1216,11 @@ test.describe('Errors', () => { if (process.env.DEV) { const lines = stack.split('\n'); - expect(lines[1]).toContain('post-explicit.js:4:9'); + expect(lines[1]).toContain('post-explicit.js:3:8'); } const error = read_errors('/errors/page-endpoint/post-explicit'); - expect(error).toContain('oops'); + expect(error).toBe(undefined); }); test.only('page endpoint thrown error respects `accept: application/json`', async ({ From ec1cddece8770bd8829a5fb5114f790404f2bf5c Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 7 Jul 2022 11:56:23 -0400 Subject: [PATCH 14/29] render error page if body instanceof Error --- packages/kit/src/runtime/server/page/load_node.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/packages/kit/src/runtime/server/page/load_node.js b/packages/kit/src/runtime/server/page/load_node.js index 4bb1bd5358ef..590e981947c4 100644 --- a/packages/kit/src/runtime/server/page/load_node.js +++ b/packages/kit/src/runtime/server/page/load_node.js @@ -459,6 +459,18 @@ async function load_shadow_data(route, event, options, prerender) { return data; } + // explicit errors cause an error page... + if (body instanceof Error) { + if (status < 400) { + data.status = 500; + data.error = new Error('A non-error status code was returned with an error body'); + } else { + data.error = body; + } + + return data; + } + // ...but 4xx and 5xx status codes _don't_ result in the error page // rendering for non-GET requests — instead, we allow the page // to render with any validation errors etc that were returned From bb1c710b70c2b990c906a8cad061b9524d943d9f Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 7 Jul 2022 12:02:36 -0400 Subject: [PATCH 15/29] preserve errors returned from page endpoint GET handlers --- packages/kit/src/runtime/server/page/load_node.js | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/packages/kit/src/runtime/server/page/load_node.js b/packages/kit/src/runtime/server/page/load_node.js index 590e981947c4..8788aa34805d 100644 --- a/packages/kit/src/runtime/server/page/load_node.js +++ b/packages/kit/src/runtime/server/page/load_node.js @@ -493,6 +493,17 @@ async function load_shadow_data(route, event, options, prerender) { add_cookies(/** @type {string[]} */ (data.cookies), headers); data.status = status; + if (body instanceof Error) { + if (status < 400) { + data.status = 500; + data.error = new Error('A non-error status code was returned with an error body'); + } else { + data.error = body; + } + + return data; + } + if (status >= 400) { data.error = new Error('Failed to load data'); return data; From 3c8360e9970daf84581f40590193051de8937ac6 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 7 Jul 2022 12:38:17 -0400 Subject: [PATCH 16/29] serialize errors consistently --- packages/kit/src/runtime/server/endpoint.js | 16 +--------------- packages/kit/src/runtime/server/index.js | 16 +++++----------- packages/kit/src/runtime/server/utils.js | 14 ++++++++++++++ .../routes/errors/page-endpoint/__error.svelte | 1 + packages/kit/test/apps/basics/test/test.js | 18 ++++++++++++------ 5 files changed, 33 insertions(+), 32 deletions(-) diff --git a/packages/kit/src/runtime/server/endpoint.js b/packages/kit/src/runtime/server/endpoint.js index d437b64e9fd4..1ff4737f9f51 100644 --- a/packages/kit/src/runtime/server/endpoint.js +++ b/packages/kit/src/runtime/server/endpoint.js @@ -1,6 +1,6 @@ import { to_headers } from '../../utils/http.js'; import { hash } from '../hash.js'; -import { is_pojo, normalize_request_method } from './utils.js'; +import { is_pojo, normalize_request_method, serialize_error } from './utils.js'; /** @param {string} body */ function error(body) { @@ -135,17 +135,3 @@ export async function render_endpoint(event, mod, options) { } ); } - -/** - * @param {Error} error - * @param {import('types').SSROptions} options - */ -function serialize_error(error, options) { - const object = { - name: error.name, - message: error.message, - stack: options.get_stack(error) - }; - - return JSON.stringify(object); -} diff --git a/packages/kit/src/runtime/server/index.js b/packages/kit/src/runtime/server/index.js index 8aa18fa3d0bf..28ac87390d86 100644 --- a/packages/kit/src/runtime/server/index.js +++ b/packages/kit/src/runtime/server/index.js @@ -3,7 +3,7 @@ import { render_page } from './page/index.js'; import { render_response } from './page/render.js'; import { respond_with_error } from './page/respond_with_error.js'; import { coalesce_to_error } from '../../utils/error.js'; -import { decode_params } from './utils.js'; +import { decode_params, serialize_error } from './utils.js'; import { normalize_path } from '../../utils/url.js'; import { exec } from '../../utils/routing.js'; import { negotiate } from '../../utils/http.js'; @@ -322,16 +322,10 @@ export async function respond(request, options, state) { ]); if (is_data_request || type === 'application/json') { - return new Response( - JSON.stringify({ - message: error.message, - stack: options.get_stack(error) - }), - { - status: 500, - headers: { 'content-type': 'application/json; charset=utf-8' } - } - ); + return new Response(serialize_error(error, options), { + status: 500, + headers: { 'content-type': 'application/json; charset=utf-8' } + }); } try { diff --git a/packages/kit/src/runtime/server/utils.js b/packages/kit/src/runtime/server/utils.js index 69831be459c1..290f225c191d 100644 --- a/packages/kit/src/runtime/server/utils.js +++ b/packages/kit/src/runtime/server/utils.js @@ -55,3 +55,17 @@ export function normalize_request_method(event) { const method = event.request.method.toLowerCase(); return method === 'delete' ? 'del' : method; // 'delete' is a reserved word } + +/** + * @param {Error} error + * @param {import('types').SSROptions} options + */ +export function serialize_error(error, options) { + const object = { + name: error.name, + message: error.message, + stack: options.get_stack(error) + }; + + return JSON.stringify(object); +} diff --git a/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/__error.svelte b/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/__error.svelte index 9dde61c6fe9c..5b4cfb3c2ed6 100644 --- a/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/__error.svelte +++ b/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/__error.svelte @@ -5,6 +5,7 @@
{JSON.stringify(
 		{
 			status: $page.status,
+			name: $page.error.name,
 			message: $page.error.message,
 			stack: $page.error.stack
 		},
diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js
index 986bfefab0ee..18f02d94e095 100644
--- a/packages/kit/test/apps/basics/test/test.js
+++ b/packages/kit/test/apps/basics/test/test.js
@@ -1141,9 +1141,10 @@ test.describe('Errors', () => {
 		await page.goto('/errors/page-endpoint');
 		await clicknav('#get-implicit');
 		const json = await page.textContent('pre');
-		const { status, message, stack } = JSON.parse(json);
+		const { status, name, message, stack } = JSON.parse(json);
 
 		expect(status).toBe(500);
+		expect(name).toBe('Error');
 		expect(message).toBe('oops');
 
 		if (process.env.DEV) {
@@ -1163,9 +1164,10 @@ test.describe('Errors', () => {
 		await page.goto('/errors/page-endpoint');
 		await clicknav('#get-explicit');
 		const json = await page.textContent('pre');
-		const { status, message, stack } = JSON.parse(json);
+		const { status, name, message, stack } = JSON.parse(json);
 
 		expect(status).toBe(400);
+		expect(name).toBe('Error');
 		expect(message).toBe('oops');
 
 		if (process.env.DEV) {
@@ -1186,9 +1188,10 @@ test.describe('Errors', () => {
 		await page.goto('/errors/page-endpoint');
 		await Promise.all([page.waitForNavigation(), page.click('#post-implicit')]);
 		const json = await page.textContent('pre');
-		const { status, message, stack } = JSON.parse(json);
+		const { status, name, message, stack } = JSON.parse(json);
 
 		expect(status).toBe(500);
+		expect(name).toBe('Error');
 		expect(message).toBe('oops');
 
 		if (process.env.DEV) {
@@ -1209,9 +1212,10 @@ test.describe('Errors', () => {
 		await page.goto('/errors/page-endpoint');
 		await Promise.all([page.waitForNavigation(), page.click('#post-explicit')]);
 		const json = await page.textContent('pre');
-		const { status, message, stack } = JSON.parse(json);
+		const { status, name, message, stack } = JSON.parse(json);
 
 		expect(status).toBe(400);
+		expect(name).toBe('Error');
 		expect(message).toBe('oops');
 
 		if (process.env.DEV) {
@@ -1232,9 +1236,10 @@ test.describe('Errors', () => {
 			}
 		});
 
-		const { message, stack } = await response.json();
+		const { message, name, stack } = await response.json();
 
 		expect(response.status()).toBe(500);
+		expect(name).toBe('Error');
 		expect(message).toBe('oops');
 
 		if (process.env.DEV) {
@@ -1253,9 +1258,10 @@ test.describe('Errors', () => {
 			}
 		});
 
-		const { message, stack } = await response.json();
+		const { message, name, stack } = await response.json();
 
 		expect(response.status()).toBe(400);
+		expect(name).toBe('Error');
 		expect(message).toBe('oops');
 
 		if (process.env.DEV) {

From 3a2127f6200c5fabb9d0fc6c7f9e38bf91bca4f8 Mon Sep 17 00:00:00 2001
From: Rich Harris 
Date: Thu, 7 Jul 2022 13:09:18 -0400
Subject: [PATCH 17/29] better error serialization

---
 packages/kit/src/runtime/server/endpoint.js   |  3 +-
 packages/kit/src/runtime/server/index.js      |  2 +-
 .../kit/src/runtime/server/page/render.js     |  1 +
 packages/kit/src/runtime/server/utils.js      | 37 +++++++++++++----
 packages/kit/src/runtime/server/utils.spec.js | 41 ++++++++++++++++++-
 5 files changed, 73 insertions(+), 11 deletions(-)

diff --git a/packages/kit/src/runtime/server/endpoint.js b/packages/kit/src/runtime/server/endpoint.js
index 1ff4737f9f51..d9bfb3e9c74f 100644
--- a/packages/kit/src/runtime/server/endpoint.js
+++ b/packages/kit/src/runtime/server/endpoint.js
@@ -112,7 +112,8 @@ export async function render_endpoint(event, mod, options) {
 
 	if (is_pojo(body) && (!type || type.startsWith('application/json'))) {
 		headers.set('content-type', 'application/json; charset=utf-8');
-		normalized_body = body instanceof Error ? serialize_error(body, options) : JSON.stringify(body);
+		normalized_body =
+			body instanceof Error ? serialize_error(body, options.get_stack) : JSON.stringify(body);
 	} else {
 		normalized_body = /** @type {import('types').StrictBody} */ (body);
 	}
diff --git a/packages/kit/src/runtime/server/index.js b/packages/kit/src/runtime/server/index.js
index 28ac87390d86..9140a00018fb 100644
--- a/packages/kit/src/runtime/server/index.js
+++ b/packages/kit/src/runtime/server/index.js
@@ -322,7 +322,7 @@ export async function respond(request, options, state) {
 		]);
 
 		if (is_data_request || type === 'application/json') {
-			return new Response(serialize_error(error, options), {
+			return new Response(serialize_error(error, options.get_stack), {
 				status: 500,
 				headers: { 'content-type': 'application/json; charset=utf-8' }
 			});
diff --git a/packages/kit/src/runtime/server/page/render.js b/packages/kit/src/runtime/server/page/render.js
index dc231a546749..f19b279f791b 100644
--- a/packages/kit/src/runtime/server/page/render.js
+++ b/packages/kit/src/runtime/server/page/render.js
@@ -330,6 +330,7 @@ function try_serialize(data, fail) {
 
 /** @param {(Error & {frame?: string} & {loc?: object}) | undefined | null} error */
 function serialize_error(error) {
+	// TODO use other serialize_error
 	if (!error) return null;
 	let serialized = try_serialize(error);
 	if (!serialized) {
diff --git a/packages/kit/src/runtime/server/utils.js b/packages/kit/src/runtime/server/utils.js
index 290f225c191d..020bdca56a0f 100644
--- a/packages/kit/src/runtime/server/utils.js
+++ b/packages/kit/src/runtime/server/utils.js
@@ -58,14 +58,35 @@ export function normalize_request_method(event) {
 
 /**
  * @param {Error} error
- * @param {import('types').SSROptions} options
+ * @param {(error: Error) => string | undefined} get_stack
  */
-export function serialize_error(error, options) {
-	const object = {
-		name: error.name,
-		message: error.message,
-		stack: options.get_stack(error)
-	};
+export function serialize_error(error, get_stack) {
+	return JSON.stringify(clone_error(error, get_stack));
+}
+
+/**
+ * @param {Error} error
+ * @param {(error: Error) => string | undefined} get_stack
+ */
+function clone_error(error, get_stack) {
+	const {
+		name,
+		message,
+		stack,
+		// @ts-expect-error i guess typescript doesn't know about error.cause yet
+		cause,
+		...custom
+	} = error;
+
+	/** @type {Record} */
+	const object = { name, message, stack: get_stack(error) };
+
+	if (cause) object.cause = clone_error(cause, get_stack);
+
+	for (const key in custom) {
+		// @ts-expect-error
+		object[key] = custom[key];
+	}
 
-	return JSON.stringify(object);
+	return object;
 }
diff --git a/packages/kit/src/runtime/server/utils.spec.js b/packages/kit/src/runtime/server/utils.spec.js
index 34ce41d8ba04..40cbb484767e 100644
--- a/packages/kit/src/runtime/server/utils.spec.js
+++ b/packages/kit/src/runtime/server/utils.spec.js
@@ -1,6 +1,6 @@
 import { test } from 'uvu';
 import * as assert from 'uvu/assert';
-import { lowercase_keys } from './utils.js';
+import { lowercase_keys, serialize_error } from './utils.js';
 
 test('lowercase_keys', () => {
 	assert.equal(lowercase_keys({ KEY: 'value' }), { key: 'value' });
@@ -9,4 +9,43 @@ test('lowercase_keys', () => {
 	assert.equal(lowercase_keys({ 1: 'Hello World' }), { 1: 'Hello World' });
 });
 
+test('serialize_error', () => {
+	class FancyError extends Error {
+		name = 'FancyError';
+		fancy = true;
+
+		/**
+		 * @param {string} message
+		 * @param {{
+		 *   cause?: Error
+		 * }} [options]
+		 */
+		constructor(message, options) {
+			// @ts-ignore go home typescript ur drunk
+			super(message, options);
+		}
+	}
+
+	const error = new FancyError('something went wrong', {
+		cause: new Error('sorry')
+	});
+
+	const serialized = serialize_error(error, (error) => error.stack);
+
+	assert.equal(
+		serialized,
+		JSON.stringify({
+			name: 'FancyError',
+			message: 'something went wrong',
+			stack: error.stack,
+			cause: {
+				name: 'Error',
+				message: 'sorry',
+				stack: error.cause.stack
+			},
+			fancy: true
+		})
+	);
+});
+
 test.run();

From c8b9febd9e78906a306e13fa90175e5778a50c36 Mon Sep 17 00:00:00 2001
From: Rich Harris 
Date: Thu, 7 Jul 2022 13:17:52 -0400
Subject: [PATCH 18/29] beef up tests

---
 .../errors/page-endpoint/__error.svelte       |  3 +-
 .../routes/errors/page-endpoint/_shared.js    |  8 +++++
 .../errors/page-endpoint/get-explicit.js      |  4 ++-
 .../errors/page-endpoint/get-implicit.js      |  4 ++-
 .../errors/page-endpoint/post-explicit.js     |  4 ++-
 .../errors/page-endpoint/post-implicit.js     |  4 ++-
 packages/kit/test/apps/basics/test/test.js    | 35 +++++++++++--------
 7 files changed, 42 insertions(+), 20 deletions(-)
 create mode 100644 packages/kit/test/apps/basics/src/routes/errors/page-endpoint/_shared.js

diff --git a/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/__error.svelte b/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/__error.svelte
index 5b4cfb3c2ed6..d07904a83968 100644
--- a/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/__error.svelte
+++ b/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/__error.svelte
@@ -7,7 +7,8 @@
 			status: $page.status,
 			name: $page.error.name,
 			message: $page.error.message,
-			stack: $page.error.stack
+			stack: $page.error.stack,
+			fancy: $page.error.fancy
 		},
 		null,
 		'  '
diff --git a/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/_shared.js b/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/_shared.js
new file mode 100644
index 000000000000..3ae26117e794
--- /dev/null
+++ b/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/_shared.js
@@ -0,0 +1,8 @@
+export class FancyError extends Error {
+	name = 'FancyError';
+	fancy = true;
+
+	constructor(message, options) {
+		super(message, options);
+	}
+}
diff --git a/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/get-explicit.js b/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/get-explicit.js
index ecd85beed882..21e6c7139780 100644
--- a/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/get-explicit.js
+++ b/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/get-explicit.js
@@ -1,4 +1,6 @@
+import { FancyError } from './_shared.js';
+
 export const get = () => ({
 	status: 400,
-	body: new Error('oops')
+	body: new FancyError('oops')
 });
diff --git a/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/get-implicit.js b/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/get-implicit.js
index a3f81411957c..abf242945a3a 100644
--- a/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/get-implicit.js
+++ b/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/get-implicit.js
@@ -1,3 +1,5 @@
+import { FancyError } from './_shared.js';
+
 export const get = () => {
-	throw new Error('oops');
+	throw new FancyError('oops');
 };
diff --git a/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/post-explicit.js b/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/post-explicit.js
index 13a1d4c54bde..302a2992c464 100644
--- a/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/post-explicit.js
+++ b/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/post-explicit.js
@@ -1,4 +1,6 @@
+import { FancyError } from './_shared.js';
+
 export const post = () => ({
 	status: 400,
-	body: new Error('oops')
+	body: new FancyError('oops')
 });
diff --git a/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/post-implicit.js b/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/post-implicit.js
index 4cdd25946513..da5561fe7696 100644
--- a/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/post-implicit.js
+++ b/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/post-implicit.js
@@ -1,3 +1,5 @@
+import { FancyError } from './_shared.js';
+
 export const post = () => {
-	throw new Error('oops');
+	throw new FancyError('oops');
 };
diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js
index 18f02d94e095..c84cb61f4359 100644
--- a/packages/kit/test/apps/basics/test/test.js
+++ b/packages/kit/test/apps/basics/test/test.js
@@ -1141,15 +1141,16 @@ test.describe('Errors', () => {
 		await page.goto('/errors/page-endpoint');
 		await clicknav('#get-implicit');
 		const json = await page.textContent('pre');
-		const { status, name, message, stack } = JSON.parse(json);
+		const { status, name, message, stack, fancy } = JSON.parse(json);
 
 		expect(status).toBe(500);
-		expect(name).toBe('Error');
+		expect(name).toBe('FancyError');
 		expect(message).toBe('oops');
+		expect(fancy).toBe(true);
 
 		if (process.env.DEV) {
 			const lines = stack.split('\n');
-			expect(lines[1]).toContain('get-implicit.js:2:8');
+			expect(lines[1]).toContain('get-implicit.js:4:8');
 		}
 
 		const error = read_errors('/errors/page-endpoint/get-implicit');
@@ -1164,15 +1165,16 @@ test.describe('Errors', () => {
 		await page.goto('/errors/page-endpoint');
 		await clicknav('#get-explicit');
 		const json = await page.textContent('pre');
-		const { status, name, message, stack } = JSON.parse(json);
+		const { status, name, message, stack, fancy } = JSON.parse(json);
 
 		expect(status).toBe(400);
-		expect(name).toBe('Error');
+		expect(name).toBe('FancyError');
 		expect(message).toBe('oops');
+		expect(fancy).toBe(true);
 
 		if (process.env.DEV) {
 			const lines = stack.split('\n');
-			expect(lines[1]).toContain('get-explicit.js:3:8');
+			expect(lines[1]).toContain('get-explicit.js:5:8');
 		}
 
 		const error = read_errors('/errors/page-endpoint/get-explicit');
@@ -1188,15 +1190,16 @@ test.describe('Errors', () => {
 		await page.goto('/errors/page-endpoint');
 		await Promise.all([page.waitForNavigation(), page.click('#post-implicit')]);
 		const json = await page.textContent('pre');
-		const { status, name, message, stack } = JSON.parse(json);
+		const { status, name, message, stack, fancy } = JSON.parse(json);
 
 		expect(status).toBe(500);
-		expect(name).toBe('Error');
+		expect(name).toBe('FancyError');
 		expect(message).toBe('oops');
+		expect(fancy).toBe(true);
 
 		if (process.env.DEV) {
 			const lines = stack.split('\n');
-			expect(lines[1]).toContain('post-implicit.js:2:8');
+			expect(lines[1]).toContain('post-implicit.js:4:8');
 		}
 
 		const error = read_errors('/errors/page-endpoint/post-implicit');
@@ -1212,15 +1215,16 @@ test.describe('Errors', () => {
 		await page.goto('/errors/page-endpoint');
 		await Promise.all([page.waitForNavigation(), page.click('#post-explicit')]);
 		const json = await page.textContent('pre');
-		const { status, name, message, stack } = JSON.parse(json);
+		const { status, name, message, stack, fancy } = JSON.parse(json);
 
 		expect(status).toBe(400);
-		expect(name).toBe('Error');
+		expect(name).toBe('FancyError');
 		expect(message).toBe('oops');
+		expect(fancy).toBe(true);
 
 		if (process.env.DEV) {
 			const lines = stack.split('\n');
-			expect(lines[1]).toContain('post-explicit.js:3:8');
+			expect(lines[1]).toContain('post-explicit.js:5:8');
 		}
 
 		const error = read_errors('/errors/page-endpoint/post-explicit');
@@ -1236,11 +1240,12 @@ test.describe('Errors', () => {
 			}
 		});
 
-		const { message, name, stack } = await response.json();
+		const { message, name, stack, fancy } = await response.json();
 
 		expect(response.status()).toBe(500);
-		expect(name).toBe('Error');
+		expect(name).toBe('FancyError');
 		expect(message).toBe('oops');
+		expect(fancy).toBe(true);
 
 		if (process.env.DEV) {
 			expect(stack.split('\n').length).toBeGreaterThan(1);
@@ -1261,7 +1266,7 @@ test.describe('Errors', () => {
 		const { message, name, stack } = await response.json();
 
 		expect(response.status()).toBe(400);
-		expect(name).toBe('Error');
+		expect(name).toBe('FancyError');
 		expect(message).toBe('oops');
 
 		if (process.env.DEV) {

From 2694afa4e8ddb9dfa961e8e43b87e46ff521d2c3 Mon Sep 17 00:00:00 2001
From: Rich Harris 
Date: Thu, 7 Jul 2022 13:18:09 -0400
Subject: [PATCH 19/29] remove test.only

---
 packages/kit/test/apps/basics/test/test.js | 22 ++++++----------------
 1 file changed, 6 insertions(+), 16 deletions(-)

diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js
index c84cb61f4359..6d8fbbbaba30 100644
--- a/packages/kit/test/apps/basics/test/test.js
+++ b/packages/kit/test/apps/basics/test/test.js
@@ -1133,7 +1133,7 @@ test.describe('Errors', () => {
 		);
 	});
 
-	test.only('page endpoint GET thrown error message is preserved', async ({
+	test('page endpoint GET thrown error message is preserved', async ({
 		page,
 		clicknav,
 		read_errors
@@ -1157,7 +1157,7 @@ test.describe('Errors', () => {
 		expect(error).toContain('oops');
 	});
 
-	test.only('page endpoint GET returned error message is preserved', async ({
+	test('page endpoint GET returned error message is preserved', async ({
 		page,
 		clicknav,
 		read_errors
@@ -1181,10 +1181,7 @@ test.describe('Errors', () => {
 		expect(error).toBe(undefined);
 	});
 
-	test.only('page endpoint POST thrown error message is preserved', async ({
-		page,
-		read_errors
-	}) => {
+	test('page endpoint POST thrown error message is preserved', async ({ page, read_errors }) => {
 		// The case where we're submitting a POST request via a form.
 		// It should show the __error template with our message.
 		await page.goto('/errors/page-endpoint');
@@ -1206,10 +1203,7 @@ test.describe('Errors', () => {
 		expect(error).toContain('oops');
 	});
 
-	test.only('page endpoint POST returned error message is preserved', async ({
-		page,
-		read_errors
-	}) => {
+	test('page endpoint POST returned error message is preserved', async ({ page, read_errors }) => {
 		// The case where we're submitting a POST request via a form.
 		// It should show the __error template with our message.
 		await page.goto('/errors/page-endpoint');
@@ -1231,9 +1225,7 @@ test.describe('Errors', () => {
 		expect(error).toBe(undefined);
 	});
 
-	test.only('page endpoint thrown error respects `accept: application/json`', async ({
-		request
-	}) => {
+	test('page endpoint thrown error respects `accept: application/json`', async ({ request }) => {
 		const response = await request.get('/errors/page-endpoint/get-implicit', {
 			headers: {
 				accept: 'application/json'
@@ -1254,9 +1246,7 @@ test.describe('Errors', () => {
 		}
 	});
 
-	test.only('page endpoint returned error respects `accept: application/json`', async ({
-		request
-	}) => {
+	test('page endpoint returned error respects `accept: application/json`', async ({ request }) => {
 		const response = await request.get('/errors/page-endpoint/get-explicit', {
 			headers: {
 				accept: 'application/json'

From 75e69e193fe5065d807e90172eefee837284c948 Mon Sep 17 00:00:00 2001
From: Rich Harris 
Date: Thu, 7 Jul 2022 13:23:39 -0400
Subject: [PATCH 20/29] reuse serialize_error

---
 .../kit/src/runtime/server/page/render.js     | 20 ++-----------------
 1 file changed, 2 insertions(+), 18 deletions(-)

diff --git a/packages/kit/src/runtime/server/page/render.js b/packages/kit/src/runtime/server/page/render.js
index f19b279f791b..7166e5838165 100644
--- a/packages/kit/src/runtime/server/page/render.js
+++ b/packages/kit/src/runtime/server/page/render.js
@@ -6,6 +6,7 @@ import { render_json_payload_script } from '../../../utils/escape.js';
 import { s } from '../../../utils/misc.js';
 import { Csp, csp_ready } from './csp.js';
 import { PrerenderingURL } from '../../../utils/url.js';
+import { serialize_error } from '../utils.js';
 
 // TODO rename this function/module
 
@@ -179,7 +180,7 @@ export async function render_response({
 			trailing_slash: ${s(options.trailing_slash)},
 			hydrate: ${resolve_opts.ssr && page_config.hydrate ? `{
 				status: ${status},
-				error: ${serialize_error(error)},
+				error: ${error && serialize_error(error, options.get_stack)},
 				nodes: [${branch.map(({ node }) => node.index).join(', ')}],
 				params: ${devalue(event.params)},
 				routeId: ${s(event.routeId)}
@@ -325,20 +326,3 @@ function try_serialize(data, fail) {
 		return null;
 	}
 }
-
-// Ensure we return something truthy so the client will not re-render the page over the error
-
-/** @param {(Error & {frame?: string} & {loc?: object}) | undefined | null} error */
-function serialize_error(error) {
-	// TODO use other serialize_error
-	if (!error) return null;
-	let serialized = try_serialize(error);
-	if (!serialized) {
-		const { name, message, stack } = error;
-		serialized = try_serialize({ ...error, name, message, stack });
-	}
-	if (!serialized) {
-		serialized = '{}';
-	}
-	return serialized;
-}

From 3940383d97c7336eea0d1436db3606a5e6750cf4 Mon Sep 17 00:00:00 2001
From: Rich Harris 
Date: Thu, 7 Jul 2022 13:30:54 -0400
Subject: [PATCH 21/29] shut up eslint, you big dummy

---
 packages/kit/src/runtime/server/utils.js | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/packages/kit/src/runtime/server/utils.js b/packages/kit/src/runtime/server/utils.js
index 020bdca56a0f..717dc85c05ea 100644
--- a/packages/kit/src/runtime/server/utils.js
+++ b/packages/kit/src/runtime/server/utils.js
@@ -72,6 +72,8 @@ function clone_error(error, get_stack) {
 	const {
 		name,
 		message,
+		// this should constitute 'using' a var, since it affects `custom`
+		// eslint-disable-next-line
 		stack,
 		// @ts-expect-error i guess typescript doesn't know about error.cause yet
 		cause,

From 091c09fb867a0fa8587024df71f32a2f1e389426 Mon Sep 17 00:00:00 2001
From: Rich Harris 
Date: Thu, 7 Jul 2022 13:38:31 -0400
Subject: [PATCH 22/29] bah typescript

---
 packages/kit/src/runtime/server/utils.spec.js                    | 1 +
 .../apps/basics/src/routes/errors/page-endpoint/__error.svelte   | 1 +
 2 files changed, 2 insertions(+)

diff --git a/packages/kit/src/runtime/server/utils.spec.js b/packages/kit/src/runtime/server/utils.spec.js
index 40cbb484767e..458338ecc7db 100644
--- a/packages/kit/src/runtime/server/utils.spec.js
+++ b/packages/kit/src/runtime/server/utils.spec.js
@@ -41,6 +41,7 @@ test('serialize_error', () => {
 			cause: {
 				name: 'Error',
 				message: 'sorry',
+				// @ts-expect-error
 				stack: error.cause.stack
 			},
 			fancy: true
diff --git a/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/__error.svelte b/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/__error.svelte
index d07904a83968..5fdd3f068c7c 100644
--- a/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/__error.svelte
+++ b/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/__error.svelte
@@ -8,6 +8,7 @@
 			name: $page.error.name,
 			message: $page.error.message,
 			stack: $page.error.stack,
+			// @ts-expect-error
 			fancy: $page.error.fancy
 		},
 		null,

From 91aa2dd61733476a0e32ff5a30d1a798435f2042 Mon Sep 17 00:00:00 2001
From: Rich Harris 
Date: Thu, 7 Jul 2022 15:02:35 -0400
Subject: [PATCH 23/29] stack is already fixed

---
 packages/kit/src/runtime/server/page/render.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/kit/src/runtime/server/page/render.js b/packages/kit/src/runtime/server/page/render.js
index 7166e5838165..e4bdae307f5e 100644
--- a/packages/kit/src/runtime/server/page/render.js
+++ b/packages/kit/src/runtime/server/page/render.js
@@ -180,7 +180,7 @@ export async function render_response({
 			trailing_slash: ${s(options.trailing_slash)},
 			hydrate: ${resolve_opts.ssr && page_config.hydrate ? `{
 				status: ${status},
-				error: ${error && serialize_error(error, options.get_stack)},
+				error: ${error && serialize_error(error, e => e.stack)},
 				nodes: [${branch.map(({ node }) => node.index).join(', ')}],
 				params: ${devalue(event.params)},
 				routeId: ${s(event.routeId)}

From 01fdc7eee8b66200bc75c1ca7b3690f96a6ffd18 Mon Sep 17 00:00:00 2001
From: Rich Harris 
Date: Thu, 7 Jul 2022 15:15:40 -0400
Subject: [PATCH 24/29] explicitly add Error to ResponseBody

---
 packages/kit/types/index.d.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/kit/types/index.d.ts b/packages/kit/types/index.d.ts
index 36d64f6fa2db..e58d45caf09e 100644
--- a/packages/kit/types/index.d.ts
+++ b/packages/kit/types/index.d.ts
@@ -267,7 +267,7 @@ export interface ResolveOptions {
 	transformPage?: ({ html }: { html: string }) => MaybePromise;
 }
 
-export type ResponseBody = JSONValue | Uint8Array | ReadableStream;
+export type ResponseBody = JSONValue | Uint8Array | ReadableStream | Error;
 
 export class Server {
 	constructor(manifest: SSRManifest);

From fcbb8998af66b1787b86611b656fdd9122817611 Mon Sep 17 00:00:00 2001
From: Rich Harris 
Date: Thu, 7 Jul 2022 15:54:46 -0400
Subject: [PATCH 25/29] overhaul endpoint docs to mention Error

---
 documentation/docs/02-routing.md | 59 ++++++++++++++++++++++----------
 1 file changed, 40 insertions(+), 19 deletions(-)

diff --git a/documentation/docs/02-routing.md b/documentation/docs/02-routing.md
index d4bf789c4628..01012e162bcc 100644
--- a/documentation/docs/02-routing.md
+++ b/documentation/docs/02-routing.md
@@ -51,7 +51,41 @@ A route can have multiple dynamic parameters, for example `src/routes/[category]
 
 ### Endpoints
 
-Endpoints are modules written in `.js` (or `.ts`) files that export [request handler](/docs/types#sveltejs-kit-requesthandler) functions corresponding to HTTP methods. Their job is to make it possible to read and write data that is only available on the server (for example in a database, or on the filesystem).
+Endpoints are modules written in `.js` (or `.ts`) files that export [request handler](/docs/types#sveltejs-kit-requesthandler) functions corresponding to HTTP methods. Request handlers make it possible to read and write data that is only available on the server (for example in a database, or on the filesystem).
+
+Their job is to return a `{ status, headers, body }` object representing the response.
+
+```js
+/// file: src/routes/random.js
+/** @type {import('@sveltejs/kit').RequestHandler} */
+export async function get() {
+	return {
+		status: 200,
+		headers: {
+			'access-control-allow-origin': '*'
+		},
+		body: {
+			number: Math.random()
+		}
+	};
+}
+```
+
+- `status` is an [HTTP status code](https://httpstatusdogs.com):
+  - `2xx` — successful response (default is `200`)
+  - `3xx` — redirection (should be accompanied by a `location` header)
+  - `4xx` — client error
+  - `5xx` — server error
+- `headers` can either be a plain object, as above, or an instance of the [`Headers`](https://developer.mozilla.org/en-US/docs/Web/API/Headers) class
+- `body` can be a plain object or, if something goes wrong, an [`Error`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error). It will be serialized as JSON
+
+A `GET` or `HEAD` response must include a `body`, but beyond that restriction all three properties are optional.
+
+#### Page endpoints
+
+If an endpoint has the same filename as a page (except for the extension), the page gets its props from the endpoint — via `fetch` during client-side navigation, or via direct function call during SSR. If a page uses syntax for [named layouts](/docs/layouts#named-layouts) or [matchers](/docs/routing#advanced-routing-matching) in its filename then the corresponding page endpoint's filename must also include them.
+
+For example, you might have a `src/routes/items/[id].svelte` page paired with a `src/routes/items/[id].js` endpoint:
 
 ```js
 /// file: src/routes/items/[id].js
@@ -77,6 +111,8 @@ export async function get({ params }) {
 
 	if (item) {
 		return {
+			status: 200,
+			headers: {},
 			body: { item }
 		};
 	}
@@ -89,20 +125,7 @@ export async function get({ params }) {
 
 > Don't worry about the `$lib` import, we'll get to that [later](/docs/modules#$lib).
 
-The type of the `get` function above comes from `./[id].d.ts`, which is a file generated by SvelteKit (inside your [`outDir`](/docs/configuration#outdir), using the [`rootDirs`](https://www.typescriptlang.org/tsconfig#rootDirs) option) that provides type safety when accessing `params`. See the section on [generated types](/docs/types#generated-types) for more detail.
-
-The job of a [request handler](/docs/types#sveltejs-kit-requesthandler) is to return a `{ status, headers, body }` object representing the response, where `status` is an [HTTP status code](https://httpstatusdogs.com):
-
-- `2xx` — successful response (default is `200`)
-- `3xx` — redirection (should be accompanied by a `location` header)
-- `4xx` — client error
-- `5xx` — server error
-
-#### Page endpoints
-
-If an endpoint has the same filename as a page (except for the extension), the page gets its props from the endpoint — via `fetch` during client-side navigation, or via direct function call during SSR. If a page uses syntax for [named layouts](/docs/layouts#named-layouts) or [matchers](/docs/routing#advanced-routing-matching) in its filename then the corresponding page endpoint's filename must also include them.
-
-A page like `src/routes/items/[id].svelte` could get its props from the `body` in the endpoint above:
+The type of the `get` function above comes from `./__types/[id].d.ts`, which is a file generated by SvelteKit (inside your [`outDir`](/docs/configuration#outdir), using the [`rootDirs`](https://www.typescriptlang.org/tsconfig#rootDirs) option) that provides type safety when accessing `params`. See the section on [generated types](/docs/types#generated-types) for more detail.
 
 ```svelte
 /// file: src/routes/items/[id].svelte
@@ -114,11 +137,11 @@ A page like `src/routes/items/[id].svelte` could get its props from the `body` i
 

{item.title}

``` -Because the page and route have the same URL, you will need to include an `accept: application/json` header to get JSON from the endpoint rather than HTML from the page. You can also get the raw data by appending `/__data.json` to the URL, e.g. `/items/[id]/__data.json`. +To get the raw data instead of the page, you can include an `accept: application/json` header in the request, or — for convenience — append `/__data.json` to the URL, e.g. `/items/[id]/__data.json`. #### Standalone endpoints -Most commonly, endpoints exist to provide data to the page with which they're paired. They can, however, exist separately from pages. Standalone endpoints have slightly more flexibility over the returned `body` type — in addition to objects, they can return a `Uint8Array`. +Most commonly, endpoints exist to provide data to the page with which they're paired. They can, however, exist separately from pages. Standalone endpoints have slightly more flexibility over the returned `body` type — in addition to objects and [`Error`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error) instances, they can return a [`Uint8Array`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Uint8Array) or a [`ReadableStream`](https://developer.mozilla.org/en-US/docs/Web/API/ReadableStream). Standalone endpoints can be given a file extension if desired, or accessed directly if not: @@ -129,8 +152,6 @@ Standalone endpoints can be given a file extension if desired, or accessed direc | src/routes/data/index.js | /data | | src/routes/data.js | /data | -> Support for streaming request and response bodies is [coming soon](https://github.com/sveltejs/kit/issues/3419). - #### POST, PUT, PATCH, DELETE Endpoints can handle any HTTP method — not just `GET` — by exporting the corresponding function: From ca762d89a60b1f75aaf607b57ce895019c74061c Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 7 Jul 2022 16:01:21 -0400 Subject: [PATCH 26/29] more doc tweaks --- documentation/docs/02-routing.md | 30 +++++++++---------- .../src/lib/docs/client/docs.css | 10 ++++++- 2 files changed, 24 insertions(+), 16 deletions(-) diff --git a/documentation/docs/02-routing.md b/documentation/docs/02-routing.md index 01012e162bcc..c3f31062d0a2 100644 --- a/documentation/docs/02-routing.md +++ b/documentation/docs/02-routing.md @@ -83,9 +83,21 @@ A `GET` or `HEAD` response must include a `body`, but beyond that restriction al #### Page endpoints -If an endpoint has the same filename as a page (except for the extension), the page gets its props from the endpoint — via `fetch` during client-side navigation, or via direct function call during SSR. If a page uses syntax for [named layouts](/docs/layouts#named-layouts) or [matchers](/docs/routing#advanced-routing-matching) in its filename then the corresponding page endpoint's filename must also include them. +If an endpoint has the same filename as a page (except for the extension), the page gets its props from the endpoint — via `fetch` during client-side navigation, or via direct function call during SSR. (If a page uses syntax for [named layouts](/docs/layouts#named-layouts) or [matchers](/docs/routing#advanced-routing-matching) in its filename then the corresponding page endpoint's filename must also include them.) -For example, you might have a `src/routes/items/[id].svelte` page paired with a `src/routes/items/[id].js` endpoint: +For example, you might have a `src/routes/items/[id].svelte` page... + +```svelte +/// file: src/routes/items/[id].svelte + + +

{item.title}

+``` + +...paired with a `src/routes/items/[id].js` endpoint (don't worry about the `$lib` import, we'll get to that [later](/docs/modules#$lib)): ```js /// file: src/routes/items/[id].js @@ -123,19 +135,7 @@ export async function get({ params }) { } ``` -> Don't worry about the `$lib` import, we'll get to that [later](/docs/modules#$lib). - -The type of the `get` function above comes from `./__types/[id].d.ts`, which is a file generated by SvelteKit (inside your [`outDir`](/docs/configuration#outdir), using the [`rootDirs`](https://www.typescriptlang.org/tsconfig#rootDirs) option) that provides type safety when accessing `params`. See the section on [generated types](/docs/types#generated-types) for more detail. - -```svelte -/// file: src/routes/items/[id].svelte - - -

{item.title}

-``` +> The type of the `get` function above comes from `./__types/[id].d.ts`, which is a file generated by SvelteKit (inside your [`outDir`](/docs/configuration#outdir), using the [`rootDirs`](https://www.typescriptlang.org/tsconfig#rootDirs) option) that provides type safety when accessing `params`. See the section on [generated types](/docs/types#generated-types) for more detail. To get the raw data instead of the page, you can include an `accept: application/json` header in the request, or — for convenience — append `/__data.json` to the URL, e.g. `/items/[id]/__data.json`. diff --git a/sites/kit.svelte.dev/src/lib/docs/client/docs.css b/sites/kit.svelte.dev/src/lib/docs/client/docs.css index 4c14f89019bc..f6e9da40a77c 100644 --- a/sites/kit.svelte.dev/src/lib/docs/client/docs.css +++ b/sites/kit.svelte.dev/src/lib/docs/client/docs.css @@ -205,13 +205,21 @@ background: rgba(255, 62, 0, 0.1) !important; } +.content ul ul { + margin-bottom: 0; +} + +.content ul > li { + margin: 0.5em 0; +} + /** hacky overrides to allow filenames inside code blocks — TODO change the CSS in site-kit so we can get rid of this */ .code-block { background-color: var(--code-bg); color: var(--code-base); border-radius: 0.5rem; - margin: 0 0 1rem 0; + margin: 0 0 2rem 0; font-size: 14px; max-width: var(--linemax); box-shadow: inset 1px 1px 6px hsla(205.7, 63.6%, 30.8%, 0.06); From b19c3f54f9ea1aac3ac9ac2367a4a799014dc81a Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 7 Jul 2022 17:31:45 -0400 Subject: [PATCH 27/29] Update packages/kit/src/runtime/server/utils.spec.js Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com> --- packages/kit/src/runtime/server/utils.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/kit/src/runtime/server/utils.spec.js b/packages/kit/src/runtime/server/utils.spec.js index 458338ecc7db..2748a8b24d6c 100644 --- a/packages/kit/src/runtime/server/utils.spec.js +++ b/packages/kit/src/runtime/server/utils.spec.js @@ -21,7 +21,7 @@ test('serialize_error', () => { * }} [options] */ constructor(message, options) { - // @ts-ignore go home typescript ur drunk + // @ts-expect-error go home typescript ur drunk super(message, options); } } From 402ae5c5deb9d81349f760d798a7dc871dc23b3e Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 7 Jul 2022 17:40:50 -0400 Subject: [PATCH 28/29] add comments --- packages/kit/src/runtime/server/utils.js | 4 ++++ packages/kit/src/utils/http.js | 2 ++ 2 files changed, 6 insertions(+) diff --git a/packages/kit/src/runtime/server/utils.js b/packages/kit/src/runtime/server/utils.js index 717dc85c05ea..de9da9cf660c 100644 --- a/packages/kit/src/runtime/server/utils.js +++ b/packages/kit/src/runtime/server/utils.js @@ -57,6 +57,10 @@ export function normalize_request_method(event) { } /** + * Serialize an error into a JSON string, by copying its `name`, `message` + * and (in dev) `stack`, plus any custom properties, plus recursively + * serialized `cause` properties. This is necessary because + * `JSON.stringify(error) === '{}'` * @param {Error} error * @param {(error: Error) => string | undefined} get_stack */ diff --git a/packages/kit/src/utils/http.js b/packages/kit/src/utils/http.js index 8ddd7b07a1eb..29fb7ce6cad7 100644 --- a/packages/kit/src/utils/http.js +++ b/packages/kit/src/utils/http.js @@ -21,6 +21,8 @@ export function to_headers(object) { } /** + * Given an Accept header and a list of possible content types, pick + * the most suitable one to respond with * @param {string} accept * @param {string[]} types */ From 83c6252817ae04753c37f1856e678b8e049d8869 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 7 Jul 2022 17:52:09 -0400 Subject: [PATCH 29/29] DRY some stuff out --- .../kit/src/runtime/server/page/load_node.js | 51 +++++++------------ 1 file changed, 19 insertions(+), 32 deletions(-) diff --git a/packages/kit/src/runtime/server/page/load_node.js b/packages/kit/src/runtime/server/page/load_node.js index 8788aa34805d..43299934fb54 100644 --- a/packages/kit/src/runtime/server/page/load_node.js +++ b/packages/kit/src/runtime/server/page/load_node.js @@ -436,28 +436,9 @@ async function load_shadow_data(route, event, options, prerender) { }; if (!is_get) { - const result = await handler(event); - - // TODO remove for 1.0 - // @ts-expect-error - if (result.fallthrough) { - throw new Error( - 'fallthrough is no longer supported. Use matchers instead: https://kit.svelte.dev/docs/routing#advanced-routing-matching' - ); - } - - const { status, headers, body } = validate_shadow_output(result); - data.status = status; - + const { status, headers, body } = validate_shadow_output(await handler(event)); add_cookies(/** @type {string[]} */ (data.cookies), headers); - - // Redirects are respected... - if (status >= 300 && status < 400) { - data.redirect = /** @type {string} */ ( - headers instanceof Headers ? headers.get('location') : headers.location - ); - return data; - } + data.status = status; // explicit errors cause an error page... if (body instanceof Error) { @@ -471,6 +452,14 @@ async function load_shadow_data(route, event, options, prerender) { return data; } + // ...redirects are respected... + if (status >= 300 && status < 400) { + data.redirect = /** @type {string} */ ( + headers instanceof Headers ? headers.get('location') : headers.location + ); + return data; + } + // ...but 4xx and 5xx status codes _don't_ result in the error page // rendering for non-GET requests — instead, we allow the page // to render with any validation errors etc that were returned @@ -479,17 +468,7 @@ async function load_shadow_data(route, event, options, prerender) { const get = (method === 'head' && mod.head) || mod.get; if (get) { - const result = await get(event); - - // TODO remove for 1.0 - // @ts-expect-error - if (result.fallthrough) { - throw new Error( - 'fallthrough is no longer supported. Use matchers instead: https://kit.svelte.dev/docs/routing#advanced-routing-matching' - ); - } - - const { status, headers, body } = validate_shadow_output(result); + const { status, headers, body } = validate_shadow_output(await get(event)); add_cookies(/** @type {string[]} */ (data.cookies), headers); data.status = status; @@ -550,6 +529,14 @@ function add_cookies(target, headers) { * @param {import('types').ShadowEndpointOutput} result */ function validate_shadow_output(result) { + // TODO remove for 1.0 + // @ts-expect-error + if (result.fallthrough) { + throw new Error( + 'fallthrough is no longer supported. Use matchers instead: https://kit.svelte.dev/docs/routing#advanced-routing-matching' + ); + } + const { status = 200, body = {} } = result; let headers = result.headers || {};