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

Status: RESOLVED FIXED

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

Description

[Georg Fritzsche [:gfritzsche]]

If a health reporter provider is leading to a AsyncShutdownTimeout, we currently can't tell which one caused it from the submitted meta data.

Comment 1

[Georg Fritzsche [:gfritzsche]]

Attachment: Add provider shutdown info to AsyncShutdownTimeout state

Just a simple forensics addition - Yoric, do you feel ok with reviewing this?

Comment 2

[David Teller [:Yoric] - still alive but not very active]

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`")?

Comment 3

[Georg Fritzsche [:gfritzsche]]

Attachment: Add provider shutdown info to AsyncShutdownTimeout state

https://hg.mozilla.org/integration/mozilla-inbound/rev/4cab36024fa3

Comment 4

[Georg Fritzsche [:gfritzsche]]

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.

Comment 5

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

https://hg.mozilla.org/mozilla-central/rev/4cab36024fa3

Comment 6

[Georg Fritzsche [:gfritzsche]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/ae5acfeeca69
https://hg.mozilla.org/releases/mozilla-beta/rev/be51af9dc8dc