846083 - Submit an FHR payload if initialization has failed

Status: RESOLVED FIXED

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

Description

[Gregory Szorc [:gps]]

Currently, if FHR fails to initialize we don't do much. The HealthReporter instance is sitting around but it has no storage and collector. Since the instance isn't initialized, the request to upload is essentially a no-op and no ping is seen. If you figure that init errors are likely to recur, this would manifest as an FHR drop-off over time.

I think it would be a nice feature if we still managed to submit an FHR payload even if initialization failed. The payload would contain an error message so we can help triage what kinds of issues people are having and where we need rigor in the initialization sequence.

Comment 1

[Gregory Szorc [:gps]]

This will likely be much easier once a document handler entity comes into existence. We can just generate a stub document if init fails and hand it off to the document handler.

Comment 2

[Gregory Szorc [:gps]]

Attachment: Submit payload on init failure, v1

I believe this should do it.

This patches really exposes the need for a document handler! The big delta in the middle is due to indentation. At some point we should factor out payload generation into helper functions!

Comment 3

[Richard Newman [:rnewman]]

Comment on attachment 725560 [details] [diff] [review]
Submit payload on init failure, v1

Review of attachment 725560 [details] [diff] [review]:
-----------------------------------------------------------------

I'm happy enough with this.

::: services/healthreport/healthreporter.jsm
@@ +710,1 @@
>      }

This method is getting big :/

@@ +1083,5 @@
> +    // backoff. We should arguably not do this. However, reporting
> +    // startup errors is important. And, they should not occur with much
> +    // frequency in the wild. So, it shouldn't be too big of a deal.
> +    if (!inShutdown && this._policy.ensureNotifyResponse(new Date()) &&
> +        this._policy.healthReportUploadEnabled) {

Three lines, plz.

Comment 4

[Gregory Szorc [:gps]]

https://hg.mozilla.org/services/services-central/rev/74b3075c2d94

Comment 5

[Gregory Szorc [:gps]]

https://hg.mozilla.org/mozilla-central/rev/74b3075c2d94

Comment 6

[Gregory Szorc [:gps]]

Comment on attachment 725560 [details] [diff] [review]
Submit payload on init failure, v1

[Approval Request Comment]
Bug caused by (feature/regressing bug #): FHR
User impact if declined: FHR errors during initialization will not be reported.
Testing completed (on m-c, etc.): It's been on the tree for a few days. There is test coverage.
Risk to taking this patch (and alternatives if risky): Low risk.
String or UUID changes made by this patch: None

This patch enables FHR to self-report errors that occur on startup. We have reason to believe that a non-trivial amount of clients may experience errors on startup that are not yet captured by FHR's error reporting mechanism. We want these errors reported so we can identify and fix issues sooner.

If we fail to uplift this patch, FHR init errors may not be identified until 22 has a sufficient user base. As a result, we may find ourselves uplifting patches to beta instead of aurora since error discovery was delayed by a release.

Comment 7

[bhavana bajaj [:bajaj]]

Comment on attachment 725560 [details] [diff] [review]
Submit payload on init failure, v1

Patch helps self report start-up errors for FHR, catching these will help us identify/fix issues sooner on our end .Its low risk & has test's .

Comment 8

[Gregory Szorc [:gps]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/f26e4a581008