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

Feature suggestion: error handling API #1096

Open
jbmoelker opened this issue Jan 11, 2018 · 57 comments
Open

Feature suggestion: error handling API #1096

jbmoelker opened this issue Jan 11, 2018 · 57 comments
Labels
feature request popular more than 20 upthumbs

Comments

@jbmoelker
Copy link

This is a feature suggestion for adding an error handling API to Svelte components based on this remark.

There is currently no built-in way to handle errors in Svelte components. React 16 introduced the concept of Error Boundaries to standardise this:

// Error handling life cycle in React:
componentDidCatch(error, info) {
    // Display fallback UI
    this.setState({ hasError: true });
    // You can also log the error to an error reporting service
    logErrorToMyService(error, info);
  }

I imagine a similar concept would be a good addition to Svelte components. Maybe

declaratively, as event on element:

<MyComponent on:error='doSomethingWith(e)' />

imperatively, as component life cycle event:

oncreate() { /* ... */ },
onerror(e) { /* ... */ },

I would check if the error is handled inside the component - with the onerror handler - first, and only if it isn't handled there, have it bubble up and trigger on:error. If a developer wants to handle the error inside the component and also wants to have it bubble up, they can use this.fire('error', e) within onerror.

@trenta3
Copy link

trenta3 commented Aug 8, 2019

I also would love error handling so much.
The problem with the current situation is that an error in whichever component completely blocks the UI.
Related to error handling, it could be useful if a setting was given to "restart" components in case of error up to a maximum numbers of errors. Another useful addition would be to "block" only the component code that raised exception.

@Crisfole
Copy link
Contributor

To fit w/ svelte v3 I think this should probably be just like the rest of the lifecycle hooks:

import { onError } from 'svelte';

onError(error => {
  Rollbar.report(error);
  return true; // Truthy to indicate handled, falsey to continue bubbling the error to the top level.
});

Internally this would do something like this:

export function onError(fn) {
	get_current_component().$$.on_error.push(fn);
}

And each component that imports onError would get its update code wrapped like so:

update = wrap(CURRENT UPDATE CODE);

Where wrap is shared code that looks like this:

function errorWrap(updater) {
  return function (...args) {
    try {
      updater(...args)
    } catch (err) {
      if (!get_current_component().$$.on_error.some(handler =>handler(err))) {
        throw;
      }
    }
  }
}

This is mostly a sketch here...

@antony
Copy link
Member

antony commented Aug 13, 2019

