Closed
Bug 1386027
Opened 7 years ago
Closed 7 years ago
Crash in java.lang.NullPointerException: Attempt to invoke virtual method ''java.lang.Class java.lang.Object.getClass()'' on a null object reference at org.mozilla.gecko.sync.telemetry.TelemetryCollector.setError(TelemetryCollector.java)
Categories
(Firefox for Android Graveyard :: Android Sync, defect, P1)
Tracking
(relnote-firefox 55+, firefox54 unaffected, firefox55blocking fixed, firefox56 fixed, firefox57 fixed)
RESOLVED
FIXED
Firefox 57
People
(Reporter: calixte, Assigned: Grisha)
References
Details
(Keywords: crash, regression)
Crash Data
Attachments
(3 files)
59 bytes,
text/x-review-board-request
|
rnewman
:
review+
|
Details |
20.63 KB,
patch
|
lizzard
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
20.63 KB,
patch
|
Sylvestre
:
approval-mozilla-release+
|
Details | Diff | Splinter Review |
This bug was filed from the Socorro interface and is report bp-56028983-ddf6-4bfe-9c36-b9fd60170731. ============================================================= There are 5 crashes in nightly 56 with buildids 20170731100320 and 20170730100245. :Grisha Kruglov, could you investigate please ?
Flags: needinfo?(gkruglov)
Assignee | ||
Comment 1•7 years ago
|
||
I also see crashes in 55, which is to be expected, since Bug 1308337 landed in 55.
Blocks: 1308337
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → gkruglov
Status: NEW → ASSIGNED
Flags: needinfo?(gkruglov)
Priority: -- → P1
Comment hidden (mozreview-request) |
Comment 3•7 years ago
|
||
This is the top Android crash in Nightly 20170804100335.
Comment 4•7 years ago
|
||
mozreview-review |
Comment on attachment 8892675 [details] Bug 1386027 - Simplify handleError interfaces for SessionCallback and TelemetryCollector https://reviewboard.mozilla.org/r/163666/#review170956 Consider uplift. ::: mobile/android/services/src/main/java/org/mozilla/gecko/fxa/sync/FxAccountSyncAdapter.java:191 (Diff revision 1) > public void handleAborted(GlobalSession globalSession, String reason) { > super.handleAborted(globalSession, reason); > // Note to future maintainers: while there are reasons, other than 'backoff', this method > // might be called, in practice that _is_ the only reason it gets called at the moment of > // writing this. If this changes, please do expand this telemetry handling. > - this.telemetryCollector.setError(TelemetryCollector.KEY_ERROR_INTERNAL, "backoff"); > + this.telemetryCollector.setError(TelemetryCollector.KEY_ERROR_INTERNAL, new BackoffException()); Why not also pass `reason` here, as on line 179?
Attachment #8892675 -
Flags: review?(rnewman) → review+
Comment hidden (mozreview-request) |
Pushed by gkruglov@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/2b09cdaae71c Simplify handleError interfaces for SessionCallback and TelemetryCollector r=rnewman
Comment 7•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/2b09cdaae71c
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
Assignee | ||
Comment 9•7 years ago
|
||
Yup, I was waiting for a nice soft landing on nightly.
Crash Signature: [@ java.lang.NullPointerException: Attempt to invoke virtual method ''java.lang.Class java.lang.Object.getClass()'' on a null object reference at org.mozilla.gecko.sync.telemetry.TelemetryCollector.setError(TelemetryCollector.java)] → [@ java.lang.NullPointerException: Attempt to invoke virtual method ''java.lang.Class java.lang.Object.getClass()'' on a null object reference at org.mozilla.gecko.sync.telemetry.TelemetryCollector.setError(TelemetryCollector.java)]
[@ java.lang.NullPoin…
Flags: needinfo?(gkruglov)
Assignee | ||
Comment 10•7 years ago
|
||
Approval Request Comment [Feature/Bug causing the regression]: Bug 1308337 or one of its follow-ups [User impact if declined]: Crash happens when sync fails in certain ways [Is this code covered by automated tests?]: Somewhat; there are tests that exercise most of this code, just not under every condition. [Has the fix been verified in Nightly?]: Should be in the next nightly. [Needs manual test from QE? If yes, steps to reproduce]: Not necessary. [List of other uplifts needed for the feature/fix]: N/A [Is the change risky?]: Not really. [Why is the change risky/not risky?]: Patch is straightforward: it simplifies sync telemetry error handling interface, and ensures its consistent use, getting its consumers to stop passing around nulls. [String changes made/needed]: N/A
Attachment #8895422 -
Flags: approval-mozilla-beta?
Comment 11•7 years ago
|
||
Comment on attachment 8895422 [details] [diff] [review] simplify-telemetry-error-handling-beta.patch Fix for crashes from telemetry changes in 55, let's uplift to beta 2. Includes tests.
Attachment #8895422 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Updated•7 years ago
|
Keywords: regression
Comment 12•7 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/2eabe4c12b9a
Flags: in-testsuite+
Comment 13•7 years ago
|
||
Probably too late for 55.
Assignee | ||
Comment 15•7 years ago
|
||
Should I request an uplift to 55 as well? I was under the impression that it was too late; I'm not sure what "it is skipping" means.
Flags: needinfo?(sledru)
Comment 16•7 years ago
|
||
I meant "spiking" :( please fill the uplift request! thanks
Flags: needinfo?(sledru)
Updated•7 years ago
|
Flags: needinfo?(gkruglov)
Updated•7 years ago
|
Assignee | ||
Comment 17•7 years ago
|
||
(same as for beta uplift) [Feature/Bug causing the regression]: Bug 1308337 or one of its follow-ups [User impact if declined]: Crash happens when sync fails in certain ways [Is this code covered by automated tests?]: Somewhat; there are tests that exercise most of this code, just not under every condition. [Has the fix been verified in Nightly?]: No recorded crashes in Nightly after this landed a few days ago, as far as I can tell. [Needs manual test from QE? If yes, steps to reproduce]: Not necessary. [List of other uplifts needed for the feature/fix]: N/A [Is the change risky?]: Not really. [Why is the change risky/not risky?]: Patch is straightforward: it simplifies sync telemetry error handling interface, and ensures its consistent use, getting its consumers to stop passing around nulls. [String changes made/needed]: N/A
Flags: needinfo?(gkruglov)
Attachment #8896420 -
Flags: approval-mozilla-release?
Comment 18•7 years ago
|
||
Comment on attachment 8896420 [details] [diff] [review] simplify-telemetry-error-release.patch Not really happy about the size of the patch but we will use nightly & beta as testing during the week end to see if it is really that safe.
Attachment #8896420 -
Flags: approval-mozilla-release? → approval-mozilla-release+
Comment 19•7 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-release/rev/361d734f9891
Comment 20•7 years ago
|
||
Added to the 55 release notes: Fix a crash with Telemetry
relnote-firefox:
--- → 55+
Updated•7 years ago
|
Product: Android Background Services → Firefox for Android
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
•