Closed
Bug 1110691
Opened 9 years ago
Closed 9 years ago
The HealthReporter AsyncShutdownTimeout state should include what provider is currently shutting down
Categories
(Firefox Health Report Graveyard :: Client: Desktop, defect)
Firefox Health Report Graveyard
Client: Desktop
Tracking
(firefox35 fixed, firefox36 fixed, firefox37 fixed)
RESOLVED
FIXED
Firefox 37
People
(Reporter: gfritzsche, Assigned: gfritzsche)
References
Details
Attachments
(1 file, 1 obsolete file)
3.38 KB,
patch
|
gfritzsche
:
review+
lsblakk
:
approval-mozilla-aurora+
lsblakk
:
approval-mozilla-beta+
gfritzsche
:
checkin+
|
Details | Diff | Splinter Review |
If a health reporter provider is leading to a AsyncShutdownTimeout, we currently can't tell which one caused it from the submitted meta data.
Assignee | ||
Comment 1•9 years ago
|
||
Just a simple forensics addition - Yoric, do you feel ok with reviewing this?
Assignee: nobody → georg.fritzsche
Status: NEW → ASSIGNED
Attachment #8535555 -
Flags: review?(dteller)
Comment on attachment 8535555 [details] [diff] [review] Add provider shutdown info to AsyncShutdownTimeout state Review of attachment 8535555 [details] [diff] [review]: ----------------------------------------------------------------- ::: services/healthreport/healthreporter.jsm @@ +613,5 @@ > this._log.info("Shutting down provider manager."); > for (let provider of this._providerManager.providers) { > try { > + this._log.info("Shutting down provider: " + provider.name); > + this._currentProviderInShutdown = provider.name; Could you declare `_currentProviderInShutdown` in the constructor and take the opportunity to document that field (e.g. "If we are in the process of shutting down providers, the name of the provider being shutdown, otherwise `null`")?
Attachment #8535555 -
Flags: review?(dteller) → review+
Assignee | ||
Comment 3•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/4cab36024fa3
Attachment #8535555 -
Attachment is obsolete: true
Attachment #8535589 -
Flags: review+
Attachment #8535589 -
Flags: checkin+
Assignee | ||
Comment 4•9 years ago
|
||
Comment on attachment 8535589 [details] [diff] [review] Add provider shutdown info to AsyncShutdownTimeout state Approval Request Comment [Feature/regressing bug #]: FHR AsyncShutdown [User impact if declined]: In case an FHR provider is blocking shutdown, we currently don't know which from the crash data. I would like to get this uplifted to be able to diagnose things properly. [Describe test coverage new/current, TBPL]: Sufficient test-coverage over FHR code. [Risks and why]: Low-risk - trivial patch that just adds a diagnostic. [String/UUID change made/needed]: None.
Attachment #8535589 -
Flags: approval-mozilla-beta?
Attachment #8535589 -
Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/4cab36024fa3
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 37
Updated•9 years ago
|
Attachment #8535589 -
Flags: approval-mozilla-beta?
Attachment #8535589 -
Flags: approval-mozilla-beta+
Attachment #8535589 -
Flags: approval-mozilla-aurora?
Attachment #8535589 -
Flags: approval-mozilla-aurora+
Assignee | ||
Comment 6•9 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/ae5acfeeca69 https://hg.mozilla.org/releases/mozilla-beta/rev/be51af9dc8dc
Updated•6 years ago
|
Product: Firefox Health Report → Firefox Health Report Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•