-
-
Notifications
You must be signed in to change notification settings - Fork 923
This issue was moved to a discussion.
You can continue the conversation there. Go to discussion →
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 handling and recovery #2273
Comments
To clarify, you're proposing the following outcomes for
Correct? I personally think this is a can of worms best kept shut. As far as the outcomes above are concerned:
There's also a dodgy notion that while class and POJO components can conceivably handle their own internal exceptions with the The solution to the above is the same as what you would do in any other situation: don't transform your Mithril application composition to address fundamental logical errors: write better code that addresses your potential for exceptions explicitly in vanilla Javascript. Take a look at this component: () => {
let data
let error
let loading = true
return {
oninit: async () => {
await delay(1000)
try {
data = await m.request('https://rem-rest-api.herokuapp.com/api/users', {
withCredentials: true,
})
}
catch(e){
error = e
}
finally{
loading = false
}
},
view: () => (
loading
?
'Loading...'
:
data
?
JSON.stringify(data)
:
error
?
error
:
'🤷'
),
oncreate: async ({dom}) => {
try {
dom.style.cssText = `
opacity: 0;
transition: 1s;
`
await frame()
dom.style.opacity = 1
}
catch(e){
console.log('No big deal, life carries on as normal: ', e)
}
},
}
} This component has 2 identified potential exception sites: one in initialising its data and one in DOM mutation. For the first, we need to communicate this back to the user - this is caught, informs the internal model, and is reflected in the view; for the second, we determine that since this isn't really a critical issue, it's fine to fail silently without any compensatory logic at all. This is by far superior to the opinionated vtree error handling proposal:
1 & 3 are the real kickers: Mithril has long prided itself on being a Javascript-first library that embraces the language and empowers its users with subtlety & flexibility. The whole idea of opinionated error heuristics and automated recovery procedures introduces more complication to the code base and patronises the user into clumsiness and confusion, where ignorance and bad design are mitigated with further compensatory bad design. We shouldn't seek to compete in the domain of "programming is hard, let's go shopping" - the market is already saturated by people who have since made shopping hard. As the old saying goes, "now you've got two problems": what would normally be identified as a code typo, service failure or application design problem is now obscured into a special class of 'Mithril problems', for which the library claims to take some responsibility but can never truly give as satisfactory an outcome as could be devised without. Instead we should encourage users to think through the nature of their problems, ask for help, and come to superior solutions which deal more concretely with the specifics of their concerns. |
I decided to drop 1, since it appears unnecessary anyways. The I added 2 not because of opinions, but because if a component is truly in an invalid state, it's unsafe and potentially destructive to leave it up and running. It's the same reason people use Forever and just let Node processes crash and restart in backend servers, rather than trying to recover using
|
One of my favourite things about mithril is when my code crashes, the stack trace is very short and mostly my code. I can step through it, and there's no magical system trying to automatically recover. I think when talking about error handling it's beneficial to divide errors into two classes: Expected | Unexpected. Expected errors, are errors that are part of the contract of a system. An expected error may be JSON.parse throwing when you give it a string. String's aren't necessarily valid JSON, it's expected that JSON.parse can throw. And as Barney said, those kind of expected errors are best handled at the source. Whether it's at parse time, request time, call time etc. It's not mithril's concern. It's the application developer's concern. And if mithril crashes as a result of that developer's error, it's actually helpful because they will know where the problem is and how to resolve it. Unexpected errors, are errors that occur due to programmer error, whether in a library or an application. It's where behavior is counter to what is expected and/or documented. When such errors occur, they are bugs and should be fixed in the appropriate place. There's no reason a mithril view should ever throw, if it does, it's an unexpected error. As opposed to exposing an error handling hook on the component, we should let it crash so we can identify the bug. And then fix the bug in the form of a release. Building it into the mithril programming model, separate to native JS, doesn't make much sense to me. I think this is another situation where other frameworks (in the pursuit of DX) are actually making things worse by abstracting too much and blurring abstraction boundaries and responsibilities. Mithril has a job, it does it well. Error handling is best handled by other parts of the stack (e.g. sum types). |
This would have no effect on stack traces, even on propagated errors.
I do agree that it's not a good idea to use this for expected errors. This proposal is about handling unexpected errors, and I would specifically note that it's not a replacement for
Keep in mind that if left uncaught, the browser itself eventually swallows it and logs it so the page doesn't crash - this is required by spec. The difference is who swallows it: is it the browser, or is it a particular component that tolerates and potentially expects unexpected errors in children? Allowing a component to handle it allows things like:
Native JS doesn't have anything here for logically linking and collecting async error handling at all, not even for native promises. Node has domains, but those require complicated native embedder hooks. There was a proposal to add the language hooks we'd need, but that's basically stalled on Node still trying to nail down the correct VM primitives they need (which they themselves need formalized almost spec-level). If native JS had anything useful at all, I'd be proposing a means for Mithril to hook into that instead. 😉
The main drive for this isn't DX, it's two-fold and mostly about end user experience:
Also, this is almost entirely opt-in. The only breaking change here is that errors would clear the tree (calling |
@isiahmeadows To echo points made by @barneycarroll and @JAForbes, one of Mithril's major design features is relying on JavaScript/HTML/CSS as much as possible -- a design choice which makes it possible to preserve a lot of underlying simplicity in the Mithril codebase and developer experience (e.g. no need for a template language). And ideally we all want to keep it that way, which is why there is some pushback on this proposal out of fears (perhaps unfounded) that Mithril will drift into excessive complexity via adding fancy error handling. Many other UI tools (Angular, React, etc.) go down undesirable paths of complexity (Angular's reliance on Zones, Observables, and an ad-hoc templating language) and premature optimization (e.g. React's reliance on setState) for questionable benefits and ultimately poor developer ergonomics -- which is why I prefer Mithril. As a benefit, Mithril 1.0's core code is roughly only 1000 lines (or now approaching 2000 lines for 2.0) which can (in theory) be understood by a motivated individual programmer in a reasonable amount of time -- another reason I prefer Mithril. Using Mithril has made me a better JavaScript/HTML/CSS developer precisely because it leverages all three so much. By contrast, when I used Dojo/Dijit UI components they tried to hide all that complexity -- but those abstractions were still leaky and so I had to eventually learn all the JavaScript/HTML/CSS basics plus debug complicated leaky abstractions in Dojo/Dijit when something went wrong. Mithril is a huge breath of fresh air compared to any of those three (Angular, React, or Dojo/Dijit). I've used Angular with Zones. In practice, Angular Zones made debugging difficult because of long stack traces of unrelated code when anything goes wrong. I'd be concerned about adding Zone-ish complexity to Mithril. Mithril's simplifying assumption that the data model is dirty on any UI event or on a request (and you can handle timeouts and some other things yourself) is a great design simplification. While the use of an error handler is "opt in" what might not be opt in is dealing with a lot of extra code complexity in Mithril when debugging other errors if it goes down the Zones route. It's not just the amount of extra code as much as the way the extra Zones code adds complexity to every single user event interaction, timeout, or request. I've also user React's Error Boundaries. They are not that bad as a concept as a component you put above children that may fail -- but they still ultimately do not solve major issues such as knowing what to do with many errors you may want to display to the user. They also do nothing for unhandled promise rejections (which requires adding an onunhandledrejection callback anyway). And errors in the Error Boundary component itself are still not handled. And of course none of this protects against poorly-written error handlers in underlying code that silently swallow errors or promise failures. So, there is no magic bullet for dealing with errors. People will still need to test a lot and debug things. Ideally buggy code should fail as quickly as possible ("fail fast") and as near as possible to the originating code to make testing and debugging easier. When that is not possible there should ideally be a way to quickly trace back to the offending bit of code that configured the code that failed (which usually means preserving an extra reference somewhere between "compiled" object and "source" definition). I don't disagree with the React design quote you referenced on the value of not showing UIs in an error state -- but I am wondering about what tradeoffs are worth it in practice for Mithril at the cost of increased library code complexity for what specific developer or tester or user benefit? As a developer (and in practice tester of code I write) I am OK with error reporting being done in the console and not done via some limping-along UI parent component of the child with the issue -- if that means the core library code I use is easier to understand and more reliable and has had more attention paid instead to helping with the most common error cases. For example, yesterday, with Mithril 2.0, I had to play guessing games with errors caused by my passing in null or undefined for various things (including for a key). The failures were reported via long Mithril stack traces that left me inspecting lots of code looking for possible causes of a null or undefined somewhere. In practice, I feel reporting more of those sorts of simple errors earlier (like when the "m" call is made) would provide more bang-for-the-buck than more complex error handlers for downstream issues. Such extra checks might involve a cost to performance -- and if so, perhaps they might only be done when setting a global flag that could be turned off in production? All that said, I don't understand all of what is being proposed here well enough to know how these general design comments really apply to this specific proposal. I don't know the scale of the changes proposed to the codebase -- even if any reference to Zones worries me from Angular debugging experience. Also, without some sort of metrics on types of Mithril error issues and their frequency and severity in practice, we only have anecdotal comments as mine above on what areas of error handling should be a priority. So, still a bunch of unknowns. Maybe what you propose is indeed a great way forward overall despite some complexity concerns? What I do know is that Mithril 2.0 already works well right now overall, even with some issues and edge cases as above, and thanks for that! And I feel it is OK for Mithril to have edge cases it does not handle well if they are documented (e.g. stay away from this cliff) -- and it is clear they are the result of design tradeoffs consciously made to preserve simplicity. Mithril 1.0 had such limitations and we got a lot of great stuff done with it despite -- or perhaps because of -- those limitations. Every library will have limitations. Whether those limitations are "bad" depends in part on managing expectations. It is fantastic to be sharp and able to juggle a lot of complexity as a programmer (and I am less sharp than I was so many years ago, enjoy it while it lasts -- good nutrition, good sleep, exercise, stress management, and so on can make a difference in extending that time). But it is sometimes much harder to find an appropriate simplicity to get something done well that does not require as much deep insight to use or understand as it did to invent. Such solutions may transcend the "three tribes of programmers" by being all of elegant, performant, and practical -- as Mithril 1.0 was compared to AngularJS reflecting Leo's insights and experiences (and maybe a bit of Elm's reactive/functional ideals as ideas bounce around in the Noosphere?). Before proceeding on this proposal, I'd suggest (re?)watching this insightful Rich Hickey presentation on "Simple Made Easy" and meditating on the challenge of how the insightful design advice there might apply to improving Mithril error handling or other features: |
@isiahmeadows On a tangent from my previous comment, if you are going to do a deep dive into making Mithril more fault-tolerant -- whether via worthwhile tradeoffs or otherwise by insightful innovation that avoids tradeoffs -- here is a related document: A key point there is the distinction between "faults" and "failures" -- where essentially a failure occurs when an unhandled fault propagates across a subsystem boundary causing the subsystem to not comply with its specifications (e.g. for JavaScript, typically when an exception is thrown that is not handled in the originating component -- although there are many other sorts of possible failures like displaying incorrect results). Since systems usually have nested components, sometimes a failure of an internal component might be handled as a fault by the enclosing component and not propagated further. The paper also discusses aspects of fault tolerance like: Fault Avoidance, Fault Removal, Fault Detection, Fault Diagnosis, Fault Containment, Fault Masking, Fault Compensation, and Fault Repair and how they can happen at different points in the system lifecycle from design to development to use. It might be interesting to think about the combination of an application and the existing Mithril library and consider how different parts of the code or development process relate to those different aspects of fault tolerance. Then we could consider how this new proposal fits into that framework. |
Hi team, sorry if this is semi-private discussion, but I have a couple of thoughts on it. First, in "iterative" projects, as I call them, we usually simply deploy hand-tested prototypes to our company's business departments. It's hard to test them thoroughly (without data / real services which may be sensitive or otherwise unavailable), but it's also hard to deliver them in a timely manner if you're after ticking all the "industry-grade" boxes. Our production is a battlefield by its nature, not an usual build-test-test-test-deploy-happy cycle. I believe that many fast-growing or fast-niched companies work like that. Since projects may (and will) throw unexpected errors randomly in such semi-production, I have to catch and report all errors from a presentation layer, and the most plausible way to do that seems to be to wrap at least a We also wrote a I think that while "don't be too cool and rely on JS" is a good idea in general, "developers should fix them bugs" actually may turn into an impractical restriction (policy), not a mechanism. If the proposed I could just wrap every method in a try-catch, but that would explode LoCs and make source code feel as if you went to the store in a power armor just in case raiders visit the city. It is not your daily JS and I doubt that that aligns with an original simplicity intent. Mithril doesn't burden us with abstractions, and that's good, but its view hierarchy rendering is its responsibility, its run-loop and an its intrinsic lifetime property. If e.g. promise.then(result => {
if (!result.ok) return result
try {
return {ok:true, value:foo(result.value)}
}
catch (error) {
report(error)
return {ok:false, error}
}
}).then(...) instead of clean: promise
.then(foo)
.then(...)
.catch(report) ... or hope for the best. |
@dead-claudia why would we put the try/catch at the level of the component that defined the error handler rather than the one where the error occurred? Doing the latter requires little more code (using a stack-managed global to track the current error handler) and would also work with islands (a reentrant Likewise, |
@pygy I'll answer that a little out of order.
Nested
There's a few reasons for this:
|
This issue was moved to a discussion.
You can continue the conversation there. Go to discussion →
Mithril currently doesn't have any error handling story. This is of course really bad.
Here's the concerns I believe need addressed when it comes to fault tolerance, in decreasing priority:
The first two are adequately covered in #1937, but the other 4 are the focus of this bug: helping users write error-tolerant apps.
Edit: Update this for clarity, remove some special semantics.
My proposal is this: add a new error handling sequence algorithm to address all errors, with an associated optional hook
onerror: function (vnode, error) { ... }
usable on both components and vnodes.The error handling sequence is as follows:
onerror
hook. If no such parent exists, use the root.onremove
as appropriate, but notonbeforeremove
. If any hook throws, the hook sets the current error to it and otherwise ignores the error.onerror
hook with its vnode and the current error.In code, it would look something like this:
Here's when the error handling sequence is executed:
view
oronerror
.onevent
handler (likeonclick
) on a child DOM vnode throws.view
- component boundaries are ignored here.When it fires, it receives the current vnode (not the origin) and the thrown error. It may choose to do one of two things:
Here's a couple concrete example of this in action:
The text was updated successfully, but these errors were encountered: