Closed
Bug 739418
Opened 12 years ago
Closed 12 years ago
If a rethrown exception is uncaught, report the stack trace of the original exception, not the rethrowing exception
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(firefox13 fixed, firefox14 fixed, firefox-esr1012+ fixed)
RESOLVED
FIXED
Firefox 14
People
(Reporter: cpeterson, Assigned: cpeterson)
References
Details
Attachments
(1 file, 1 obsolete file)
1.70 KB,
patch
|
blassey
:
review+
akeybl
:
approval-mozilla-aurora+
akeybl
:
approval-mozilla-esr10+
|
Details | Diff | Splinter Review |
A rethrown exception's stack trace lists the rethrowing exception before the original exception (the `cause`). When reviewing crash reports in Socorro, unrelated crashes may look like dupes because their stack traces point to the same rethrowing error handler. If we move the original exception to the top of the crash report, Socorro can correctly collate related crash reports. Examples of unrelated bugs that may look like dupes: bug 734624, bug 738977, and bug 738976.
Assignee | ||
Comment 1•12 years ago
|
||
When reporting a rethrown exception, report the original exception's stack trace.
Attachment #609499 -
Flags: review?(blassey.bugs)
Comment 2•12 years ago
|
||
Comment on attachment 609499 [details] [diff] [review] bug-739418-report-rethrown-exceptions.patch Review of attachment 609499 [details] [diff] [review]: ----------------------------------------------------------------- ::: mobile/android/base/GeckoAppShell.java @@ +161,5 @@ > public void uncaughtException(Thread thread, Throwable e) { > + String stackTrace = getStackTraceString(e); > + > + Throwable cause = e.getCause(); > + if (cause != null) { should this be while(cause != null)? I'd do it this way: Throwable cause; while (null != (cause = e.getCause())) e = cause; reportJavaCrash(getStackTraceString(e));
Attachment #609499 -
Flags: review?(blassey.bugs) → review-
Assignee | ||
Comment 3•12 years ago
|
||
If the uncaught exception was rethrown, walk the exception `cause` chain to find the original exception.
Attachment #609499 -
Attachment is obsolete: true
Attachment #609816 -
Flags: review?(blassey.bugs)
Comment 4•12 years ago
|
||
Comment on attachment 609816 [details] [diff] [review] bug-739418-report-rethrown-exceptions-v2.patch Review of attachment 609816 [details] [diff] [review]: ----------------------------------------------------------------- ::: mobile/android/base/GeckoAppShell.java @@ +169,2 @@ > Log.e(LOGTAG, ">>> REPORTING UNCAUGHT EXCEPTION FROM THREAD " > + thread.getId() + " (\"" + thread.getName() + "\")", e); its probably worth moving this logging above the walking since it will include all the stacks
Attachment #609816 -
Flags: review?(blassey.bugs) → review+
Assignee | ||
Comment 5•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/1dd339df6a7c
Status: NEW → ASSIGNED
Flags: in-testsuite-
Target Milestone: --- → Firefox 14
Assignee | ||
Comment 6•12 years ago
|
||
Comment on attachment 609816 [details] [diff] [review] bug-739418-report-rethrown-exceptions-v2.patch [Approval Request Comment] Regression caused by (bug #): N/A User impact if declined: Socorro crash reports will be harder to diagnose. Testing completed (on m-c, etc.): Risk to taking this patch (and alternatives if risky): Low. Java only code that is only executed when reporting an uncaught exception. String changes made by this patch: None
Attachment #609816 -
Flags: approval-mozilla-aurora?
Assignee | ||
Comment 7•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/1dd339df6a7c
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Comment 8•12 years ago
|
||
Comment on attachment 609816 [details] [diff] [review] bug-739418-report-rethrown-exceptions-v2.patch [Triage Comment] Low risk, mobile-only fix. Approved for Aurora 13.
Attachment #609816 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Assignee | ||
Comment 9•12 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/5c0f104db5e9
Assignee | ||
Comment 10•12 years ago
|
||
Comment on attachment 609816 [details] [diff] [review] bug-739418-report-rethrown-exceptions-v2.patch If we are going to land bug 701002 to improve Java crash reporting on ESR10, then I recommend also landing this patch to make Java crash reports easier to collate in Socorro.
Attachment #609816 -
Flags: approval-mozilla-esr10?
Assignee | ||
Updated•12 years ago
|
tracking-firefox-esr10:
--- → ?
Updated•12 years ago
|
Updated•12 years ago
|
status-firefox13:
--- → fixed
status-firefox14:
--- → fixed
Comment 11•12 years ago
|
||
Comment on attachment 609816 [details] [diff] [review] bug-739418-report-rethrown-exceptions-v2.patch [Triage Comment] Approved for ESR10 in support of crash killing. This has already had time to bake on Nightly/Aurora.
Attachment #609816 -
Flags: approval-mozilla-esr10? → approval-mozilla-esr10+
Assignee | ||
Comment 12•12 years ago
|
||
https://hg.mozilla.org/releases/mozilla-esr10/rev/4246cbfe8c4b
Updated•12 years ago
|
Updated•3 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•