Skip to content

Commit

Permalink
[fix] more informative serialization error messages (#7303)
Browse files Browse the repository at this point in the history
* [fix] more informative serialization error messages

* Update packages/kit/src/runtime/server/page/actions.js

Co-authored-by: Geoff Rich <4992896+geoffrich@users.noreply.github.com>

* Update packages/kit/src/runtime/server/page/actions.js

* fix message, apply to client-side as well, update tests

Co-authored-by: Rich Harris <richard.a.harris@gmail.com>
Co-authored-by: Geoff Rich <4992896+geoffrich@users.noreply.github.com>
Co-authored-by: Rich Harris <hello@rich-harris.dev>
  • Loading branch information
4 people authored Oct 18, 2022
1 parent 7875003 commit 091c950
Show file tree
Hide file tree
Showing 6 changed files with 27 additions and 9 deletions.
5 changes: 5 additions & 0 deletions .changeset/serious-llamas-smoke.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': patch
---

[fix] more informative serialization error messages
6 changes: 3 additions & 3 deletions packages/kit/src/runtime/server/data/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ export async function render_data(event, route, options, state) {
nodes: nodes.slice(0, length)
};

return data_response(server_data);
return data_response(server_data, event);
} catch (e) {
const error = normalize_error(e);

Expand All @@ -122,10 +122,10 @@ export async function render_data(event, route, options, state) {
location: error.location
};

return data_response(server_data);
return data_response(server_data, event);
} else {
// TODO make it clearer that this was an unexpected error
return data_response(handle_error_and_jsonify(event, options, error));
return data_response(handle_error_and_jsonify(event, options, error), event);
}
}
}
6 changes: 5 additions & 1 deletion packages/kit/src/runtime/server/page/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -248,5 +248,9 @@ function check_serializability(value, id, path) {
}
}

throw new Error(`${path} returned from action in ${id} cannot be serialized as JSON`);
throw new Error(
`${path} returned from action in ${id} cannot be serialized as JSON without losing its original type` +
// probably the most common case, so let's give a hint
(value instanceof Date ? ' (Date objects are serialized as strings)' : '')
);
}
6 changes: 5 additions & 1 deletion packages/kit/src/runtime/server/page/render.js
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,11 @@ export async function render_response({
// function, but it would mean passing more stuff around than we currently do
const error = /** @type {any} */ (e);
const match = /\[(\d+)\]\.data\.(.+)/.exec(error.path);
if (match) throw new Error(`${error.message} (data.${match[2]})`);
if (match) {
throw new Error(
`Data returned from \`load\` while rendering /${event.routeId} is not serializable: ${error.message} (data.${match[2]})`
);
}
throw error;
}

Expand Down
11 changes: 8 additions & 3 deletions packages/kit/src/runtime/server/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,11 @@ export function allowed_methods(mod) {
return allowed;
}

/** @param {any} data */
export function data_response(data) {
/**
* @param {any} data
* @param {import('types').RequestEvent} event
*/
export function data_response(data, event) {
const headers = {
'content-type': 'application/json',
'cache-control': 'private, no-store'
Expand All @@ -79,7 +82,9 @@ export function data_response(data) {
} catch (e) {
const error = /** @type {any} */ (e);
const match = /\[(\d+)\]\.data\.(.+)/.exec(error.path);
const message = match ? `${error.message} (data.${match[2]})` : error.message;
const message = match
? `Data returned from \`load\` while rendering /${event.routeId} is not serializable: ${error.message} (data.${match[2]})`
: error.message;
return new Response(JSON.stringify(message), { headers, status: 500 });
}
}
Expand Down
2 changes: 1 addition & 1 deletion packages/kit/test/apps/basics/test/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,7 @@ test.describe('Shadowed pages', () => {

expect(await page.textContent('h1')).toBe('500');
expect(await page.textContent('#message')).toBe(
'This is your custom error page saying: "Cannot stringify arbitrary non-POJOs (data.nope)"'
'This is your custom error page saying: "Data returned from `load` while rendering /shadowed/serialization is not serializable: Cannot stringify arbitrary non-POJOs (data.nope)"'
);
});
}
Expand Down

0 comments on commit 091c950

Please sign in to comment.