Closed Bug 1042929 Opened 7 years ago Closed 7 years ago
Crash in Obsolete
Document Tracker .get Obsolete Ids
java.lang.Error: Error: could not match input at org.json.simple.parser.Yylex.yylex(Yylex.java:681) at org.json.simple.parser.JSONParser.parse$66bc622e(JSONParser.java:118) at org.mozilla.gecko.sync.ExtendedJSONObject.<init>(ExtendedJSONObject.java:172) at org.mozilla.gecko.sync.ExtendedJSONObject.<init>(ExtendedJSONObject.java:181) at org.mozilla.gecko.sync.ExtendedJSONObject.parseJSONObject(ExtendedJSONObject.java:142) at org.mozilla.gecko.background.healthreport.upload.ObsoleteDocumentTracker.getObsoleteIds(ObsoleteDocumentTracker.java:45) at org.mozilla.gecko.background.healthreport.upload.ObsoleteDocumentTracker.markIdAsUploaded(ObsoleteDocumentTracker.java:196) Reports in Play.
This is entirely because org.json.simple decided that throwing an Exception wasn't enough for a parse error -- it has to be an Error. Oh really? --- An Error is a subclass of Throwable that indicates serious problems that a reasonable application should not try to catch. Most such errors are abnormal conditions. The ThreadDeath error, though a "normal" condition, is also a subclass of Error because most applications should not try to catch it. A method is not required to declare in its throws clause any subclasses of Error that might be thrown during the execution of the method but not caught, since these errors are abnormal conditions that should never occur. That is, Error and its subclasses are regarded as unchecked exceptions for the purposes of compile-time checking of exceptions. --- *shakes fist at stupid programmers*
Comment on attachment 8461815 [details] [review] Proposed patch. v1 lgtm.
Attachment #8461815 - Flags: review?(michael.l.comella) → review+
Comment on attachment 8461815 [details] [review] Proposed patch. v1 Approval Request Comment [Feature/regressing bug #]: Forever. [User impact if declined]: A persistent crash rate on Play. [Describe test coverage new/current, TBPL]: None new. [Risks and why]: Essentially zero: turns an uncaught Error into a catchable Exception, which calling code was already expecting and handles. I would take this on release if we plan a dot release. [String/UUID change made/needed]: None.
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 34
Comment on attachment 8461815 [details] [review] Proposed patch. v1 Setting the approval-mozilla-release flag to have it on our radar in case we make a dot release.
Comment on attachment 8461815 [details] [review] Proposed patch. v1 We are not planning to do a dot release for 31. However, this 31 is going to be an ESR for Armv6 and Android 2.2, taking this patch for our users.
https://hg.mozilla.org/releases/mozilla-esr31/rev/bb18fa444011 Would be nice if we could get an esr31 tracking flag for this component :)
Ryan: I reported bug 1048297 for this :)
[Tracking Requested - why for this release]: Needs to be set to 32+ IIUC
You need to log in before you can comment on or make changes to this bug.