-
Notifications
You must be signed in to change notification settings - Fork 82
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
Error Boundary RFC #46
base: master
Are you sure you want to change the base?
Conversation
I love that you're pushing this forward. If this is implementable as a directive, though, could it also be implementable as a lifecycle hook? I don't see a significant technical difference between:
and
But the second feels more svelte-y to me: an error occurring is a natural part of the component lifecycle in my mind. Undesirable but natural. |
That's another option, yeah. I don't know the implementation detail impact between the two. |
It's a great idea, whatever the syntax used. I'd like to add that it's not possible to properly implements this "in userland" today because there is no API for HOC component now in svelte, but it's probably another subject here. |
Just been informed about this existing RFC which is a different approach (and IMO has some drawbacks regarding how a handler would be called since it is strictly presentation layer) |
I think both the directive and lifecycle approaches are preferable over the template approach. The tricky part both with the existing error boundary lib (that I wrote) and I presume all these other options is how to reliably catch errors further down the tree. The current lib catches errors originating inside the child slot, as well as any errors that occur in children as a result of an update originating in or "passing through" said slot. Updates caused by internal state changes inside child components are not always caught tho. See CrownFramework/svelte-error-boundary#3 for details. |
@jonatansberg My intuition on this is that the |
It might be interesting if you have some concrete examples of what isn't caught, so that we can look into considering these for this implementation discussion. |
Yeah, that seems reasonable. Although maybe it would be better if you have to re-throw it since it's easy to introduce accidental implicit returns with arrow functions. The main problem with the current implementation is that errors do not bubble through the component tree but rather the call stack. These two may or may not be the same.
There is an open issue here CrownFramework/svelte-error-boundary#3 with a REPL repro. I don't have much more details than that I'm afraid as I haven't had the time to look in to further. Intuitively it makes sense tho as the current error boundary monkey patches the internal component lifecycle methods in order to catch errors. Updates further down the tree that are "self contained" are not affected by this. One way to address that would be to catch all errors by default and then use the context to propagate errors up through the component tree rather than the call stack. |
How about a Separate RFC? What do you think? |
Here's a REPL with an adapted example from your RFC. Essentially, as a developer, I have multiple use cases for error boundary:
All of these cases cannot be solved with current implementation and rely heavily on catching errors all the way down the tree. Current implementation won't help with Kit/Sapper issue as well, because it relies on slots, so there would be no way of restricting errors to a single page/layout. I would even go as far as to say error boundary loses its value if it only protects you from errors inside current component, because most of the time you can predict possible error states of it and handle them properly with simple a Additional Here's a quote from React's Error boundary documentation, which I can relate to. The idea is to make sure that this portion of tree never crushes the app in general.
|
As to the initial API proposal. But I really like the idea of wrapping some parts of the code inside a new expression. I think, it would be more versatile if Consider this snippet as an example. <script>
import CalendarWidget from 'calendar-dep';
let showFallback = false;
const genericHandler = (e) => {
// Log error, trigger fallback UI, etc.
}
</script>
{#if showFallback}
<input type="date" />
{:else}
<svelte:error onError={() => (showFallback = true)}>
<CalendarWidget />
</svelte:error>
{/if}
<svelte:error onError={genericHandler}>
// Here goes the content of the app.
</svelte:error> Not sure if it complicates stuff though. It can be achieved by just separating it into another component. |
I'm in favor of a component similar to React's I think we could do one better: combine it with the functionality provided by Each place where a developer put dkzlv's example might look something like this: <script>
import { Suspense } from 'svelte/suspense'
import CalendarWidget from 'calendar-dep'
function genericHandler (e) {
// Log error, trigger fallback UI, etc.
}
</script>
<Suspense>
<input slot="error" type="date" />
<p slot="loading">Loading Calendar...</p>
<CalendarWidget />
</Suspense>
<Suspense on:error={ genericHandler }>
// Here goes the content of the app.
</Suspense> Something like this might also provide a relatively straight forward path to handling top level awaits in components, lazy loading, or other features. |
@ADantes It is a nice addition, but Suspense is way out of the context of this RFC, I think. It's less about error handling and more about an opinionated way of handling data transfer (component code, prefetch). Btw, we already have I really like the idea of a slot and the idea that failed tree that has uncaught errors should be unmounted completely. |
To provide another use case - I'm looping through widgets installed on a dashboard and each will render their own component,e.g.
There is a growing library of hundreds of widget components, and performing error handling in each is wasteful. Implementing an error boundary in the I prefer the Is this currently in development / is there somewhere to track development or contribute? |
Since handling exceptions in component constructors is relatively trivial without compiler support, I don't think it's worth committing to error handling semantics without covering the entire life cycle. IMO this Take, for example: /*--------------------- index.js --------------------*/
import App from "./App.svelte";
try {
const app = new App({target: ...});
// Do stuff
} catch (err) {
// ... catastrophic failure ...
// root `<svelte:error>` directives set error handlers at the end of initialization
// since any exceptions in App constructor should be handled outside of Svelte
} <!------------------- App.svelte ------------------->
<script>
import ExternalFormComponent from "component-library";
import Component from "./Component.svelte";
import { onFormError, onAppError } from "./helpers/errorHandlers.js";
import { onSave, onSubmit, onCancel } from "./helpers/forms.js";
const onExternalError = async (err) => {
// Opaque component we hope is well tested, nothing to do but report it and refresh
// Oh no! PM rushed our dev so our error handler has an error!
Sentry.whatever(err); // <-- `whatever` is not a function
// Bubbles up to defaultErrorHandler in Component, then to onAppError
window.location.reload();
};
</script>
<svelte:error on:error={onAppError}>
<Component>
<svelte:error on:error={onExternalError}>
<ExternalFormComponent />
</svelte:error>
<svelte:error on:error={onFormError}>
<!-- All exceptions in onSubmit/onSave handled by `onFormError` -->
<button on:click={onSubmit}>Submit</button>
<button on:click={onSave}>Save</button>
</svelte:error>
<!-- Exception handled by `<svelte:error>` boundary around `Component` default slot -->
<button on:click={onCancel}>Cancel</button>
</Component>
</svelte:error>
<!-------------- Component.svelte ------------->
<script>
import { errorNotifications } from "./helpers/state.js";
export let title = "";
const defaultErrorHandler = (err) => {
$errorNotifications = [...$errorNotifications, { title, message: err.message }];
};
</script>
<svelte:error on:error={defaultErrorHandler}>
<slot />
</svelte:error> When the Svelte compiler encounters a
promise
.then(() => {...}, () => { /*...handleError...*/ })
.catch(() => { /*...handle error in error handler...*/ });
Off the top of my head: Pros:
Cons:
|
I've created a draft PR (sveltejs/svelte#6585). Of course nothing is set in stone, but I've opted to implement the
The third point does need some discussion when handling slots: <script>
import Wrapper from "./Wrapper.svelte";
let a = {}
</script>
<Wrapper>
{a.b.c}
</Wrapper> What would be preferable in this case if Again, even though I already created a draft PR, it's just to explore the implementation and would like your thoughts in this. |
I think about slots as a part of the providing component... If an error occurred in a slot I'd expect the onError of the parent (provider) component to handle it. I could see the case for making that configurable, but.... |
So I think that the idea is to both implement |
@Rich-Harris @Conduitry @dummdidumm @benmccann @TehShrike @orta @benaltair It's a huge unresolved issue for Svelte community: https://www.reddit.com/r/sveltejs/comments/q068rn/so_how_do_people_avoid_svelte_crashing_the_whole/ How do we resume activity on this RFC? |
@kyrylkov Please do not tag all the maintainers, it is unnecessary and impolite. This RFC is on our radar and will be dealt with as time allows. |
@antony I apologize, but this seems to be the only way to get attention. Last week I tagged 2 core team members and got no replies. I'm not asking for active contribution. I want to understand what needs to be done to accelerate the progress. Because So please let me know. |
Don't tag any maintainers, ever. We see all issues, and we allocate our own, volunteered time where possible. I'm not sure what "is not good enough" refers to exactly, but when addressing a team of volunteers who are building something on their own time and money, for the benefit of the community, I don't think it's appropriate. Please don't further pollute this RFC, if you want to chat about the contract between the svelte core team and yourself, then please come and find me in discord. |
To give a bit more detail on this, it is something that we've discussed at the monthly maintainers meeting. Rich had some ideas for rewriting a lot of the Svelte internals that would allows us to address this in a nice way, make hydration improvements, and make a number of other improvements. We'd like to consider those ideas in more depth as a possible solution. This is a very core part of the language, so it's important to us that we get the best solution even if that takes longer. I understand that it's frustrating not to have better error handling in the meantime of course, but think that's best for the long-term success of Svelte to do it right rather than quickly. I'm afraid we're unlikely to have many other updates on the topic for awhile. We're quite busy for the next couple months with other priorities such as getting to SvelteKit 1.0 and preparing for the upcoming SvelteSummit. I think we're likely to refocus on Svelte core for awhile after getting SvelteKit 1.0 out the door While it's not the error boundary feature, I do recommend folks interested in improve error handling upgrade to the latest version of Svelte, which does have improved error handling for |
What Rich has in mind would result in Svelte 4 or still Svelte 3? Is it worth putting effort into sveltejs/svelte#6585 or it has no chances of landing so we should just really give it more time and wait until after SvelteKit 1.0? |
I always find the awkward edge cases easier to suss out with code in front of me...i mean, just the conversation around "what would you expect here" had been valuable in my eyes. Maybe other folks are more talented than I am in that regard (I would totally believe that). |
Is there a workaround? I just discovered an open source package from npm that is a Svelte component, is throwing exceptions in prod, which causes the frontend app to freeze. |
Any update on this yet? Still a massive pain |
I think I'm missing something here... how is this still not implemented? What are people doing if you install a component from npm and it throws while it renders? Currently the entire app freezes, and there is no opportunity to recover and display a fallback. Your user simply can no longer use the page until they refresh This is akin to releasing a new programming language without try/catch - not good enough |
@mattfysh Our temporary work around has been to use window.onerror and window.onunhandledrejection handlers to show a fallback section on the layout page (and to handle the error with logging or whatever else). That being said, I agree with your sentiment. I strongly feel the framework should provide an opinionated path for handling this scenario. |
We will be releasing Svelte 4 before long as a smaller release and then moving onto Svelte 5 as a larger release where this is a likely goal. You can read more in the roadmap: https://svelte.dev/roadmap |
Volunteers, free time, priorities, day jobs, and anything else that might stop people instantly delivering every feature requested.
My solution? The community error boundary component, tied in to sentry. That and pick good third-party libraries that don't throw errors randomly!
Nobody is forcing you to use Svelte. Like Ben says, it is on our near term roadmap. |
@antony You guys are doing great work! I know online forums can feel like thankless work, but to be clear I love what you guys are doing and am happy with a very young product. Still want to provide feedback, but definitely don't want it to come off as unappreciative. |
This does sound very non-committal, I do worry for the companies building their apps with Svelte, who realize too late there is no granular try/catch available... and then find this thread to learn the Svelte team considers try/catch an optional feature.
Does not capture all scenarios and has not been updated in 2 years. There is a thread on hackernews about its shortcomings, 16 months ago: https://news.ycombinator.com/item?id=29808033
This is not a solution, and library authors do not throw errors at random or on a whim. To suggest that bugs cannot appear in even the most robust of community libraries is not a winning argument.
For fear of coming off as flippant, I would focus less on the developers who are trying to build production-grade apps, and think more about the users of those products... and the frustrating experience they suffer because the framework lacks appropriate error handling. On that note, I do not use Svelte any longer because of this very issue. If you look up you'll see I was the last to comment on this issue in June last year and received no reply (as I've learned, you do need to be somewhat inflammatory on github if you want a response). I ended up migrating back to React which was not a trivial amount of work. Serves me right for not performing proper due diligence I suppose. Signed, |
Hello universe... I'm totaly confused why is here 50+ thread messages xD Anyway... Is there currently any better solution to this thing... window.onerror = (msg, url, line, col, error) => {
alert("You done goofed!")
};
window.onunhandledrejection = (event) => {
alert("Ya don goofed more!")
}; |
You can’t treat different components’ errors differently, etc. very different solution than what is being requested |
It seems that this is no longer the case, from the Svelte 5 docs:
I also couldn't find anything directly related to this in the 5.0 milestone |
That's too bad. I didn't need or want runes. But error handling is the biggest lack in Svelte. |
Abandon all hope, ye who enter |
For Svelte 5.0 the focus is just in getting it released and having a good migration story. Additional features are being considered for 5.x releases |
"being considered" 🤔 "a likely goal" 🔮 "near term roadmap" -- one year ago 📆 Do the Svelte team appreciate there are a whole class of errors that cannot be caught by Svelte in its current form? When these errors occur the page locks up 🔒 completely freezes ❄️ the visitor cannot do a single thing more without reloading the entire page There is nothing available in the Svelte library to handle any errors The userland library mentioned only catches some errors Svelte team: 🤷♂️ |
Yeah, I absolutely love Svelte. Love the natural syntax, love the compile-time approach, love the performance focus, love how the tutorial is made. But with "runes", the current team is bringing Svelte 5 in a direction that is opposite to what I love about Svelte. I don't want to "migrate" my code, I love the syntax as-is in Svelte 3/4. We just need to add the 2 things that are missing the most in Svelte:
|
To be clear: we cannot implement error boundaries without rearchitecting Svelte. Error boundaries are one of the reason we undertook this massive rewrite. Your options are:
Either way, you'll get them at the same time! |
@Rich-Harris Thanks, that's re-assuring. I appreciate that you have it on your radar and are working towards it. FWIW, there are multiple classes of errors and error handling, as listed in my comment sveltejs/svelte#3733 (comment) . Maybe some other error classes are less difficult to handle (than error boundaries) for your code architecture? |
@Rich-Harris I think it is probably worth calling out in the docs that error handling specifically is planned for after the 5.0 release. You'll get less whining ;). This response is the first "This is definitely part of the reason we've built svelte-5" that I've heard, and I keep my ear to these issues. |
This comment was marked as off-topic.
This comment was marked as off-topic.
@sifat-at-work Rich's Comment is pretty unambiguous that it's not coming with Svelte 5.0 release, but was also pretty unambiguous that it's an active project for Svelte 5.x. Patience is a better look. |
After reading some of the discussion. I found this blogpost (Advanced error handling in svelte) helpful in implementing error boundaries yourself, it's not that complicated. Do this in combination with type checks in code and e2e tests on production builds in different browsers. |
I'm sure the maintainers are already painfully aware that this is a critically important feature for production applications, especially if they reach any level of complexity... but I'm just chiming in to also express my intense desire for this feature. Also, thanks for the hard work on the re-write. Runes and the improved Typescript support are a big plus (despite people's complaints and comparing it to React... they really aren't the same) - we expect our code to be significantly cleaner with the eventual migration to v5. |
rendered