1778754 - WDBA doesn't show notification or send telemetry if checking default PDF application fails

Status: RESOLVED FIXED

(Toolkit :: Default Browser Agent, defect, P3)

Description

[Robin Steuber (she/her) [:bytesized]]

I'm trying to run some manual testing on the code that I'm writing for Bug 1776069. But I'm not seeing any notifications at all. When I look in the Event Viewer, I see

0x80070483 in GetDefaultPdf:54

Judging by this line, it looks to me like we never show any notifications or send any pings if we fail determine the default PDF viewer. It doesn't seem to me like this ought to be a fatal error.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1742674

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

:nrishel, since you are the author of the regressor, bug 1742674, could you take a look?
For more information, please visit auto_nag documentation.

Comment 3

[[e-mail] (:bhearsum)]

If we're truly losing Telemetry (and it looks like we are) this seems like a P1 issue.

Comment 4

[Nick Rishel [:nrishel]]

Is there an apparent drop in telemetry when it landed in stable or is this a rare scenario?

The same behavior applies to reading the default browser, so I guess that should be updated while I'm investigating?

Comment 5

[Robin Steuber (she/her) [:bytesized]]

(In reply to Nick Rishel [:nrishel] from comment #4)

The same behavior applies to reading the default browser, so I guess that should be updated while I'm investigating?

I'm happy going either way on this.

I do feel like the optimal thing would be to report errors back to telemetry in either failure case. But I would consider showing the notifications to be the WDBA's primary purpose. So it seems important to me that, if we can, we show that notification.

Since we can't really show the notification without knowing the default browser, I'm okay with bailing out if that fails. But we can still show the notification if the PDF check fails, so I don't really want to bail out completely in that case.

Comment 6

[Robin Steuber (she/her) [:bytesized]]

(In reply to Nick Rishel [:nrishel] from comment #4)

Is there an apparent drop in telemetry when it landed in stable or is this a rare scenario?

I wouldn't consider myself a pro at analyzing telemetry, but a cursory look suggests to me that we didn't see a significant drop when this merged.

---

I think that I figured out what was causing me to get that error though. I could reproduce it consistently. Looking in the settings, it showed that I had a PDF viewer set. However, I tried opening a PDF file and was greeted with the "what program do you want to open this with" prompt. I seem to recall that Windows does this when you install a new handler for an application? I selected my PDF reader in the prompt and now the WDBA is no longer consistently hitting that error for me.

Comment 7

[[e-mail] (:bhearsum)]

(In reply to Kirk Steuber (he/him) [:bytesized] from comment #6)

(In reply to Nick Rishel [:nrishel] from comment #4)

Is there an apparent drop in telemetry when it landed in stable or is this a rare scenario?

I wouldn't consider myself a pro at analyzing telemetry, but a cursory look suggests to me that we didn't see a significant drop when this merged.

Given this, I'm lowering the priority.

Comment 8

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1742674

Comment 10

[BugBot [:suhaib / :marco/ :calixte]]

The severity field for this bug is set to S3. However, the following bug duplicate has higher severity:

• Bug 1780213: S2

:nrishel, could you consider increasing the severity of this bug to S2?

For more information, please visit auto_nag documentation.

Comment 11

[Nick Rishel [:nrishel]]

Clearing ni, severity is appropriate as-is.

Comment 12

[Nick Rishel [:nrishel]]

Attachment: Bug 1778754 - Send WDBA telemetry even when default browser or PDF telemetry fails. r=bytesized

Adds error enum and strings to WDBA telemetry.

Depends on D174973

Comment 13

[BugBot [:suhaib / :marco/ :calixte]]

There is an r+ patch which didn't land and no activity in this bug for 2 weeks.
:nrishel, could you have a look please?
If you still have some work to do, you can add an action "Plan Changes" in Phabricator.
For more information, please visit BugBot documentation.

Comment 14

[Robin Steuber (she/her) [:bytesized]]

I'm going to leave this for nrishel to answer.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d412fdc431d3
Send WDBA telemetry even when default browser or PDF telemetry fails. r=bytesized

Comment 16

[Iulian Moraru]

Backed out for causing multiple build bustages.
So far, this has only affected windows.

• Push with failures

• Failure log example 1

• Failure log example 2

• Backout link

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/341d00ab9386
Send WDBA telemetry even when default browser or PDF telemetry fails. r=bytesized

Comment 18

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/341d00ab9386

Comment 19

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:nrishel, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox118 to wontfix.

For more information, please visit BugBot documentation.

Comment 20

[Ryan VanderMeulen [:RyanVM]]

Is this something that should be considered for ESR?

Comment 21

[Nick Rishel [:nrishel]]

Impact is low and not user facing so my initial instinct is probably not.

Comment 22

[Nick Rishel [:nrishel]]

Attachment: WIP: Backed out 18 changesets (Bug 1778754, bug 1852412, bug 1850631, bug 1853144, bug 1851937, bug 1851714, bug 1851930, bug 1838754, bug 1852881, bug 1851857)

Backed out changeset efc3444da9bb (bug 1851714)
Backed out changeset df36b070d90e (bug 1853144)
Backed out changeset 996e3cf63378 (bug 1852881)
Backed out changeset daceab96e958 (bug 1850631)
Backed out changeset db960ca5d490 (bug 1852412)
Backed out changeset b0b5db15d567 (bug 1851937)
Backed out changeset a2f73b54db1f (bug 1851857)
Backed out changeset f06abc41688e (bug 1851930)
Backed out changeset 214f0e1df0dd (bug 1851930)
Backed out changeset 341d00ab9386 (bug 1778754)
Backed out changeset 33dc6c6f62d7 (bug 1838754)
Backed out changeset 1da5e2dc15f9 (bug 1838754)
Backed out changeset ef4f82aa3980 (bug 1838754)
Backed out changeset 27b6015a6b28 (bug 1838754)
Backed out changeset fb4dfb4d76c1 (bug 1838754)
Backed out changeset ff651e1a09c7 (bug 1838754)
Backed out changeset 617f15d7dff8 (bug 1838754)
Backed out changeset 868fdde83bdc (bug 1838754)

Conflict resolved with changeset 845f618bbdf6 (bug 1840096)

Backed out due to changes breaking both Default Agent telemetry and launching Firefox in response to interaction with the Default Agent notifications.

Comment 23

[Nick Rishel [:nrishel]]

Attachment: Backed out changeset 341d00ab9386 (Bug 1778754)

Backed out due to changes breaking both Default Agent telemetry and launching Firefox in response to interaction with the Default Agent notifications.

Conflict resolved with changeset 845f618bbdf6 (bug 1840096)

Depends on D190873

Comment 24

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/releases/mozilla-release/rev/14b83f35b744
Backed out changeset 341d00ab9386