Feature: Fire a global error-callback on errors

[getify]

This is a tricky one, and it's been asked before and I balked at it. But I can see that there's value in something like this, so we should investigate if there's a way to cleanly and reliably detect that the LABjs chain did or did not complete as expected.

It would likely need to be a per-chain option, specified at the beginning, which could only fire once, and once it fired, the rest of the chain would need to abort (if possible).

[scriby]

We were having an issue with runtime errors getting swallowed in scripts loaded by LABjs. I thought I had uncovered a browser issue with not always outputting the error messages. This is a pretty big deal for us, and anyone writing a serious javascript front-end.

I ran across https://github.com/getify/LABjs/issues/19 and noticed the debug mode.

This is better than nothing, but we want to be able to use window.onerror to catch as much as possible in production. LABjs's behavior will prevent errors in scripts loaded with LAB from being caught by onerror.

I think one solution might look like this...

try {
    //run script
} catch(e) {
    //whatever LAB normally does

    //Throw the exception from another context so the browser's usual error handling can occur
    setTimeout(function(){
        throw e;
    });
}

If that doesn't work, then a way to specify an error handling function would also be welcome.

Thanks,

Chris

[scriby]

Did you notice the solution I provided will still allow LAB.js to work? The error is thrown from within a setTimeout, so it won't affect the current execution. But it will allow the error to behave like errors normally do in javascript ... I don't understand why we must accept different error behavior for code that's run from LAB.

I just tried doing it from your catch block and it works fine.

I appreciate your opinions on the best way to do error handling, but please understand that all projects are different and have different needs, constraints, and trade-offs. I simply want to be notified of errors that come up in production that were missed through other means so they can be resolved.

[getify]

we want to be able to use window.onerror to catch as much as possible in production.

I think it's a poor development form to rely on errors bubbling up to window.onerror in production just so you can find them. You would be better off just wrapping every JS file in a big try/catch, if the "sledge hammer, meet tiny nail" approach is what you're going after.

Why not focus on proper code techniques for robust error handling, rather than haphazardly letting errors bubble up?

Moreover, the problem is I can't just let an error happen, or it will break LAB's internal functionality (possibly putting it in an indeterminate state) and stop the rest of the chain from executing. LABjs has to swallow the errors to prevent them from interferring with itself. The problem with not doing so is that people will tend to blame LABjs (since the dev tools will often "lie" and say that the error occurred in LABjs code).

By contrast, swallowing the errors and then letting a debug mode let you see those errors if you want to is far more graceful, especially when production environments are considered. I have found that most people would rather have graceful "most of the code" load and run, rather than "fail hard and early".

[getify]

The error is thrown from within a setTimeout, so it won't affect the current execution.

Actually, this breaks error handling in a far worse way than what LABjs currently does. Not all browsers are smart enough to allow re-throwing an error and have it preserve the original context, so in many cases, your dev tools end up identifying the throw line as the offending error line, not the original line that was catchd. Moreover, if you wrap the error handling in a setTimeout, now you further exacerbate debugging problems because you eliminate the call-stack from what the dev-tools can report.

Overall, I think it's a very bad idea to try and re-throw an error, and even worse to do so in a delayed fashion as you suggest.

I'd be better off literally calling window.onerror manually and passing in the error object myself, because at least the call-stack would be preserved (although muddied).

[getify]

Suggestion: before loading LAB.js, install a "proxy" on top of console.error. That way, when LABjs calls console.error, you'll be notified (much like if the JS engine calls window.onerror).

if (!console) console = {};
if (!console.error) console.error = function(){};
(function(){
  var old_error = console.error;
  console.error = function() {
     old_error.apply(this,arguments);
     alert("yay, I caught an error!");
  };
})();

[scriby]

Ok, I'll just leave you with one parting thought.

The current default behavior is much worse. We've had a very hard time debugging our app because some scripts would just stop executing with no information. We didn't even know an error was occurring.

After weeks of guessing I finally figured out the pattern that it only occurs in scripts loaded from LAB. The default behavior cost my team some serious time (others came to me asking about things breaking with no details).

No matter what you do, I encourage you to make the default behavior less surprising. I did read through available documentation when we adopted LAB, and don't recall reading anything about errors getting swallowed. But even if it is in the docs, it's a terrible thing to miss.

Thanks for all your hard work on this library.

Chris

[getify]

I think it's a fair point that perhaps this issue needs more specific and direct attention in the documentation. It's inferred loosely by the "Debug" mode but not particularly called out. I will put it on my TODO list to make sure the documentation is more complete on the topic of error handling.

