From 6520ffa0f1b738a79a5f7bc041209378184dd446 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 16 May 2022 14:04:03 -0400 Subject: [PATCH 1/8] throw load validation errors so they are caught by handleError - closes #3388 --- .changeset/famous-birds-cheer.md | 5 +++++ packages/kit/src/runtime/load.js | 17 +++-------------- packages/kit/test/apps/basics/test/test.js | 5 ++++- 3 files changed, 12 insertions(+), 15 deletions(-) create mode 100644 .changeset/famous-birds-cheer.md diff --git a/.changeset/famous-birds-cheer.md b/.changeset/famous-birds-cheer.md new file mode 100644 index 000000000000..8b4141cbb1a1 --- /dev/null +++ b/.changeset/famous-birds-cheer.md @@ -0,0 +1,5 @@ +--- +'@sveltejs/kit': patch +--- + +Throw load validation errors so that they are caught by handleError diff --git a/packages/kit/src/runtime/load.js b/packages/kit/src/runtime/load.js index 5e031989c9d8..f943872ae55a 100644 --- a/packages/kit/src/runtime/load.js +++ b/packages/kit/src/runtime/load.js @@ -46,19 +46,11 @@ export function normalize(loaded) { if (loaded.redirect) { if (!loaded.status || Math.floor(loaded.status / 100) !== 3) { - return { - status: 500, - error: new Error( - '"redirect" property returned from load() must be accompanied by a 3xx status code' - ) - }; + throw new Error('"redirect" property returned from load() must be accompanied by a 3xx status code'); } if (typeof loaded.redirect !== 'string') { - return { - status: 500, - error: new Error('"redirect" property returned from load() must be a string') - }; + throw new Error('"redirect" property returned from load() must be a string'); } } @@ -67,10 +59,7 @@ export function normalize(loaded) { !Array.isArray(loaded.dependencies) || loaded.dependencies.some((dep) => typeof dep !== 'string') ) { - return { - status: 500, - error: new Error('"dependencies" property returned from load() must be of type string[]') - }; + throw new Error('"dependencies" property returned from load() must be of type string[]'); } } diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js index 3e56c2de803c..d3ee2195e78a 100644 --- a/packages/kit/test/apps/basics/test/test.js +++ b/packages/kit/test/apps/basics/test/test.js @@ -1861,7 +1861,7 @@ test.describe.parallel('Redirects', () => { } }); - test('errors on missing status', async ({ baseURL, page, clicknav }) => { + test.only('errors on missing status', async ({ baseURL, page, clicknav, read_errors }) => { await page.goto('/redirect'); await clicknav('[href="/redirect/missing-status/a"]'); @@ -1871,6 +1871,9 @@ test.describe.parallel('Redirects', () => { expect(await page.textContent('#message')).toBe( 'This is your custom error page saying: ""redirect" property returned from load() must be accompanied by a 3xx status code"' ); + + const lines = read_errors('/redirect/missing-status/a').split('\n'); + expect(lines[0]).toBe('Error: "redirect" property returned from load() must be accompanied by a 3xx status code'); }); test('errors on invalid status', async ({ baseURL, page, clicknav }) => { From fd36f415b7540893e2b159aec929ceceaf85912a Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 16 May 2022 14:04:37 -0400 Subject: [PATCH 2/8] remove .only --- packages/kit/test/apps/basics/test/test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js index d3ee2195e78a..cd609fbe3d48 100644 --- a/packages/kit/test/apps/basics/test/test.js +++ b/packages/kit/test/apps/basics/test/test.js @@ -1861,7 +1861,7 @@ test.describe.parallel('Redirects', () => { } }); - test.only('errors on missing status', async ({ baseURL, page, clicknav, read_errors }) => { + test('errors on missing status', async ({ baseURL, page, clicknav, read_errors }) => { await page.goto('/redirect'); await clicknav('[href="/redirect/missing-status/a"]'); From 162298035480674614ba880120ed8026236ce44b Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 23 May 2022 11:38:32 -0400 Subject: [PATCH 3/8] always remove errors.json to make local testing more consistent with CI --- packages/kit/test/apps/basics/package.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/kit/test/apps/basics/package.json b/packages/kit/test/apps/basics/package.json index 066d8c4dcea9..6d7fac230ff5 100644 --- a/packages/kit/test/apps/basics/package.json +++ b/packages/kit/test/apps/basics/package.json @@ -8,8 +8,8 @@ "preview": "node ../../cli.js preview", "check": "tsc && svelte-check", "test": "npm run test:dev && npm run test:build", - "test:dev": "cross-env DEV=true playwright test", - "test:build": "playwright test" + "test:dev": "rimraf test/errors.json && cross-env DEV=true playwright test", + "test:build": "rimraf test/errors.json && playwright test" }, "devDependencies": { "@sveltejs/kit": "workspace:*", From a76084564b20edaa2d05c3777659bf7b7c8ab7bf Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 23 May 2022 11:42:13 -0400 Subject: [PATCH 4/8] format --- packages/kit/src/runtime/load.js | 4 +++- packages/kit/test/apps/basics/test/test.js | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/kit/src/runtime/load.js b/packages/kit/src/runtime/load.js index f943872ae55a..099e3ccafc98 100644 --- a/packages/kit/src/runtime/load.js +++ b/packages/kit/src/runtime/load.js @@ -46,7 +46,9 @@ export function normalize(loaded) { if (loaded.redirect) { if (!loaded.status || Math.floor(loaded.status / 100) !== 3) { - throw new Error('"redirect" property returned from load() must be accompanied by a 3xx status code'); + throw new Error( + '"redirect" property returned from load() must be accompanied by a 3xx status code' + ); } if (typeof loaded.redirect !== 'string') { diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js index 10d833f1eba6..1dce1cba160a 100644 --- a/packages/kit/test/apps/basics/test/test.js +++ b/packages/kit/test/apps/basics/test/test.js @@ -1918,7 +1918,9 @@ test.describe.parallel('Redirects', () => { ); const lines = read_errors('/redirect/missing-status/a').split('\n'); - expect(lines[0]).toBe('Error: "redirect" property returned from load() must be accompanied by a 3xx status code'); + expect(lines[0]).toBe( + 'Error: "redirect" property returned from load() must be accompanied by a 3xx status code' + ); }); test('errors on invalid status', async ({ baseURL, page, clicknav }) => { From a06f27992459f8f0de0a50f20cf46d98723b6438 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 23 May 2022 11:58:38 -0400 Subject: [PATCH 5/8] fix test --- packages/kit/test/apps/basics/test/test.js | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js index 1dce1cba160a..f8ae449f0148 100644 --- a/packages/kit/test/apps/basics/test/test.js +++ b/packages/kit/test/apps/basics/test/test.js @@ -1906,7 +1906,13 @@ test.describe.parallel('Redirects', () => { } }); - test('errors on missing status', async ({ baseURL, page, clicknav, read_errors }) => { + test('errors on missing status', async ({ + baseURL, + page, + clicknav, + javaScriptEnabled, + read_errors + }) => { await page.goto('/redirect'); await clicknav('[href="/redirect/missing-status/a"]'); @@ -1917,10 +1923,13 @@ test.describe.parallel('Redirects', () => { 'This is your custom error page saying: ""redirect" property returned from load() must be accompanied by a 3xx status code"' ); - const lines = read_errors('/redirect/missing-status/a').split('\n'); - expect(lines[0]).toBe( - 'Error: "redirect" property returned from load() must be accompanied by a 3xx status code' - ); + if (!javaScriptEnabled) { + // handleError is not invoked for client-side navigation + const lines = read_errors('/redirect/missing-status/a').split('\n'); + expect(lines[0]).toBe( + 'Error: "redirect" property returned from load() must be accompanied by a 3xx status code' + ); + } }); test('errors on invalid status', async ({ baseURL, page, clicknav }) => { From c5d905afa82bf2efe1c1cb853f76b9f05edf2fdb Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 23 May 2022 12:15:07 -0400 Subject: [PATCH 6/8] force cache to clear --- packages/kit/test/apps/basics/test/test.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js index f8ae449f0148..99e05de3e2da 100644 --- a/packages/kit/test/apps/basics/test/test.js +++ b/packages/kit/test/apps/basics/test/test.js @@ -8,6 +8,8 @@ import { start_server, test } from '../../../utils.js'; /** @typedef {import('@playwright/test').Response} Response */ +/** please clear the cache, turborepo */ + test.describe.parallel('a11y', () => { test('resets focus', async ({ page, clicknav, browserName }) => { const tab = browserName === 'webkit' ? 'Alt+Tab' : 'Tab'; From 34d1f960e54330c9025f9506cd64ea5091a56aed Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 23 May 2022 12:17:55 -0400 Subject: [PATCH 7/8] gah --- packages/kit/src/cli.js | 2 ++ packages/kit/test/apps/basics/test/test.js | 2 -- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/kit/src/cli.js b/packages/kit/src/cli.js index 9f9e34e467d0..9b8d07d2f8e7 100755 --- a/packages/kit/src/cli.js +++ b/packages/kit/src/cli.js @@ -9,6 +9,8 @@ import { networkInterfaces, release } from 'os'; import { coalesce_to_error } from './utils/error.js'; import { logger } from './core/utils.js'; +/** please clear the cache, turborepo */ + /** @param {unknown} e */ function handle_error(e) { const error = coalesce_to_error(e); diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js index 99e05de3e2da..f8ae449f0148 100644 --- a/packages/kit/test/apps/basics/test/test.js +++ b/packages/kit/test/apps/basics/test/test.js @@ -8,8 +8,6 @@ import { start_server, test } from '../../../utils.js'; /** @typedef {import('@playwright/test').Response} Response */ -/** please clear the cache, turborepo */ - test.describe.parallel('a11y', () => { test('resets focus', async ({ page, clicknav, browserName }) => { const tab = browserName === 'webkit' ? 'Alt+Tab' : 'Tab'; From ddbff1bb5c2d0ef251c2a8edb9474f9900b0a989 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 23 May 2022 12:30:51 -0400 Subject: [PATCH 8/8] Update packages/kit/src/cli.js --- packages/kit/src/cli.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/kit/src/cli.js b/packages/kit/src/cli.js index 9b8d07d2f8e7..9f9e34e467d0 100755 --- a/packages/kit/src/cli.js +++ b/packages/kit/src/cli.js @@ -9,8 +9,6 @@ import { networkInterfaces, release } from 'os'; import { coalesce_to_error } from './utils/error.js'; import { logger } from './core/utils.js'; -/** please clear the cache, turborepo */ - /** @param {unknown} e */ function handle_error(e) { const error = coalesce_to_error(e);