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.

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

feat: add error boundaries #14211

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open

feat: add error boundaries #14211

wants to merge 13 commits into from

Conversation

trueadm
Copy link
Contributor

@trueadm trueadm commented Nov 7, 2024

This PR adds support for error boundaries to Svelte. Specifically, it adds <svelte:boundary>, which is a special element that can capture errors that occur from within its subtree during client rendering (error boundaries are no-ops during SSR).

The error boundary will capture all errors that occur in any effects (such as $effect and $effect.pre) within its subtree, as long as the code is run synchronously (code in an async or setTimeout will not be captured). <svelte:boundary> can report errors with using onerror, this can be a place where the error can be re-thrown to the next boundary:

Note: Errors in event handlers are not captured.

<script>
  function throw_error() {
    throw new Error('test')
  }
</script>

<svelte:boundary onerror={(e) => console.log('error caught')}>
  {throw_error()}
</svelte:boundary>

In addition, some fallback content can be rendered when an error occurs in a boundary using the failed snippet prop:

<script>
  function throw_error() {
    throw new Error('test')
  }
</script>

<svelte:boundary>
  {throw_error()}

  {#snippet failed(error)}
    <div>An error occurred! {e}</div>
  {/snippet}
</svelte:boundary>

Additionally, a reset function is passed as the second argument to both onerror and the failed prop:

<script>
  function throw_error() {
    throw new Error('test')
  }
</script>

<svelte:boundary>
  {throw_error()}

  {#snippet failed(error, reset)}
    <div>An error occurred! {e}</div>
    <button onclick={reset}>Try again</button>
  {/snippet}
</svelte:boundary>

Feel free to play around with them on the playground.

Closes #3733, closes #14054

Copy link

changeset-bot bot commented Nov 7, 2024

🦋 Changeset detected

Latest commit: c468c6d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
svelte Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@Rich-Harris
Copy link
Member

preview: https://svelte-dev-git-preview-svelte-14211-svelte.vercel.app/

this is an automated message

@trueadm trueadm changed the title feat: adds error boundaries feat: add error boundaries Nov 7, 2024
Copy link
Contributor

github-actions bot commented Nov 7, 2024

Playground

pnpm add https://pkg.pr.new/svelte@14211

tweak

tweak again

retry -> reset

tweaks

add tests

tweaks

tweaks

tweaks

more tests

more tests and tweaks

comments

tweak

tweak

tweak

tweak

tweak
tweak

tweak

tweak

more fixes

tweak

tweak

more fixes

changeset
@JReinhold
Copy link

JReinhold commented Nov 7, 2024

Can this be feature detected?
Say I'm building a library that will support Svelte 5.0.0 and up, and I want to progressively enhance the experience, with an error boundary if available, or just no handling if the project has Svelte v5.0.0.

Maybe there's a general way to feature detect <svelte:X> components that I just don't know about?

trueadm and others added 2 commits November 7, 2024 19:50
Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com>
Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com>
@levibassey
Copy link

levibassey commented Nov 7, 2024

Was kinda expecting the api to look like this

{#boundary}
    ...
{:else e}
    <div>An error occurred! {e}</div>
{/boundary}

Maybe even

{#try}
    ...
{:catch e}
    <div>An error occurred! {e}</div>
{/try}

Aren't error boundaries more like, control flow?

@trueadm
Copy link
Contributor Author

trueadm commented Nov 7, 2024

Was kinda expecting the api to look like this

{#boundary}
    ...
{:else}
    ...
{/boundary}

Maybe even

{#try}
    ...
{:catch}
    ...
{/try}

Aren't error boundaries more like control flow?

What if you want to re-throw an error, or log an error to sentry? What if you want to render the error message somewhere else? We explored this API and it has too many drawbacks. Not to mention, <svelte:boundary> can support other usages in the future other than just capturing errors.

@paoloricciuti
Copy link
Member

Can this be feature detected? Say I'm building a library that will support Svelte 5.0.0 and up, and I want to progressively enhance the experience, with an error boundary if available, or just no handling if the project has Svelte v5.0.0.

Maybe there's a general way to feature detect <svelte:X> components that I just don't know about?

What are you thinking about? Considering this should basically be for unexpected errors how would know about it help a library?

@dominikg
Copy link
Member

dominikg commented Nov 7, 2024

Can this be feature detected? Say I'm building a library that will support Svelte 5.0.0 and up, and I want to progressively enhance the experience, with an error boundary if available, or just no handling if the project has Svelte v5.0.0.
Maybe there's a general way to feature detect <svelte:X> components that I just don't know about?

What are you thinking about? Considering this should basically be for unexpected errors how would know about it help a library?

lets say storybook used it, and a user using storybook is on svelte 5.1, if we add this with 5.2, storybook either has to release a breaking change to add a boundary to their stories or feature detect it so that <svelte:boundary> does not trip the svelte 5.1 compiler to not break their users.

@JReinhold you should be able to use the exported version from svelte/compiler and compare that its equal or larger to the first release of this feature.

@VityaSchel

This comment was marked as spam.

@VityaSchel

This comment was marked as spam.

@dominikg

This comment was marked as off-topic.

@Leonidaz
Copy link

Leonidaz commented Nov 7, 2024

What if you don't want to show anything from inside the boundary if an error occurs, and instead use a fallback?

Right now it seems that whatever has a chance to get rendered / added to the dom, stays in, which could result in a broken state. I added an {#if} with a reactive error var but it doesn't seem to matter.

Demo from a commit in this pr

@trueadm
Copy link
Contributor Author

trueadm commented Nov 7, 2024

What if you don't want to show anything from inside the boundary if an error occurs, and instead use a fallback?

Right now it seems that whatever has a chance to get rendered / added to the dom, stays in, which could result in a broken state. I added an {#if} with a reactive error var but it doesn't seem to matter.

Demo from a commit in this pr

Looks like a bug, looking into that now.

Update:fixed!

@Ocean-OS
Copy link
Contributor

Ocean-OS commented Nov 7, 2024

I like this, however, like other people, I am not sure about the execution of the idea; maybe it could be a logic block, as the svelte: elements usually don't have children, and are often meant more for data or compiler options instead of actual DOM-related functions. Additionally, it doesn't feel as "magical" as other Svelte features/elements to have to use a snippet for the fallback, instead of the fallback being declared in a separate part of the block (eg an "else" statement).
Also, if merged, shouldn't this close #3733?

@trueadm
Copy link
Contributor Author

trueadm commented Nov 7, 2024

I like this, however, like other people, I am not sure about the execution of the idea; maybe it could be a logic block, as the svelte: elements usually don't have children, and are often meant more for data or compiler options instead of actual DOM-related functions. Additionally, it doesn't feel as "magical" as other Svelte features/elements to have to use a snippet for the fallback, instead of the fallback being declared in a separate part of the block (eg an "else" statement). Also, if merged, shouldn't this close #3733?

They compose far better than logic blocks. For example, you can create custom error boundaries for your app using the same snippet pattern:

<MyAppErrorBoundary>
  <Content />
  
  {#snippet failed(error)}
    <ErrorMessage {error} />
  {/snippet}
</MyAppErrorBoundary>

@JReinhold
Copy link

you should be able to use the exported version from svelte/compiler and compare that its equal or larger to the first release of this feature.

@dominikg Unless I'm misunderstanding you, I'm not sure this is true, since it happens at the compiler level. So even if you only have the boundary on a condition based on the version, the compiler will still try to compile it and fail. This will still fail in v5.1:

{#if false}
	<svelte:boundary>
		whoops
	</svelte:boundary>
{/if}

See https://www.sveltelab.dev/5doeu6jrx5dqxj1?files=.%2Fsrc%2Froutes%2F%2Bpage.svelte

@paoloricciuti
Copy link
Member

you should be able to use the exported version from svelte/compiler and compare that its equal or larger to the first release of this feature.

@dominikg Unless I'm misunderstanding you, I'm not sure this is true, since it happens at the compiler level. So even if you only have the boundary on a condition based on the version, the compiler will still try to compile it and fail. This will still fail in v5.1:

{#if false}
	<svelte:boundary>
		whoops
	</svelte:boundary>
{/if}

See https://www.sveltelab.dev/5doeu6jrx5dqxj1?files=.%2Fsrc%2Froutes%2F%2Bpage.svelte

I guess you could have a component that just renders the children snippet and one that wrap in boundary and dynamically import based on the condition

@Ocean-OS

This comment has been minimized.

@trueadm
Copy link
Contributor Author

trueadm commented Nov 7, 2024

@Ocean-OS I'm not sure what you're trying to do in that example? Half the code is commented out, is it meant to be there? What am I looking for?

@Leonidaz
Copy link

Leonidaz commented Nov 7, 2024

What if you don't want to show anything from inside the boundary if an error occurs, and instead use a fallback?
Right now it seems that whatever has a chance to get rendered / added to the dom, stays in, which could result in a broken state. I added an {#if} with a reactive error var but it doesn't seem to matter.
Demo from a commit in this pr

Looks like a bug, looking into that now.

Update:fixed!

Awesome! @trueadm

One more thing I thought of, currently if an error occurs in the failed snippet itself it leads to an infinite loop and crashes. I thought that just in case, it would be cool if it could be handled in some kind of try / catch and re-thrown further up the component tree.

here's a repro of the error in the failed snippet

@Ocean-OS
Copy link
Contributor

Ocean-OS commented Nov 7, 2024

@Ocean-OS I'm not sure what you're trying to do in that example? Half the code is commented out, is it meant to be there? What am I looking for?

I was testing to see how many times the code inside the boundary would rerun, using a counter. I was primarily trying to see when the DOM tree was replaced in the boundary. The counter never ran, which made me think something was wrong, so I tried the same thing in an if statement, which didn't seem to work either. I then tried commenting the boundary out to see if it would change anything, which it didn't.

@Ocean-OS
Copy link
Contributor

Ocean-OS commented Nov 7, 2024

@Ocean-OS I'm not sure what you're trying to do in that example? Half the code is commented out, is it meant to be there? What am I looking for?

I was testing to see how many times the code inside the boundary would rerun, using a counter. I was primarily trying to see when the DOM tree was replaced in the boundary. The counter never ran, which made me think something was wrong, so I tried the same thing in an if statement, which didn't seem to work either. I then tried commenting the boundary out to see if it would change anything, which it didn't.

Nevermind, it appears this didn't work in the latest release either. Sorry for the confusion.

@trueadm
Copy link
Contributor Author

trueadm commented Nov 8, 2024

@Leonidaz addressed your issue in a later commit

@Leonidaz
Copy link

Leonidaz commented Nov 8, 2024

addressed your issue

Amazing! 🤩 Everything works buttery smooth! Thank you!

@Thiagolino8
Copy link

svelte:boundary is a confusing name, it's a boundary of what? a suspense boundary? client-server boundary?

@dummdidumm
Copy link
Member

dummdidumm commented Nov 8, 2024

an error boundary, and possibly more in the future

@benbucksch
Copy link
Contributor

I often want the component to handle its own errors, including all $: , e.g. showing an inline error. How would I do that with this proposal?

@benbucksch
Copy link
Contributor

Does it catch all errors, including on:click event handlers throwing? This and $: are the most frequent source of errors.

I need the component to handle these errors, without adding try/catch in every single event handler and $: statement

Copy link

@AbhiPrasad AbhiPrasad left a comment

Choose a reason for hiding this comment

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

I work at Sentry on the JavaScript SDKs - this is exactly the kind of API we were hoping come from Svelte! Awesome job.

It would be nice to get a component stack (a stack trace with the component that threw, as well as the names and source locations of all its parent components) at time of error like we get with react error boundaries. In the react implementation they stitch together a synthetic stacktrace by walking the component tree and throwing errors. Not sure if a similar approach is viable here. Because the component stack is a synthetic stacktrace representing the component call chain, libraries like Sentry can parse it (and sourcemap it) to display in logging UI accordingly.

@@ -2026,6 +2026,10 @@ export interface SvelteHTMLElements {
[name: string]: any;
};
'svelte:head': { [name: string]: any };
'svelte:boundary': {
onerror?: (error: Error, reset: () => void) => void;

Choose a reason for hiding this comment

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

What do you think about adding a onreset prop here as well? There are some scenarios where executing a side effect could be useful.

I know users can reach in and wrap the reset method, but that feels like it introduces friction.

Copy link
Contributor Author

@trueadm trueadm Nov 8, 2024

Choose a reason for hiding this comment

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

We already provide component stack traces on our errors during DEV, this PR actually extends this so the stack happens in more places too.

Choose a reason for hiding this comment

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

Awesome, I didn't realize we had the component stack on error.message, seems like it was implemented earlier! That's great, any error logging sdk can parse that and use right away.

Right now this component stack is attached to an error with define_property(error, 'message'... in handle_error. Could we evaluate defining it as a separate field on the error? This would make it easier for error logging to just grab and parse the field instead of having to read through error.message. It would also be nice to pass the component_stack into onerror callback so it can be used in userland (users don't have to parse error.message to get access to it).

@Ocean-OS
Copy link
Contributor

Ocean-OS commented Nov 8, 2024

I often want the component to handle its own errors, including all $: , e.g. showing an inline error. How would I do that with this proposal?

Basically, you'd put the component that might throw an error inside the boundary, and the error handling inside a snippet in the boundary. Here's an example:

<svelte:boundary>
<ComponentThatMightThrow />
{#snippet failed(error, retry)}
There was an error: {error}<br>
Try again? <button onclick={retry} >Retry</button>
{/snippet} 
</svelte:boundary>

@Ocean-OS
Copy link
Contributor

Ocean-OS commented Nov 8, 2024

I work at Sentry on the JavaScript SDKs - this is exactly the kind of API we were hoping come from Svelte! Awesome job.

It would be nice to get a component stack (a stack trace with the component that threw, as well as the names and source locations of all its parent components) at time of error like we get with react error boundaries. In the react implementation they stitch together a synthetic stacktrace by walking the component tree and throwing errors. Not sure if a similar approach is viable here. Because the component stack is a synthetic stacktrace representing the component call chain, libraries like Sentry can parse it (and sourcemap it) to display in logging UI accordingly.

I agree, it would be nice if it threw an actual Error instead of just the error message, so you would be able to get both the stack and the message.

@Leonidaz
Copy link

Leonidaz commented Nov 8, 2024

@trueadm

tested a few more use cases, potentially a couple more issues.

not sure if this is expected but re-throwing without any top boundary handlers doesn't seem to result in an error, nothing happens.

rethrow - no error

And not sure if this can be handled, two boundaries and the same error in both only causes the top one to error and to render the failed snippet.

same error, only one boundary triggered, second stuck in the broken state

@trueadm
Copy link
Contributor Author

trueadm commented Nov 8, 2024

@Leonidaz Thank you so much for those test cases! I've fixed them and added regression test for both :)

@Ocean-OS
Copy link
Contributor

Ocean-OS commented Nov 8, 2024

I agree, it would be nice if it threw an actual Error instead of just the error message, so you would be able to get both the stack and the message.

@AbhiPrasad, it turns out it does have the error stack, here's an example.

packages/svelte/src/internal/client/runtime.js Outdated Show resolved Hide resolved
@@ -2026,6 +2026,10 @@ export interface SvelteHTMLElements {
[name: string]: any;
};
'svelte:head': { [name: string]: any };
'svelte:boundary': {
onerror?: (error: Error, reset: () => void) => void;

Choose a reason for hiding this comment

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

Awesome, I didn't realize we had the component stack on error.message, seems like it was implemented earlier! That's great, any error logging sdk can parse that and use right away.

Right now this component stack is attached to an error with define_property(error, 'message'... in handle_error. Could we evaluate defining it as a separate field on the error? This would make it easier for error logging to just grab and parse the field instead of having to read through error.message. It would also be nice to pass the component_stack into onerror callback so it can be used in userland (users don't have to parse error.message to get access to it).

@trueadm
Copy link
Contributor Author

trueadm commented Nov 9, 2024

Right now this component stack is attached to an error with define_property(error, 'message'... in handle_error. Could we evaluate defining it as a separate field on the error? This would make it easier for error logging to just grab and parse the field instead of having to read through error.message. It would also be nice to pass the component_stack into onerror callback so it can be used in userland (users don't have to parse error.message to get access to it).

@AbhiPrasad I've added the component_stack property to the error too. We don't want to overload the callback with extra arguments, so attaching to the error object itself seems the most ideal.

@benbucksch
Copy link
Contributor

benbucksch commented Nov 11, 2024

I often want the component to handle its own errors, including all $: , e.g. showing an inline error. How would I do that with this proposal?

Basically, you'd put the component that might throw an error inside the boundary, and the error handling inside a snippet in the boundary. Here's an example:

<svelte:boundary>
<ComponentThatMightThrow />
{#snippet failed(error, retry)}
There was an error: {error}<br>
Try again? <button onclick={retry} >Retry</button>
{/snippet} 
</svelte:boundary>

This is doesn't address the stated need of catching the error within the same component where the error appears. I cannot wrap every component (which handles its own errors) with another component.

@Leonidaz
Copy link

I often want the component to handle its own errors, including all $: , e.g. showing an inline error. How would I do that with this proposal?

Basically, you'd put the component that might throw an error inside the boundary, and the error handling inside a snippet in the boundary. Here's an example:

<svelte:boundary>
<ComponentThatMightThrow />
{#snippet failed(error, retry)}
There was an error: {error}<br>
Try again? <button onclick={retry} >Retry</button>
{/snippet} 
</svelte:boundary>

This is doesn't address the stated need of catching the error within the same component where the error appears. I cannot wrap every component (which handles its own errors) with another component.

@benbucksch You can wrap any part of your component markup with one or more boundaries.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add guidance for handling render errors, try block, like error boundary