I'm not sure I would call "let the error bubble to the top so we make sure that end users get annoyed by it" is the proper default pattern for handling errors. I also don't think the proper default is "as soon as 1 error occurs, abandon all hope and do a full stop". These are things that I think represent a greater surprise to most developers (especially the less adept technical audience) than is "hey, if you wanna see what is going on under the covers, use 'Debug' mode".

I want the default behavior of LABjs to be that it does as much as it can, not that it gives up (because JS gives up) at first error.

---

The technical consequences of LABjs bailing on its part early are, I think, MUCH harder to find and debug, and much more likely to be blamed on LABjs. The problem is that we can't "fully abort" when an error happens, we can only partially abort at best. This leads to crazy to diagnose errors almost instantaneously.

For instance, imagine you have 5 scripts you're loading, and for each one, you have a .wait(..) callback to run after that script runs. Now, let's say that a script error occurs in the first of those callbacks. What will the behavior be if LABjs doesn't try to robustly handle that case? All 5 scripts themselves will run, in order, because that part is handled by the browser, but the 4 subsequent callbacks will NOT fire, because LABjs was aborted (by the JS engine) with the first error.

What will happen? Probably LOTS of errors will bubble up, from all of your scripts. And most of them will be WTF red herrings that lead you down the wrong path, because it won't be at all obvious that LABjs only ran 1 of the 5 callbacks, even though all 5 scripts themselves ran.

What's the more sane behavior by default? To keep running what callbacks we can, so that the only errors which occur are those DIRECTLY related to the actual problem, and not artifacts of weird internal implementation details of LABjs and how browsers behave.

Fewer errors, even if you have to go looking for them in debug output, are easier to solve than more errors. That's why the default behavior is as it stands.

[getify]

I would also say that this feature request tracking (a per-chain callback) is not really fundamentally different than what I suggested earlier about overriding console.log. In both cases, you the developer have to, by virtue of good engineering practice, know what you're looking for and install the right listeners for it. In neither case will you just happen to find errors that you didn't know about.

This feature request makes it slightly more direct and graceful to detect them, and perhaps provide you an opportunity to "recover", but you would still have had to read the documentation thoroughly to even understand the implications of why such a thing would be necessary. There just is no clean way to make the default behavior be noisy.

[scriby]

If the default behavior was to print something to the console, I think you would be on to something.

We're talking about development here, where we can't expect perfect error handling (maybe we should never expect it...). We need to iterate on the app, find and fix problems quickly.

Getting some indication an error occurred is critical, even if it's not perfect. Things not working and no console output is even more head scratching confusing.

And the first error reported would be the one worth looking at ... which is what I think everyone is used to when debugging.

[getify]

It's a fair point that if you are using the debug-build of LABjs, perhaps the default then should be to have debug mode on. I'm willing to agree to the sense in that argument.

I'm not sure if that would have helped in your case? I provide a debug version of LABjs, with this debug mode functionality in it, because I hope that most people will use it by default, in their dev environment. By contrast, I hope that people DON'T use that build in production, at least not by regular practice, because they're not only using a larger version of the code unnecessarily, but I also think it's bad practice to "leak" implementation data to the console in production environments.

In this scenario, I would have suggested, were I consulted, to first switch to using debug mode, and see what LABjs says. If you see errors, fix them. If not, you've eliminated anything that LABjs is doing from the equation.

But we're back to a fundamental assumption from earlier in this thread: you have to have good practice, and know about the tools you're using, before you'll see what you hoped you would see.

[scriby]

Yeah, the fundamental problem was that I didn't know about LAB's behavior. I would guess most people adopting the library wouldn't be aware of it.

Now that I know about the debug version, we can make that work.

[jasonhinkle]

