Closed Bug 1110691 Opened 5 years ago Closed 5 years ago

The HealthReporter AsyncShutdownTimeout state should include what provider is currently shutting down

Categories

(Firefox Health Report Graveyard :: Client: Desktop, defect)

defect
Not set

Tracking

(firefox35 fixed, firefox36 fixed, firefox37 fixed)

RESOLVED FIXED
Firefox 37
Tracking Status
firefox35 --- fixed
firefox36 --- fixed
firefox37 --- fixed

People

(Reporter: gfritzsche, Assigned: gfritzsche)

References

Details

Attachments

(1 file, 1 obsolete file)

If a health reporter provider is leading to a AsyncShutdownTimeout, we currently can't tell which one caused it from the submitted meta data.
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+
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: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 37
Attachment #8535589 - Flags: approval-mozilla-beta?
Attachment #8535589 - Flags: approval-mozilla-beta+
Attachment #8535589 - Flags: approval-mozilla-aurora?
Attachment #8535589 - Flags: approval-mozilla-aurora+
Product: Firefox Health Report → Firefox Health Report Graveyard
You need to log in before you can comment on or make changes to this bug.