878458 - improve error handling behavior of telemetry ping tests

Status: RESOLVED FIXED

(Toolkit :: Telemetry, defect)

Comment 1

[Nathan Froyd [:froydnj]]

Attachment: improve error handling behavior of telemetry ping tests

Comment 2

[Nathan Froyd [:froydnj]]

Comment on attachment 756998 [details] [diff] [review]
improve error handling behavior of telemetry ping tests

This patch fixes a few instances of bad error-handling behavior during tests: since a lot of tests are run asynchronously, there's no top-level error handling, so errors disappear into the void and the test hangs.

To remedy this, add error handling in a couple of places where it seems convenient.  This doesn't fix everything, as errors inside TelemetryPing (especially when sending XHR bits and running on* handlers defined on those requests) can still hang tests, but this is an improvement on the status quo.

Comment 3

[Vladan Djeric (:vladan)]

Comment on attachment 756998 [details] [diff] [review]
improve error handling behavior of telemetry ping tests

>+function wrapWithExceptionHandler(f) {
>+  function wrapper() {
>+    try {
>+      f.apply(null, arguments);
>+    } catch (ex if typeof(ex) == 'object') {
>+      dump("Caught exception: " + ex.message + "\n");
>+      dump(ex.stack);
>+      do_test_finished();
>+    }
>+  }
>+  return wrapper;
>+}

Why don't we catch non-object exceptions here? e.g. "throw Components.results.NS_ERROR_NOT_IMPLEMENTED"

Comment 4

[Nathan Froyd [:froydnj]]

(In reply to Vladan Djeric (:vladan) from comment #3)
> Comment on attachment 756998 [details] [diff] [review]
> improve error handling behavior of telemetry ping tests
> 
> >+function wrapWithExceptionHandler(f) {
> >+  function wrapper() {
> >+    try {
> >+      f.apply(null, arguments);
> >+    } catch (ex if typeof(ex) == 'object') {
> >+      dump("Caught exception: " + ex.message + "\n");
> >+      dump(ex.stack);
> >+      do_test_finished();
> >+    }
> >+  }
> >+  return wrapper;
> >+}
> 
> Why don't we catch non-object exceptions here? e.g. "throw
> Components.results.NS_ERROR_NOT_IMPLEMENTED"

Because such exceptions are thrown by the testing machinery itself when tests fail.  I didn't dig around too much in tracking this down.

http://hg.mozilla.org/integration/mozilla-inbound/rev/4d2d0bc38fec

Comment 5

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/4d2d0bc38fec