Closed Bug 1042929 Opened 7 years ago Closed 7 years ago

Crash in ObsoleteDocumentTracker.getObsoleteIds

Categories

(Android Background Services Graveyard :: Firefox Health Report Service, defect)

Firefox 31
All
Android
defect
Not set
critical

Tracking

(firefox31 wontfix, firefox32 fixed, firefox33 fixed, firefox34 fixed, firefox-esr3132+ fixed)

RESOLVED FIXED
Firefox 34
Tracking Status
firefox31 --- wontfix
firefox32 --- fixed
firefox33 --- fixed
firefox34 --- fixed
firefox-esr31 32+ fixed

People

(Reporter: rnewman, Assigned: rnewman)

Details

(Keywords: crash)

Attachments

(1 file)

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*
Attached file Proposed patch. v1
Attachment #8461815 - Flags: review?(michael.l.comella)
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.
Attachment #8461815 - Flags: approval-mozilla-beta?
Attachment #8461815 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/c0560ab11ad2
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.
Attachment #8461815 - Flags: approval-mozilla-release?
Attachment #8461815 - Flags: approval-mozilla-beta?
Attachment #8461815 - Flags: approval-mozilla-beta+
Attachment #8461815 - Flags: approval-mozilla-aurora?
Attachment #8461815 - Flags: approval-mozilla-aurora+
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.
Attachment #8461815 - Flags: approval-mozilla-release?
Attachment #8461815 - Flags: approval-mozilla-release-
Attachment #8461815 - Flags: approval-mozilla-esr31+
https://hg.mozilla.org/releases/mozilla-esr31/rev/bb18fa444011

Would be nice if we could get an esr31 tracking flag for this component :)
Whiteboard: [status-esr31:fixed]
Ryan: I reported bug 1048297 for this :)
[Tracking Requested - why for this release]: Needs to be set to 32+ IIUC
Whiteboard: [status-esr31:fixed]
You need to log in before you can comment on or make changes to this bug.