Failing Silently vs. Failing noisily

[Martin Häcker]

Hi all,

I'd like to propose that Sproutcore internally adopts a policy of
failing fast and noisily instead of just silently ignoring wrong calls.

This would make debugging and finding such calls so much easier.

Some examples where I encountered this in the past:

SC.ListView caching - invalid caches where just silently corrupted /
rebuilt.

SC.View, calling .design() multiple times just ignores the other calls

sc_super() when a super method was already set on a method it was just
overwritten

There's probably more but those I remember right now.

I think that is something that should be part of the coding style for
SproutCore to make it explicit and clear also for all contributors from
outside of strobe.

Regards,
Martin

[Yehuda Katz]

I agree. Tom and I have already started to adopt this policy in any new code we have written (see, for instance, our change to sc_super calls to superclasses without the method).

Yehuda Katz
Chief Technologist | Strobe
(ph) 718.877.1325

[Alex Iskander]

Agreed.

Whenever it takes extra code to do this kind of checking, I think we should wrap it in:

// @if (debug) 

...

// @endif

To keep it from affecting build size.

Alex

[Yehuda Katz]

One additional benefit of @if (debug) is that failing loudly is probably NOT what you want in production (it can cause apps to totally fail when they would otherwise limp along).

Yehuda Katz
Chief Technologist | Strobe
(ph) 718.877.1325

[Tom Dale]

This is bad because it can leave the application in a bad state (e.g., unable to persist data to the server) but the user may continue assuming their work will be saved.

[Yehuda Katz]

On the other hand, if we adopt a policy of failing noisily, we may be over-aggressively failing to help people in development, and accidentally hosing apps for no reason in production. Thoughts?

Yehuda Katz
Chief Technologist | Strobe
(ph) 718.877.1325

[Alex Iskander]

I think there are levels. I think we should fail not just loudly, but easily. 

Anywhere we can put error checking that could help in development, we should perhaps consider doing so. This would help debugging a lot, but would slow down the app and potentially increase its size greatly. Having @if(debug) lets us not worry too much.

Alex

[Chad Eubanks (gMail)]

Would it be feasible to have a development and production mode in our apps?  When in development mode the app would fail loudly and when one runs sc-build it switches to production mode therefore failing silently.

Banks

Sent from my iPhone

[Yehuda Katz]

That's exactly what @if(debug) would do. The concern that Tom raised was that failing in production might be necessary to avoid the illusion of a working app saving state, when the app was actually broken and not saving state.

Yehuda Katz
Chief Technologist | Strobe
(ph) 718.877.1325

[Topher Fangio]

It seems like a good idea to somehow fail when the production application has trouble, but on the other hand, it seems like this should be something handled by each application. Some applications may want to fail with a very specific error message because their users are smart (or developers themselves), whereas others may just want to say "Sorry for the trouble, we'll get it fixed ASAP".

Perhaps it might be good if developers had a way to trap the errors in production and do something specific for the application at hand.

--

Topher Fangio

System Architect
Pharos Resources

office: 325.216.2908
mobile: 325.660.7141

[Tom Dale]

This is already possible with SC.ExceptionHandler:

https://github.com/sproutcore/sproutcore/blob/master/frameworks/foundation/system/exception_handler.js

I should probably write a guide about it.

[Yehuda Katz]

Confirm,

And maybe we can have production errors allow the app to continue but still trigger SC.ExceptionHandler?

Yehuda Katz
Chief Technologist | Strobe
(ph) 718.877.1325

[Tom Dale]

FWIW I was once using OmniGraffle and it encountered an exception. It displayed an alert telling me that the application was in a bad state and I should try to save my work and quit and relaunch as soon as possible.

I'm not sure how elegant that is, but at least there's precedent.

Tom

[Yehuda Katz]

It seems to me that there are a few kinds of cases:

• Development mode: you want noisy errors as much and as often as possible.
• Production mode, fatal error: If an error will cause unexpected behavior vis a vis the server, you want a warning that something serious has happened. In most cases, the user will want to reload. This is the equivalent of the current white-screen of death. At the very least, we should make the white-screen much prettier.
• Production mode, non-fatal error: Some errors are not fatal and the user can continue operation. The trick is figuring out what kind of errors these are. Do we have any cases where we know for sure that saving to the server will not be impacted? My gut says no, but I'm unsure.

-- Yehuda

Yehuda Katz
Chief Technologist | Strobe
(ph) 718.877.1325

[Erich Ocean]

My opinion is that uncaught exceptions in production mode should bring
up an Omni Group-style handler and have a restart button, with an
optional dialogue box with allowing the user to provide info (what
happened?) and a hook in the app to send error info back to the
server. Then the page should reload.

The hook will allow devs to gather information (statechart info is
particularly nice). What we should not do is silently catch an
unanticipated error and continue.

Best,

Erich Ocean
SproutCore Training and Mentoring
http://erichocean.com/training/index.html
http://erichocean.com/mentoring/index.html

[jtaby]

On that same note though, what if your app failed for a trivial reason
or because we're trying to encourage best-practices, but now the fact
that the app is essentially "frozen" means the user can't take an
essential action? e.g. if a list view threw an error, you won't be
able to hit "save" in your main app, hence losing data.

Barring that, I'm fine with where this discussion is headed.

On Mar 26, 5:38 pm, Erich Ocean <ad...@erichocean.otherinbox.com>
wrote:

> My opinion is that uncaught exceptions in production mode should bring
> up an Omni Group-style handler and have a restart button, with an
> optional dialogue box with allowing the user to provide info (what
> happened?) and a hook in the app to send error info back to the
> server. Then the page should reload.
>
> The hook will allow devs to gather information (statechart info is
> particularly nice). What we should not do is silently catch an
> unanticipated error and continue.
>
> Best,
>
> Erich Ocean

> SproutCore Training and Mentoringhttp://erichocean.com/training/index.htmlhttp://erichocean.com/mentoring/index.html

> >>>https://github.com/sproutcore/sproutcore/blob/master/frameworks/found...

[Geoffrey Donaldson]

Well, then I don't think your app should be in production if it has list views throwing errors ;)

You should have a handler that catches the state of the app, tells the server, apologises to the user, and offer a restart link.

If your want to write some smarts into your error handler to figure out if you can keep going, then awesome, but default should just error.

Geoffrey D.

[Tim]

We should consider the fact that failing nosily is all fine and good, but it's useless if what SproutCore says makes absolutely no sense. To really give 100% on this, I believe that the error messages that are thrown need to be improved (especially in the datastore layer).

I'm on the bandwagon of throw as soon as possible, as noisily as possible and as informative as possible. This way, devs can get 90% of the bugs out of the way themselves through testing and basic user testing, then have a more graceful interface for introducing errors to the user, apologizing, and giving the user options to: continue with a buggy app or restart the app. (FYI- this is currently our plan for the app that we're making).

We should not turn off errors and exceptions in production, as this will cause countless headaches. Rule of thumb: no product is perfect and you'll always throw an error in some way or another. We can minimize this from happening by providing recovery mechanisms for devs to use, etc. (much like Yehuda's categorization of errors)

For the most part, I'm in agreement with the direction this thread is taking. Just wanted to make sure that we don't get any embarrassing "Internal barf inconsistency" shown to our users and sent back to us as one of the most annoying and uninformative errors in SC.

Cheers

Tim