From c9018017572963092995d1f789df8f03ae12b64c Mon Sep 17 00:00:00 2001 From: Tee Ming Date: Mon, 13 Nov 2023 14:18:19 +0800 Subject: [PATCH 1/5] catch json parse error when loading data --- .changeset/unlucky-knives-bake.md | 5 ++ packages/kit/src/runtime/client/client.js | 84 +++++++++++++---------- 2 files changed, 52 insertions(+), 37 deletions(-) create mode 100644 .changeset/unlucky-knives-bake.md diff --git a/.changeset/unlucky-knives-bake.md b/.changeset/unlucky-knives-bake.md new file mode 100644 index 000000000000..588a1b7f28a7 --- /dev/null +++ b/.changeset/unlucky-knives-bake.md @@ -0,0 +1,5 @@ +--- +'@sveltejs/kit': patch +--- + +fix: correctly show 404 for prerendered dynamic routes when navigating client-side without a root layout server load diff --git a/packages/kit/src/runtime/client/client.js b/packages/kit/src/runtime/client/client.js index 80abcfd65742..500bc123ae2f 100644 --- a/packages/kit/src/runtime/client/client.js +++ b/packages/kit/src/runtime/client/client.js @@ -1860,12 +1860,19 @@ async function load_data(url, invalid) { if (!res.ok) { // error message is a JSON-stringified string which devalue can't handle at the top level // turn it into a HttpError to not call handleError on the client again (was already handled on the server) - throw new HttpError(res.status, await res.json()); + throw new HttpError( + res.status, + await res.json().catch(() => { + // JSON parsing fails if the server responds with a HTML error page. + if (res.status >= 500) { + return 'Internal Server Error'; + } + return `Not found: ${url.pathname}`; + }) + ); } - // TODO: fix eslint error - // eslint-disable-next-line - return new Promise(async (resolve) => { + return new Promise((resolve) => { /** * Map of deferred promises that will be resolved by a subsequent chunk of data * @type {Map} @@ -1889,50 +1896,53 @@ async function load_data(url, invalid) { let text = ''; - while (true) { - // Format follows ndjson (each line is a JSON object) or regular JSON spec - const { done, value } = await reader.read(); - if (done && !text) break; + async function handle_stream() { + while (true) { + // Format follows ndjson (each line is a JSON object) or regular JSON spec + const { done, value } = await reader.read(); + if (done && !text) break; - text += !value && text ? '\n' : decoder.decode(value); // no value -> final chunk -> add a new line to trigger the last parse + text += !value && text ? '\n' : decoder.decode(value); // no value -> final chunk -> add a new line to trigger the last parse - while (true) { - const split = text.indexOf('\n'); - if (split === -1) { - break; - } + while (true) { + const split = text.indexOf('\n'); + if (split === -1) { + break; + } - const node = JSON.parse(text.slice(0, split)); - text = text.slice(split + 1); + const node = JSON.parse(text.slice(0, split)); + text = text.slice(split + 1); - if (node.type === 'redirect') { - return resolve(node); - } + if (node.type === 'redirect') { + return resolve(node); + } - if (node.type === 'data') { - // This is the first (and possibly only, if no pending promises) chunk - node.nodes?.forEach((/** @type {any} */ node) => { - if (node?.type === 'data') { - node.uses = deserialize_uses(node.uses); - node.data = deserialize(node.data); - } - }); + if (node.type === 'data') { + // This is the first (and possibly only, if no pending promises) chunk + node.nodes?.forEach((/** @type {any} */ node) => { + if (node?.type === 'data') { + node.uses = deserialize_uses(node.uses); + node.data = deserialize(node.data); + } + }); - resolve(node); - } else if (node.type === 'chunk') { - // This is a subsequent chunk containing deferred data - const { id, data, error } = node; - const deferred = /** @type {import('types').Deferred} */ (deferreds.get(id)); - deferreds.delete(id); + resolve(node); + } else if (node.type === 'chunk') { + // This is a subsequent chunk containing deferred data + const { id, data, error } = node; + const deferred = /** @type {import('types').Deferred} */ (deferreds.get(id)); + deferreds.delete(id); - if (error) { - deferred.reject(deserialize(error)); - } else { - deferred.fulfil(deserialize(data)); + if (error) { + deferred.reject(deserialize(error)); + } else { + deferred.fulfil(deserialize(data)); + } } } } } + handle_stream(); }); // TODO edge case handling necessary? stream() read fails? From 2701a8e8f3b569792405350ae64da43dc43e71c2 Mon Sep 17 00:00:00 2001 From: Tee Ming Date: Mon, 13 Nov 2023 21:34:29 +0800 Subject: [PATCH 2/5] fallback to native navigation instead --- packages/kit/src/runtime/client/client.js | 39 ++++++++++------------- 1 file changed, 17 insertions(+), 22 deletions(-) diff --git a/packages/kit/src/runtime/client/client.js b/packages/kit/src/runtime/client/client.js index 500bc123ae2f..75999ade7680 100644 --- a/packages/kit/src/runtime/client/client.js +++ b/packages/kit/src/runtime/client/client.js @@ -55,6 +55,17 @@ function update_scroll_positions(index) { scroll_positions[index] = scroll_state(); } +/** + * Loads `href` the old-fashioned way, with a full page reload. + * Returns a `Promise` that never resolves (to prevent any + * subsequent work, e.g. history manipulation, from happening) + * @param {URL} url + */ +function native_navigation(url) { + location.href = url.href; + return new Promise(() => {}); +} + /** * @param {import('./types.js').SvelteKitApp} app * @param {HTMLElement} target @@ -1196,17 +1207,6 @@ export function create_client(app, target) { return await native_navigation(url); } - /** - * Loads `href` the old-fashioned way, with a full page reload. - * Returns a `Promise` that never resolves (to prevent any - * subsequent work, e.g. history manipulation, from happening) - * @param {URL} url - */ - function native_navigation(url) { - location.href = url.href; - return new Promise(() => {}); - } - if (import.meta.hot) { import.meta.hot.on('vite:beforeUpdate', () => { if (current.error) location.reload(); @@ -1842,7 +1842,7 @@ export function create_client(app, target) { /** * @param {URL} url * @param {boolean[]} invalid - * @returns {Promise} + * @returns {Promise} */ async function load_data(url, invalid) { const data_url = new URL(url); @@ -1857,19 +1857,14 @@ async function load_data(url, invalid) { const res = await native_fetch(data_url.href); + if (res.headers.get('content-type') !== 'application/json') { + await native_navigation(url); + } + if (!res.ok) { // error message is a JSON-stringified string which devalue can't handle at the top level // turn it into a HttpError to not call handleError on the client again (was already handled on the server) - throw new HttpError( - res.status, - await res.json().catch(() => { - // JSON parsing fails if the server responds with a HTML error page. - if (res.status >= 500) { - return 'Internal Server Error'; - } - return `Not found: ${url.pathname}`; - }) - ); + throw new HttpError(res.status, await res.json()); } return new Promise((resolve) => { From 1eeb4f8b634910a5ad650f563c728da9f63262f9 Mon Sep 17 00:00:00 2001 From: Tee Ming Date: Mon, 13 Nov 2023 23:42:07 +0800 Subject: [PATCH 3/5] check only for html responses --- packages/kit/src/runtime/client/client.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/kit/src/runtime/client/client.js b/packages/kit/src/runtime/client/client.js index 75999ade7680..f0b94d1671ac 100644 --- a/packages/kit/src/runtime/client/client.js +++ b/packages/kit/src/runtime/client/client.js @@ -1857,7 +1857,7 @@ async function load_data(url, invalid) { const res = await native_fetch(data_url.href); - if (res.headers.get('content-type') !== 'application/json') { + if (res.headers.get('content-type')?.includes('text/html')) { await native_navigation(url); } From e9b9381905c9e9426d0aed49eecae66ef53115fc Mon Sep 17 00:00:00 2001 From: Tee Ming Date: Tue, 14 Nov 2023 00:32:00 +0800 Subject: [PATCH 4/5] add comment --- packages/kit/src/runtime/client/client.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/kit/src/runtime/client/client.js b/packages/kit/src/runtime/client/client.js index f0b94d1671ac..b209e0e47dfb 100644 --- a/packages/kit/src/runtime/client/client.js +++ b/packages/kit/src/runtime/client/client.js @@ -1857,6 +1857,8 @@ async function load_data(url, invalid) { const res = await native_fetch(data_url.href); + // if `__data.json` doesn't exist or the server has an internal error, + // fallback to native navigation so we avoid parsing the HTML error page as a JSON if (res.headers.get('content-type')?.includes('text/html')) { await native_navigation(url); } From a1dace138f75552c07b2b8aa245d02d29c8a7ff5 Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Tue, 14 Nov 2023 10:34:48 +0100 Subject: [PATCH 5/5] undo lint fix which doesnt really fix the underlying issue the lint tries to warn again --- packages/kit/src/runtime/client/client.js | 73 +++++++++++------------ 1 file changed, 36 insertions(+), 37 deletions(-) diff --git a/packages/kit/src/runtime/client/client.js b/packages/kit/src/runtime/client/client.js index b209e0e47dfb..b68f4bfbade4 100644 --- a/packages/kit/src/runtime/client/client.js +++ b/packages/kit/src/runtime/client/client.js @@ -1869,7 +1869,9 @@ async function load_data(url, invalid) { throw new HttpError(res.status, await res.json()); } - return new Promise((resolve) => { + // TODO: fix eslint error / figure out if it actually applies to our situation + // eslint-disable-next-line + return new Promise(async (resolve) => { /** * Map of deferred promises that will be resolved by a subsequent chunk of data * @type {Map} @@ -1893,53 +1895,50 @@ async function load_data(url, invalid) { let text = ''; - async function handle_stream() { - while (true) { - // Format follows ndjson (each line is a JSON object) or regular JSON spec - const { done, value } = await reader.read(); - if (done && !text) break; + while (true) { + // Format follows ndjson (each line is a JSON object) or regular JSON spec + const { done, value } = await reader.read(); + if (done && !text) break; - text += !value && text ? '\n' : decoder.decode(value); // no value -> final chunk -> add a new line to trigger the last parse + text += !value && text ? '\n' : decoder.decode(value); // no value -> final chunk -> add a new line to trigger the last parse - while (true) { - const split = text.indexOf('\n'); - if (split === -1) { - break; - } + while (true) { + const split = text.indexOf('\n'); + if (split === -1) { + break; + } - const node = JSON.parse(text.slice(0, split)); - text = text.slice(split + 1); + const node = JSON.parse(text.slice(0, split)); + text = text.slice(split + 1); - if (node.type === 'redirect') { - return resolve(node); - } + if (node.type === 'redirect') { + return resolve(node); + } - if (node.type === 'data') { - // This is the first (and possibly only, if no pending promises) chunk - node.nodes?.forEach((/** @type {any} */ node) => { - if (node?.type === 'data') { - node.uses = deserialize_uses(node.uses); - node.data = deserialize(node.data); - } - }); + if (node.type === 'data') { + // This is the first (and possibly only, if no pending promises) chunk + node.nodes?.forEach((/** @type {any} */ node) => { + if (node?.type === 'data') { + node.uses = deserialize_uses(node.uses); + node.data = deserialize(node.data); + } + }); - resolve(node); - } else if (node.type === 'chunk') { - // This is a subsequent chunk containing deferred data - const { id, data, error } = node; - const deferred = /** @type {import('types').Deferred} */ (deferreds.get(id)); - deferreds.delete(id); + resolve(node); + } else if (node.type === 'chunk') { + // This is a subsequent chunk containing deferred data + const { id, data, error } = node; + const deferred = /** @type {import('types').Deferred} */ (deferreds.get(id)); + deferreds.delete(id); - if (error) { - deferred.reject(deserialize(error)); - } else { - deferred.fulfil(deserialize(data)); - } + if (error) { + deferred.reject(deserialize(error)); + } else { + deferred.fulfil(deserialize(data)); } } } } - handle_stream(); }); // TODO edge case handling necessary? stream() read fails?