I don't understand the problem with adding a callback so that devs can choose to listen for errors or not? I can appreciate a separate debug build for outputting debug info, but even on the production build i'd want to know about errors. (though I wouldn't want the library writing directly to console.error)

Obviously you've put way more thought into it than I have so perhaps I'm missing something.

[scriby]

I ended up forking and adding an extension so it can call a method on error...

On May 29, 2012, at 8:31 PM, jasonhinkle reply@reply.github.com wrote:

I don't understand the problem with adding a callback so that devs can choose to listen for errors or not? I can appreciate a separate debug build for outputting debug info, but even on the production build i'd want to know about errors. (though I wouldn't want the library writing directly to console.error)

Obviously you've put way more thought into it than I have so perhaps I'm missing something.

---

Reply to this email directly or view it on GitHub: https://github.com/getify/LABjs/issues/57#issuecomment-5999559

[getify]

@jasonhinkle

The big discussion in this thread is not whether or not it's "possible" to fire an error callback... it was about whether it's proper to do so. My feeling is a library like LABjs should not expose a functionality as part of the API if it cannot expose that functionality in a reliable way across browsers.

Not all browsers have the same behavior for "error" conditions, which means in some browsers an error handler would fire, and in other cases the error handle would sit silent (even though the error did occur). Moreover, some browsers would abort a chain at the point of an error, whereas other browsers the "abort" of a chain would not be possible. So, in some of your browser testings, you'd see an error, but the chain would complete still (which could be a good or bad thing, depending on your code and how much it's inter-dependent), and in other browser testings the error would abort the chain (which again might be desired or not).

By me not being able to guarantee the behavior of these error handling features, I think it detracts from LABjs rather than adds to it. Consider that most people who use LABjs don't understand all the internal quirks, and so if they see a feature documented, and try to use it and it doesn't work as expected, they will file bugs which I'll constantly be closing as "can't fix".

[scriby]

As a user of the library, here's what I think is the most reasonable behavior.

1. Expose an "event" on lab which is fired when an error occurs (from your catch block). Allow client code to register handlers for the event.
2. If no handlers are registered, print the error to console.error with some help text about how to use the event. (Testing if console.error exists first)

This prevents users of your library from wasting debugging time on mysterious problems where no issues are apparent. It also allows the app to handle errors however it normally does, such as logging them somewhere.

In the majority of cases I've seen, if an error occurs in a js file on load, the page is broken anyway. It doesn't matter what your library does as long as it surfaces the information that there was some problem and how to get more info.

Simply swallowing errors is in my opinion the worst default behavior as it makes it practically impossible to debug. Any solution that can at least surface the fact that an error occurred is an improvement.

Chris

On May 29, 2012, at 9:39 PM, Kyle Simpson reply@reply.github.com wrote:

@jasonhinkle

The big discussion in this thread is not whether or not it's "possible" to fire an error callback... it was about whether it's proper to do so. My feeling is a library like LABjs should not expose a functionality as part of the API if it cannot expose that functionality in a reliable way across browsers.

Not all browsers have the same behavior for "error" conditions, which means in some browsers an error handler would fire, and in other cases the error handle would sit silent (even though the error did occur). Moreover, some browsers would abort a chain at the point of an error, whereas other browsers the "abort" of a chain would not be possible. So, in some of your browser testings, you'd see an error, but the chain would complete still (which could be a good or bad thing, depending on your code and how much it's inter-dependent), and in other browser testings the error would abort the chain (which again might be desired or not).

By me not being able to guarantee the behavior of these error handling features, I think it detracts from LABjs rather than adds to it. Consider that most people who use LABjs don't understand all the internal quirks, and so if they see a feature documented, and try to use it and it doesn't work as expected, they will file bugs which I'll constantly be closing as "can't fix".

---

Reply to this email directly or view it on GitHub: https://github.com/getify/LABjs/issues/57#issuecomment-6000369

[jasonhinkle]

Thanks for the response. I added the following which works great for my purposes. All that I really care about is whether an error occurred. I'd prefer if execution halts at that point but from what you say that's not guaranteed depending on what browser you're working with. I wouldn't see this as a way of actually "handling" errors in order to keep the app in a known state - if this fires I'd assume everything is fubar, but at least I can log it or during development just visually see there was a problem. A try/catch that swallows the error seems dangerous to me and makes it a lot tougher during development.

I'd be interested to know if you think this changes LAB.js functionality in a negative way? From what I can see it has no impact on anything unless you implement the ErrorHandler function.

https://github.com/jasonhinkle/LABjs/blob/master/LAB.src.js (relevant line 346)

In my application code I put the following:

$LAB.setGlobalDefaults({'ErrorHandler': function(err){
    throw err;
}});

Obviously this is a drastic example of a handler, but for development on Chrome it's exactly what I want. Are you saying that on other browsers this would not work at all - never get fired? Or are you just worried that people will use something like this to try to recover from errors - when that is not reliable due to browser differences..? Or that they will use it and it'll result in a bunch of false error reports for LABjs?

[getify]

@scriby

That sounds like a reasonable expectation, until you dig a little deeper into its implications.

