Closed
Bug 1052158
Opened 10 years ago
Closed 10 years ago
Determine how to handle GeckoRequest error callbacks
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 36
People
(Reporter: bnicholson, Assigned: bnicholson)
References
Details
Attachments
(1 file)
7.23 KB,
patch
|
wesj
:
review+
|
Details | Diff | Splinter Review |
Right now, errors that occur in JS during a GeckoRequest can be handled by catching the exception on the JS side, then responding to the GeckoRequest with any error data in the callback object. We should consider whether we want to use onError for this. We do have a GeckoRequest#onError callback already, but we advise against using it in the comments because the origin of the error is unclear in Java.
Assignee | ||
Comment 1•10 years ago
|
||
Looking at bug 1097098, it's impossible to figure out where we threw in Gecko, making it difficult to fix bugs. We should forward the error message/stack to use in the RuntimeException. With this patch, errors will now look like this: E/GeckoCrashHandler( 7383): >>> REPORTING UNCAUGHT EXCEPTION FROM THREAD 72678 ("Gecko") E/GeckoCrashHandler( 7383): java.lang.RuntimeException: Unhandled error for GeckoRequest Robocop:GeckoRequestException: error! E/GeckoCrashHandler( 7383): JS stack: E/GeckoCrashHandler( 7383): add_exception_listener/<@testGeckoRequest.js:18:11 E/GeckoCrashHandler( 7383): requestHandler.observe<@resource://gre/modules/Messaging.jsm:147:28 E/GeckoCrashHandler( 7383): TaskImpl_run@resource://gre/modules/Task.jsm:314:40 E/GeckoCrashHandler( 7383): TaskImpl@resource://gre/modules/Task.jsm:275:3 E/GeckoCrashHandler( 7383): createAsyncFunction/asyncFunction@resource://gre/modules/Task.jsm:249:14 E/GeckoCrashHandler( 7383): E/GeckoCrashHandler( 7383): at org.mozilla.gecko.util.GeckoRequest.onError(GeckoRequest.java:89) E/GeckoCrashHandler( 7383): at org.mozilla.gecko.GeckoAppShell$3.handleMessage(GeckoAppShell.java:438) E/GeckoCrashHandler( 7383): at org.mozilla.gecko.EventDispatcher.dispatchEvent(EventDispatcher.java:168) E/GeckoCrashHandler( 7383): at org.mozilla.gecko.GeckoAppShell.handleGeckoMessage(GeckoAppShell.java:2260) E/GeckoCrashHandler( 7383): at org.mozilla.gecko.mozglue.GeckoLoader.nativeRun(Native Method) E/GeckoCrashHandler( 7383): at org.mozilla.gecko.mozglue.GeckoLoader.nativeRun(Native Method) E/GeckoCrashHandler( 7383): at org.mozilla.gecko.mozglue.GeckoLoader.nativeRun(Native Method) E/GeckoCrashHandler( 7383): at org.mozilla.gecko.GeckoAppShell.runGecko(GeckoAppShell.java:371) E/GeckoCrashHandler( 7383): at org.mozilla.gecko.GeckoThread.run(GeckoThread.java:190)
Comment 2•10 years ago
|
||
Nice!
Comment 3•10 years ago
|
||
Comment on attachment 8522559 [details] [diff] [review] Forward the GeckoRequest error stack to Java Review of attachment 8522559 [details] [diff] [review]: ----------------------------------------------------------------- I think you can do this a little more resiliently (so that you didn't have to rewrite your test). What happens if you "throw 'Error'" with this code? I guess you just get nothing in the Java stack? ::: mobile/android/modules/Messaging.jsm @@ +158,5 @@ > > + Messaging.sendRequest({ > + type: "Gecko:Request" + wrapper.id, > + error: { > + message: e.message, I would probably do e.message || e to try and catch things like throw "Error" @@ +159,5 @@ > + Messaging.sendRequest({ > + type: "Gecko:Request" + wrapper.id, > + error: { > + message: e.message, > + stack: e.stack Building the stack using Components.stack would also make this work even if the error wasn't and Error-object. i.e. something like: var stackString = ""; var s = Components.stack while (s) { stackString += s.formattedStack + "\n" s = s.caller; }
Attachment #8522559 -
Flags: review?(wjohnston) → review+
Assignee | ||
Comment 4•10 years ago
|
||
(In reply to Wesley Johnston (:wesj) from comment #3) > I would probably do e.message || e to try and catch things like throw "Error" Went a step further and changed this to "e.message || (e && e.toString()" in case the user does something like "throw true" (which would previously fail at the optString call in Java). > Building the stack using Components.stack would also make this work even if > the error wasn't and Error-object Simply using "Components.stack.formattedStack" seems to give us a stack, so I made this change. But I'm not sure how useful it is; the stack will always end up looking like this in such situations: requestHandler.observe<@resource://gre/modules/Messaging.jsm:163:10 TaskImpl_run@resource://gre/modules/Task.jsm:314:39 TaskImpl@resource://gre/modules/Task.jsm:275:2 createAsyncFunction/asyncFunction@resource://gre/modules/Task.jsm:249:13 https://hg.mozilla.org/integration/fx-team/rev/1b946e43e5dc
Comment 5•10 years ago
|
||
You might be interested in: http://mxr.mozilla.org/mozilla-central/source/toolkit/modules/Log.jsm#164
Comment 6•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/1b946e43e5dc
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 36
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
•