865073 - remote debugging protocol: handlers should systematically catch and log exceptions

Status: RESOLVED FIXED

(DevTools :: Debugger, defect)

Description

[Jim Blandy :jimb]

Attachment: debugging protocol: Systematically catch and log exceptions thrown by handler functions that their callers would otherwise ignore.

It's the established pattern in Firefox for things like the observer service, listener callers, and the like to ignore exceptions thrown by the observer/listener/what-have-you.

(This is kind of like littering, in that's it's a completely avoidable sin, and not even especially satisfying - except that this one can waste hours of someone's time. Not as bad as burglary, but... honestly, people.)

Anyway, another way to look at it is to say that handlers must be infallible. The attached patch makes all the handlers I could find in the remote protocol server infallible, by wrapping them with a function that catches and logs errors. This can then replace the extant one-off catch clauses attempting to address the problem.

Not requesting review yet, as this patch reveals some silent failures. (Whodathunkit!)

Comment 1

[Dave Camp (:dcamp)]

Comment on attachment 741126 [details] [diff] [review]
debugging protocol: Systematically catch and log exceptions thrown by handler functions that their callers would otherwise ignore.

Review of attachment 741126 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/devtools/debugger/dbg-transport.js
@@ +31,5 @@
> + *
> + * (SpiderMonkey does generate good names for anonymous functions, but we
> + * don't have a way to get at them from JavaScript at the moment.)
> + */
> +function MakeInfallible(aHandler, aName) {

Most of our non-constructor functions are camelCase, is this different on purpose?

(also: dbg-transport.js is a weird place for this, but it's the best option for now.  We can fix that soon).

Comment 2

[Jim Blandy :jimb]

(In reply to Dave Camp (:dcamp) from comment #1)
> Most of our non-constructor functions are camelCase, is this different on
> purpose?

No; I've changed it. Thanks for pointing this out.

> (also: dbg-transport.js is a weird place for this, but it's the best option
> for now.  We can fix that soon).

Yeah; it's the only thing shared between dbg-client.jsm and the server-side stuff. It would be nice to have a devtools utilities file... although those always turn into junk drawers, or grow more structure than they ought.

Comment 3

[Jim Blandy :jimb]

https://hg.mozilla.org/integration/mozilla-inbound/rev/77ca8f2074c9

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/77ca8f2074c9