1. As we have mentioned earlier in the thread, in some cases this error event will fire once (because the chain aborts at first error), and in some browsers the error event may fire many, many, many times, because the chain doesn't abort and keeps going, and now you probably have lots of "reference undefined" issues. This inconsistency cross-browser is almost certain to be a "hidden" (unexpected) behavior, one which I simply cannot correct for (if I could, we wouldn't be having this discussion). LABjs will get blamed for that unexpected behavior just like you blame LABjs for the unexpected error swallowing it currently does. This is a no-win from my perspective.
2. The very definition of what constitutes an "error" is not easy to make obvious. script.onerror (which most people would mentally closely associate with a chain-specific error handler) fires for network loading issues (404's, etc). However, all browsers have somewhat different behavior on which loading errors they fire for or not. So, in some browsers you'll get an error event triggered for a 404 load, and in other browsers, you'll get no error fired at all, even with the 404. This is yet another unexpected inconsistency that's VERY hard to make obvious, or more to the point, make it so that people don't blame LABjs.
3. Moreover, the browser's built-in script.onerror does NOT fire for run-time errors (that's window.onerror), so this LABjs error handler would be (I think somewhat confusingly) combining the behavior of window.onerror and script.onerror into one handler. Documentation can say that, but again, I don't think that's naturally what everyone will assume.

And if we don't try to respond to any of the network load errors at all, and only respond to the run-time errors we catch, then a failed load of a library won't be directly detectable (which is at least a major part of what people asking for these features want to detect for), and will only be indirectly obvious if it happens to cause an "undefined reference" error in the chain.

What if the "undefined reference" call doesn't happen in the chain at all, but not until later? Then you didn't get any indication by LABjs that a problem occured with your chain.

So, you almost certainly have to do both kinds of errors.

Do we combine them into one event, or do we publish two events (loadError, runtimeError)? Even if we do, how do we make it so the above browser inconsistencies don't create more problems than we are solving?

[scriby]

All I can say is that since I added something to lab's catch block to call a registered method, I haven't missed any errors I care about.

Maybe you're overthinking it?

Chris

On May 29, 2012, at 10:36 PM, Kyle Simpson reply@reply.github.com wrote:

@scriby

That sounds like a reasonable expectation, until you dig a little deeper into its implications.

1. As we have mentioned earlier in the thread, in some cases this error event will fire once (because the chain aborts at first error), and in some browsers the error event may fire many, many, many times, because the chain doesn't abort and keeps going, and now you probably have lots of "reference undefined" issues. This inconsistency cross-browser is almost certain to be a "hidden" (unexpected) behavior, one which I simply cannot correct for (if I could, we wouldn't be having this discussion).
2. The very definition of what constitutes an "error" is not easy to make obvious. script.onerror (which most people would mentally closely associate with a chain-specific error handler) fires for network loading issues (404's, etc). However, all browsers have somewhat different behavior on which loading errors they fire for or not. So, in some browsers you'll get an error event triggered for a 404 load, and in other browsers, you'll get no error fired at all, even with the 404. This is yet another unexpected inconsistency that's VERY hard to make obvious, or more to the point, make it so that people don't blame LABjs.
3. Moreover, the browser's built-in script.onerror does NOT fire for run-time errors (that's window.onerror), so this LABjs error handler would be (I think somewhat confusingly) combining the behavior of window.onerror and script.onerror into one handler. Documentation can say that, but again, I don't think that's naturally what everyone will assume.

And if we don't try to respond to any of the network load errors at all, and only respond to the run-time errors we catch, then a failed load of a library won't be directly detectable (which is at least a major part of what people asking for these features want to detect for), and will only be indirectly obvious if it happens to cause an "undefined reference" error in the chain.

What if the "undefined reference" call doesn't happen in the chain at all, but not until later? Then you didn't get any indication by LABjs that a problem occured with your chain.

So, you almost certainly have to do both kinds of errors.

Do we combine them into one event, or do we publish two events (loadError, runtimeError)? Even if we do, how do we make it so the above browser inconsistencies don't create more problems than we are solving?

---

Reply to this email directly or view it on GitHub: https://github.com/getify/LABjs/issues/57#issuecomment-6000984

[jasonhinkle]

I think all of the reasons you mention are absolutely legit, but they are all lesser evils than masking errors. if you implement it as a handler, then it's not really LABjs's problem to deal with the different browser behaviors.

If it's mainly about avoiding people blaming LABjs for bugs in their own code - I think good naming of the handler and instructions for it's purpose would help to let people know it is meant only as notification only - and not for handling and recovering from errors.

[ghost]

Looks like there's a handy little fork here https://github.com/jasonhinkle/LABjs-eh

I hope that this can be added as an option in the future.

[getify]

https://github.com/getify/LABjs/commit/001781117ddefb1fea7388479d3c614e523ac880