I like this for two reasons:

  • An unhandled component error will irrecoverably destroy a Sapper application (until refresh, which some users just don't seem to think to do)
  • I would like to report all of such errors to an API for triaging/inspection.

Gets my vote.

@carcinocron
Copy link

Here's the docs on Vue's implementation https://vuejs.org/v2/api/#errorCaptured

@trenta3

This comment has been minimized.

@Crisfole
Copy link
Contributor

@trenta3, let's not oversimplify here. Every feature requires a design, implementation, tests, and documentation. Even the implementation is bigger than just adding a try catch, you have to handle bubbling, adding event handlers, making any changes to parsers and code generators, writing the 'onError' function and all supporting code, etc.

I'm very much I'm favor of this, but it's certainly not a done deal. Even the design is under specified at this time.

@trenta3

This comment has been minimized.

@Crisfole

This comment has been minimized.

@lastqubit
Copy link

Writing a component like this would be nice:

<svelte:head>
    <title>Title</title>
</svelte:head>

<svelte:error>
    <h1>Opps error</h1>
</svelte:error>

<div>My component</div>

only svelte:error renders if there are any errors

@antony
Copy link
Member

antony commented Sep 13, 2019

@lastqubit I think the idea is to add error handling in component script code, rather than have a new svelte tag. The scope of a <svelte:error> tag is too narrow, as well as being cumbersome to use (how would error messages be passed, etc).

An onError lifecycle hook would allow for reporting, handling, and display, among other things, which a tag would not.

@lastqubit
Copy link

lastqubit commented Sep 14, 2019

@antony Without a svelte:error component everyone has to write the boilerplate code to display errors and that doesn't fit with sveltes strength of being user friendly.

If you somehow can get the error object in the error component it's very elegant. Everyone has to handle errors, so to have a specialized component that's very easy to use is not a narrow scope in my opinion. svelte:error becomes a more user friendly and elegant version of ErrorBoundary in react.

You can log like this:

const app = new App({
    target: document.body,
    onError: (e) = > console.error(e)
})

Without my suggestion you have to wrap every component in an if statement(very ugly)

<script>
    import { onError } from 'svelte'
    let error
    onError((e) => {
        error = e
    });
</script>

{#if error }
    <div>Opps error...</div>
{:else}
    <div>My component here</div>
{/if}

@antony
Copy link
Member

antony commented Sep 14, 2019

@lastqubit I feel that is very lazy error handling. Errors should be dealt with on a case by case basis, and only thrown to a global handler if there really is no way for the user to recover.

Boilerplate is only boilerplate if it's the same everywhere, which it shouldn't be. Additionally, if you're writing a notification display in every single component, wrapped in a <svelte:error> tag, that's the very definition of boilerplate.

Here's how I'd deal with errors in my components:

<script>
import { onError } from 'svelte'

onError(e => {
  // can I handle it automatically? do so
  stuff()
  // should I be informed about this?
  reportingApi.send(e)
  // should the user know about it?
  notifications.send('Some problem occured', e) 
})
</script>

and if I think this is too boilerplatey, I can export a handler from some .js file and pass the error to that:

<script>
import { onError } from 'svelte'
import { genericHandler } from '../my-error-handler.js'

onError(genericHandler(e => {
  // code which is called first to try to handle this locally
  return true // we've handled it here, don't do anything else.
})
</script>

@lastqubit
Copy link

@antony My example is more like ErrorBoundary in react, where your entire component should become an error page on exceptions. Your code handles errors by notifications and doesn't really show the code for actually displaying errors as html.

I see alot of value in a svelte:error component whose only purpose is to remove that extra ugly wrapping {#if} statement for displaying error pages.

You can have both an onError and svelte:error and choose what fits your needs.

@antony
Copy link
Member

antony commented Sep 14, 2019

@lastqubit I didn't want to include my entire notifications library in my example, but I use this: https://www.npmjs.com/package/@beyonk/svelte-notifications

I'm not averse to both a component and a handler, however I feel that having both could cause some confusion as to who handles the error first? The code, or the view component?

@lastqubit
Copy link

@antony The code handles first, and you get a chance to transform the error object. I don't know what syntax to use to get the error object in the component but it would be nice if you didn't have to declare it yourself in the script tag

<script>
    import { onError } from 'svelte'

    onError((e) => {
        console.error(e)
        return {
            statusCode: 500
        }
    });

</script>

<svelte:error as error>
    <div>Opps error {error.statusCode}</div>
</svelte:error>

@antony
Copy link
Member

antony commented Sep 14, 2019

I don't think there's a need for a <svelte:error> tag. It could be implemented trivially (and simply - a better solution might use contexts) thus:

{#if error}
<h1>An Error! {error.message}</h1>
<pre>
  {error.stackTrace}
</pre>
{:else}
<slot></slot>
{/if}

<script>
  import errorStore from './error-store.js'

  const unsubscribe = errorStore.subscribe(value => {
    if (!value) { return }
    error = value
    errorStore.set()
  })
  
  onDestroy(unsubscribe)

  let error = null
</script>

@lastqubit
Copy link

@antony That's like the example i wrote, and i think it's very ugly. It's annoying when frameworks are very elegant in demos and presentations but then that elegance disappear when you have to write real world code.

Everything is trivial in react to, but when you constantly have to write way more ugly code than needed it becomes a mess pretty quickly.

If i wanna write a simple image component that renders differently on exceptions i should write that code everytime instead of just adding svelte:error and render my error code in that?

@antony
Copy link
Member

antony commented Sep 14, 2019

@lastqubit The example above would not be repeated at all, it would be a component:

as a "boundary" style component:

<ErrorBoundary>
  <h1>Your other code</h1>
</ErrorBoundary>

or as a simple error notification:

<ErrorDisplay />

Roughly equivalent to your suggestion of <svelte:error>.

An annoying thing about frameworks is when they get too opinonated, which is, in my view, a problem React has.

@lastqubit
Copy link

@antony I don't understand why everyone should write their own error boundary component when svelte:error would be perfect. I don't understand why svelte:error would be opinionated and svelte:head for example is not.

It's very easy to change title in react but i love svelte:head. It's very elegant and right away you get a perfect understanding of your component, svelte:error would continue that tradition instead of having to write that annoying boiler plate code that everyone who starts using svelte have to write.

<svelte:head>
    <title>Title</title>
</svelte:head>

<svelte:error>
    <h1>Opps error</h1>
</svelte:error>

<div>My component</div>

is very elegant and right away you understand the component. The code variant is a mess in comparison.

@Crisfole
Copy link
Contributor

@lastqubit So, I think the issue is that it's not totally perfect. It doesn't define what should happen in parent components, it's not as flexible as onError and it doesn't allow you (for instance) to nest a svelte:head inside, or decide what to do with the rest of the rendering. What do you do with <div>My component</div> in your example? What about changing the <title>? I assume you can inspect the error...does <svelte:error> allow specifying which error types to expect?

With an onError function those questions are left in the capable hands of the coder. onError just becomes a lifecycle function that interacts with the rest of svelte seamlessly.

Here I feel like I must re-iterate that the implementation of this is non-trivial...

@Crisfole
Copy link
Contributor

One last comment here:

It's annoying when frameworks are very elegant in demos and presentations but then that elegance disappear when you have to write real world code.

Unfortunately the real world is tricky. Especially handling errors which can be for any one of an unlimited number of reasons. I don't think you'll find that the elegance has actually gone, though, just because a convenient piece of syntactic sugar is not present. Having a consistent and predictable pattern is key to the elegance.

Maybe once the core onError lifecycle is implemented (if maintainers decide to go that way) everyone will discover you're right and an implementation will be built in. I think that's probably what's going to happen. But until real life has proved it, it's usually best to go for the smallest most broadly applicable solution. I can definitely imagine <svelte:error> eventually being a thing, but it's a pretty dramatic change compared to an added importable function.

@bdoms
Copy link

bdoms commented Oct 28, 2019

Not having some sort of built in handler for errors is really painful, as I just discovered today. I have a window.onerror handler that was working fine, but I noticed today that an error occurred and it wasn't called. It's scary to think about an error happening and the fact that you might not know about it.

After some digging, I found out that the culprit was this line:

resolved_promise.then(flush);
resolved_promise.then(flush);

When an error is thrown inside a Promise it looks like Firefox still calls window.onerror, but Chrome swallows the error silently. I had to add a window.onunhandledrejection handler for this special case - I don't use any Promises in my own code, so I never thought I'd have to include something like that. For anyone finding this page with similar issues: even after adding that handler, Chrome would display the error in the console, but never trigger it while I was accessing the code over a hot reload web socket. I had to build a whole file and run it as if I were in production before Chrome actually triggered that other handler.

That last piece with the websockets is probably outside svelte's purview, but if svelte had provided some sort of onError handler I could've attached a function to, then it would simplify things for me, and prevent other developers from having to track down similar issues.

Ideally that same line above - and everywhere else svelte uses Promises like this - becomes something like resolved_promise.then(flush).catch(onErrorHandler) - that way there's one handler for everything from my perspective as a svelte user, and I don't have to worry about whether svelte has some internal promise that doesn't have a catch on it or not.

@RedHatter
Copy link
Contributor

This would be a very useful feature.

@happybeing
Copy link

Learning svelte I landed here looking for guidance on handling errors and think the onError() approach looks promising.

In the mean time something in the REPL or tutorial about this would help newcomers like me.

@eddpeterson
Copy link

If at least one component has smallest unhandled error, the whole app will crash and users will not know what to do and developers will not know such an error occurred.
@bdoms solution seems most pragmatic at the moment (thanks), but I hope more graceful error handling comes soon enough.

Here is the code snippet of how I am using it at the moment:
https://svelte.dev/repl/c6dddc73cbdd4f81883add43f5e3aa25?version=3.18.2

@carcinocron
Copy link

carcinocron commented Feb 13, 2020

Important to note that the snippet https://svelte.dev/repl/c6dddc73cbdd4f81883add43f5e3aa25?version=3.18.2 will not work for all types of errors, and there's no way to get something like Sentry to be able to record all errors and/or show a proper error message for all errors, so your app just freezes and if its' your users experiencing the error you won't even know about it. You can't write cypress tests for business logic edge cases and browser incompatibilities you don't know about.

@HazAT
Copy link

HazAT commented Feb 24, 2020

Just make sure to call the original onunhandledrejection after you are done, otherwise we (Sentry) are not able to pick up the error.

onMount(() => {
	const originalUnhandledRejection = window.onunhandledrejection;
	window.onunhandledrejection = (e) => {
          console.log('we got exception, but the app has crashed', e);
          // or do Sentry.captureException(e);
          originalUnhandledRejection(e);
	}
})

Another option is to do nothing at all and just call Sentry.init, our SDK is able to pick up most errors.

@antony
Copy link
Member

antony commented Apr 9, 2020

I think this would make a great RFC, if there is appetite.

@jonatansberg
Copy link

There is a related issue here:
#3733
I've created a basic error boundary component (based on a previous proof-of-concept by @halfnelson) available here:
https://svelte.dev/repl/9d44bbcf30444cd08cca6b85f07f2e2a?version=3.29.4

Curious, has anybody tried calling Sentry or Log Rocket using that onError prop/ functionality, that you implemented? (I intend to try it, just split attention at the moment)

Yes, that’s exactly how we use it :)

@antony
Copy link
Member

antony commented Feb 9, 2021

@nickolasgregory
Copy link

nickolasgregory commented Feb 9, 2021

For those without discord, that item links to;
https://github.com/sveltejs/rfcs/blob/4940d243733b284e93b154c2d3b989b2b396cda9/text/0000-error-boundary.md
Which I guess is;
https://github.com/sveltejs/rfcs/blob/rfc/error-boundary/text/0000-error-boundary.md

@antony
Copy link
Member

antony commented Feb 9, 2021

Whoops, my bad - copied the link from discord. thanks @nickolasgregory

@lastqubit
Copy link

@antony I'm glad you listened and implemented my idea in svelte kit

@al6x
Copy link

al6x commented Jun 9, 2021

An issue with alternative solution as try/catch looks like a better design.

@rster2002
Copy link
Contributor

I've taken the initiative to start and implement the onError idea as it's the simplest and would allow for more specialization down the line, but I have a question about what the expected behavior would be in your opinion in the following example:

<script>
    import { onError } from "svelte";

    let a;
    let b = {};
    let error = false;
    onError(() => {
        error = true;
    });

    a.b; // Error is thrown here

    b.c = true;
</script>

{#if error}
    Caught
{/if}

{#if b.c}
    Property `c` on `b` is true
{/if}

In this case, would you expect the behavior to be in this case? Should the 'Caught' message show? {#if b.c} would also throw because it's value is set after the error so what would you expect to happen there?

A case can also be make for not mounting the component at all because it would make it clearer for the developer what is being shown after an error and what isn't but this also defeats the purpose a bit of having onError.

Just let me know any of your thoughts on this.

@98mux
Copy link
Contributor

98mux commented Aug 18, 2021

@rster2002 Thoughts about this approach?

<script>
    import { onError } from "svelte";
    import ErrorComponent from "./ErrorComponent.svelte";

    let a;
    let b = {};
    onError(() => {
        return {component: ErrorComponent, props: {}};
    });

    a.b; // Error is thrown here

    b.c = true;
</script>

{#if b.c}
    Property `c` on `b` is true
{/if}

The onError return value will render when there is an error.
You could use

<svelte:component this={component} {...props}/>

To render the error component behind the scenes.

Fewer amount of lines, easier to refactor and reuse

@rster2002
Copy link
Contributor

Interesting approach. So this would basically replace the current component with the ErrorComponent in this case. What would happen if you didn't return something in onError? Would the current component not mount or should something else happen?

@98mux
Copy link
Contributor

98mux commented Aug 18, 2021

@rster2002 Yeah, i guess you could just show nothing then. Or maybe, there could be some default fallback ErrorComponent with some basic information about the error.

@98mux
Copy link
Contributor

98mux commented Aug 18, 2021

@rster2002 I'm not sure if it is possible, but if you solved the error you could return this, to attempt again? Probably would give kinda weird syntax

    onError(() => {
        if(solved){
            return {component:"this", props: {}}
        }
        else{
            return {component: ErrorComponent, props: {}};
        }
    });

@rster2002
Copy link
Contributor

It would probably be possible, but this would make it very easy to create infinite loops where the component fails initializing no matter what.

@98mux
Copy link
Contributor

98mux commented Aug 18, 2021

@rster2002
With some compiler magic, it could replace the onError implementation for all components returned within onError. It would replace it with an onError that returnes the default fallback ErrorComponent, with additional text such as "The returned component from onError also had an error".
Such that onErrors only gets 1 level deep.

Thought that would definitely complicate things haha :P

@basaran
Copy link

basaran commented Sep 5, 2021

while return {component: ErrorComponent, props: {}}; looks nice, it also feels reacty and vuey.

I like @eddpeterson

https://svelte.dev/repl/c6dddc73cbdd4f81883add43f5e3aa25?version=3.18.2

approach to be better. It's vanilla JS, doesn't bind you to specifc syntax and that's the main reason why I like svelte that it doesn't try to sandbox you into framework constraints.

@98mux
Copy link
Contributor

98mux commented Oct 9, 2021

@basaran @rster2002
This is probably more svelte like:

<script>
    import ErrorComponent from "./ErrorComponent.svelte";

    let a;
    let b = {};

    a.b; // Error is thrown here

    b.c = true;
</script>

<svelte:error on:error={(error) => {}} let:error={error}>
  <ErrorComponent statusCode={error.statusCode}/>
</svelte:error>

{#if b.c}
    Property `c` on `b` is true
{/if}

One idea is to ignore the error when returning false on the on:error

Seems like this guy has the same idea: sveltejs/rfcs#46 (comment)

@tv42
Copy link

tv42 commented Oct 29, 2021

I think it's worth mentioning that SvelteKit seems to be solving this problem one layer higher, with a separate src/routes/**/__error.svelte page: https://kit.svelte.dev/docs#layouts-error-pages

@mlandisbqs
Copy link

mlandisbqs commented Mar 5, 2022

I'm at a complete loss with this issue. We are preparing our svelte 3 application for a production release and encounter the following scenario:

  • page with components relies on a session store with a user in it.
  • session expires on the server - axios error is handled - and a subscriber to the session store redirects to log-in screen (using routify)

^-- this works gangbusters, except...

I noticed a scenario where a component on a particular page references a property we just removed from the session store ($session.user.email). Before the page route is redirected, the component on the current page throws

 TypeError: Cannot read properties of undefined (reading 'email')

...this appears to halt all execution and silently crashes the page, instead of redirecting the page route. I can see the errors in the console.

I can fix this specific scenario easily with a

$session.user?.email

...however there is no way for me to prevent the next guy from breaking this with the next code change unless we exercise session expiration on every page when we regression test the app. See the problem?

Note: We have no issues handling specific try catch scenarios that the code is anticipating. But there must be a way to implement a global error handler that I can wire up to my existing comprehensive error store + component for these kind of issues. I have tried Boundary and ErrorBoundary solutions as they have been floated in this thread, but none of those invoke the handler under the scenario described above when I tested them out.

IMHO we need a solution analogous to a global-scoped exception filter in NestJS (https://docs.nestjs.com/exception-filters) - a safety net for unanticipated errors that we can hook into and handle in our own preferred way without halting client-side app execution.

@hyrious
Copy link

hyrious commented Mar 15, 2022

Just want to add an example demonstrating the issue here: https://svelte.dev/repl/990219d2c7a34ce59bd7697ff552ea8a?version=3.46.4

In short, $: throw will shutdown the whole svelte app. Because these reactive expressions will be compiled to comp.$$.update() and then be called in flush() directly without a try-catch. The flush() is so fragile that if it is broken at the middle, it will never set update_scheduled to true, then there will be no flush() be called.

@gabrielnvian
Copy link

Hey everyone, is there any progress on this? It is the last major feature I would need before shipping my app to production

@benbucksch
Copy link
Contributor

benbucksch commented Aug 7, 2023

Errors can happen (at least) in the following places:

  • Sub-component creation
  • Component JS body code
  • onMount()/onDestroy()
  • Reactivity statements $:
  • JS expressions {} within the HTML section
  • Event handlers (on:click etc.)

If I missed some error classes, please add them.

For many of these error classes, the developer can manually add a try/catch, for each instance. However, for a large app, that can quickly become so cumbersome to not be realistic. And it also goes against the philosophy of Svelte of writing natural, concise code that expresses clearly what it does, not mechanical boilerplate. ("No boilerplate" is one of the Svelte mantras.)

Just to quantify the magnitude of the problem: Our app currently has almost 800 on:foo event handlers and over 300 $: reactivity statements. We now have a wrapper function to catch errors in event handlers, but even with that, it's barely realistic to change all this code and add the extra boilerplate everywhere. And if we did, we would then run into other Svelte issues.

JS, Java, C#, C++ and many other languages have exceptions, which bubble up errors the function stack and allow to catch and handle them on any level. We need the same on the UI level, to catch and bubble up errors to container Svelte components. Error boundaries similar to React seem like a good concept for that. It allows me to write code to handle the error - either replace the child component, or display the error.

Please add this to Svelte. Solid error handling is necessary for a reliable application in production.

@FeldrinH
Copy link

FeldrinH commented Aug 7, 2023

Hey everyone, is there any progress on this?

This comment on the Error Boundary RFC indicates that some kind of error boundaries are (probably) coming in Svelte 5.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request popular more than 20 upthumbs
Projects
None yet
Development

No branches or pull requests