On 8/31/2015 3:34 PM, Nicholas Alexander wrote:
> Over in a review [1] for Bug 1191067, sebastian asks the following
> excellent question:
>
> ::: mobile/android/base/AccountsHelper.java:127
> (Diff revision 1)
> > + } catch (Exception e) {
>
> This is not related to these reviews and patches or this line in
> particular but I've seen us catching generic exceptions in various
> places. Is this to ensure the callback is triggered at all costs?
> I'm always afraid that this will silently catch all kinds of
> unchecked exceptions (e.g. NullPointerException,
> ClassCastException, ..) in later iterations. Do we have any
> guidelines in what situations it's okay to do this? Most projects
> seem to discourage you from doing this.
>
>
> I'll answer from my own perspective, but I'd like to get more comments.
>
> For some context, this code is handling a message from
> chrome-privileged JavaScript to Java. The message is expected to be
> triggered from an add-on -- i.e., I don't control the inputs
> entirely. In such contexts -- essentially, message handling -- I
> never want to fail. Given how hard it can be to handle unchecked
> Exception-sub classes, and how hard it can be to sanitize input
> completely, I err on the side of caution. It would be nice to
> differentiate "routine mistake" NPEs from "bad input, need more null
> check" NPEs but I often am not fully confident that I haven't made an
> error. In this situation, I blanket catch Exception for safety.
>
> I am not aware of guidelines, expectations, or a style guide for
> Fennec. What do others do? What do others think we should recommend?
>
At my last employer, catching all exceptions was considered a bad thing.
Granted, that was providing a service, so failing out meant, at worst,
the remote connection had to be re-established. I'll agree that eating
exceptions here is probably not a bad thing since killing the browser
would not be a good user experience.
In any case, "logging the hell out of it" should be the right action for
this catch. We should never silently eat exceptions (unless there's a
privacy or vetted security reason to do so and there's no way to
sanitize the event).
> Thanks, sebastian, for an excellent question!
> Nick
>
> [1] https://bugzilla.mozilla.org/show_bug.cgi?id=1191067#c25
>
>
> _______________________________________________
> mobile-firefox-dev mailing list
> [email protected]
> https://mail.mozilla.org/listinfo/mobile-firefox-dev
_______________________________________________
mobile-firefox-dev mailing list
[email protected]
https://mail.mozilla.org/listinfo/mobile-firefox-dev
