Skip to content

Commit

Permalink
throw load validation errors so they are caught by handleError (#4953)
Browse files Browse the repository at this point in the history
* throw load validation errors so they are caught by handleError - closes #3388

* remove .only

* always remove errors.json to make local testing more consistent with CI

* format

* fix test

* force cache to clear

* gah

* Update packages/kit/src/cli.js
  • Loading branch information
Rich-Harris authored May 23, 2022
1 parent a72123a commit 7d9d9c4
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 17 deletions.
5 changes: 5 additions & 0 deletions .changeset/famous-birds-cheer.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': patch
---

Throw load validation errors so that they are caught by handleError
19 changes: 5 additions & 14 deletions packages/kit/src/runtime/load.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,19 +46,13 @@ 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');
}
}

Expand All @@ -67,10 +61,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[]');
}
}

Expand Down
4 changes: 2 additions & 2 deletions packages/kit/test/apps/basics/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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:*",
Expand Down
16 changes: 15 additions & 1 deletion packages/kit/test/apps/basics/test/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1927,7 +1927,13 @@ test.describe.parallel('Redirects', () => {
}
});

test('errors on missing status', async ({ baseURL, page, clicknav }) => {
test('errors on missing status', async ({
baseURL,
page,
clicknav,
javaScriptEnabled,
read_errors
}) => {
await page.goto('/redirect');

await clicknav('[href="/redirect/missing-status/a"]');
Expand All @@ -1937,6 +1943,14 @@ 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"'
);

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 }) => {
Expand Down

0 comments on commit 7d9d9c4

Please sign in to comment.