1042929 - Crash in ObsoleteDocumentTracker.getObsoleteIds

Status: RESOLVED FIXED

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

Description

[Richard Newman [:rnewman]]

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.

Comment 1

[Richard Newman [:rnewman]]

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 2

[Richard Newman [:rnewman]]

Attachment: Proposed patch. v1

Comment 3

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8461815 [details] [review]
Proposed patch. v1

lgtm.

Comment 4

[Richard Newman [:rnewman]]

https://hg.mozilla.org/integration/fx-team/rev/c0560ab11ad2

Comment 5

[Richard Newman [:rnewman]]

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.

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/c0560ab11ad2

Comment 7

[Sylvestre Ledru [:Sylvestre]]

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 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/aca05ddaa93f
https://hg.mozilla.org/releases/mozilla-beta/rev/b295f329dfd3

Comment 9

[Sylvestre Ledru [:Sylvestre]]

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.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr31/rev/bb18fa444011

Would be nice if we could get an esr31 tracking flag for this component :)

Comment 11

[Sylvestre Ledru [:Sylvestre]]

Ryan: I reported bug 1048297 for this :)

Comment 12

[Ryan VanderMeulen [:RyanVM]]

[Tracking Requested - why for this release]: Needs to be set to 32+ IIUC