Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

reload after error #9441

Merged
merged 2 commits into from
Mar 20, 2023
Merged

reload after error #9441

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3d21bf4
reload after error
Mar 17, 2023
b8e6860
only apply to unexpected errors, and warn that the page will be reloaded
Mar 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/lucky-paws-work.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': patch
---

fix: recover from errors during dev by reloading
20 changes: 18 additions & 2 deletions packages/kit/src/runtime/client/client.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ import { validate_common_exports } from '../../utils/exports.js';
import { compact } from '../../utils/array.js';
import { validate_depends } from '../shared.js';

let errored = false;

// We track the scroll position associated with each history entry in sessionStorage,
// rather than on history.state itself, because when navigation is driven by
// popstate it's too late to update the scroll position associated with the
Expand Down Expand Up @@ -1193,7 +1195,7 @@ export function create_client(app, target) {
});
}

if (__SVELTEKIT_DEV__ && status !== 404) {
if (DEV && status !== 404) {
console.error(
'An error occurred while loading the page. This will cause a full page reload. (This message will only appear during development.)'
);
Expand Down Expand Up @@ -1273,7 +1275,7 @@ export function create_client(app, target) {
if (priority <= options.preload_data) {
const intent = get_navigation_intent(/** @type {URL} */ (url), false);
if (intent) {
if (__SVELTEKIT_DEV__) {
if (DEV) {
preload_data(intent).then((result) => {
if (result.type === 'loaded' && result.state.error) {
console.warn(
Expand Down Expand Up @@ -1327,6 +1329,12 @@ export function create_client(app, target) {
if (error instanceof HttpError) {
return error.body;
}

if (DEV) {
errored = true;
console.warn('The next HMR update will cause the page to reload');
}

return (
app.hooks.handleError({ error, event }) ??
/** @type {any} */ ({ message: event.route.id != null ? 'Internal Error' : 'Not Found' })
Expand Down Expand Up @@ -1947,4 +1955,12 @@ if (DEV) {
}
console_warn(...args);
};

if (import.meta.hot) {
import.meta.hot.on('vite:beforeUpdate', () => {
if (errored) {
location.reload();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we add something here like console.warn('Hit an unrecoverable error. Reloading the page')?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no-one would see the message, because the page would reload the moment it was logged. we could add it when errored is set to true though

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think by default that's true, but I believe there's a checkbox you can toggle to persist it across a page navigation and I thought people might try that if they were confused why the reload was happening

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I have this turned on by default in my dev profile. Might be a nice addition in retrospect.

}
});
}
}