1689380 - Addons manager logging says that download completed when it actually failed

Status: RESOLVED FIXED

(Toolkit :: Add-ons Manager, defect, P5)

Description

[Mike Kaply [:mkaply]]

When installing an addon, if you turn on extensions.logging.enabled, you see this when there is a network failure:

1611261400470 addons.xpi DEBUG Download started for https://addons.mozilla.org/firefox/downloads/file/3698097/deutsch_de_language_pack-84.0buildid20201221152838-fx.xpi to file C:\Users\mkaply\AppData\Local\Temp\tmp-psm.xpi
1611261400470 addons.xpi DEBUG Download of https://addons.mozilla.org/firefox/downloads/file/3698097/deutsch_de_language_pack-84.0buildid20201221152838-fx.xpi completed.
1611261400470 addons.xpi WARN Download of https://addons.mozilla.org/firefox/downloads/file/3698097/deutsch_de_language_pack-84.0buildid20201221152838-fx.xpi failed: 2152398878
Policies.jsm: Download failed - ERROR_NETWORK_FAILURE - https://addons.mozilla.org/firefox/downloads/file/3698097/deutsch_de_language_pack-84.0buildid20201221152838-fx.xpi Policies.jsm:2308
1611261400471 addons.xpi DEBUG downloadFailed: removing temp file for https://addons.mozilla.org/firefox/downloads/file/3698097/deutsch_de_language_pack-84.0buildid20201221152838-fx.xpi
1611261400471 addons.xpi DEBUG removeTemporaryFile: https://addons.mozilla.org/firefox/downloads/file/3698097/deutsch_de_language_pack-84.0buildid20201221152838-fx.xpi removing temp file C:\Users\mkaply\AppData\Local\Temp\tmp-psm.xpi

The message:

1611261400470 addons.xpi DEBUG Download of https://addons.mozilla.org/firefox/downloads/file/3698097/deutsch_de_language_pack-84.0buildid20201221152838-fx.xpi completed.

is misleading. Completed implies success when the download actually failed here. We should eliminate this message or come up with some other way to say this:

https://searchfox.org/mozilla-central/rev/0dfbe5a699cc6c73cf8c14d1aa10ba10ef3ec8fa/toolkit/mozapps/extensions/internal/XPIInstall.jsm#2409

The deleting of the tmp file at the end is a little confusing as well.

1611261400471 addons.xpi DEBUG removeTemporaryFile: https://addons.mozilla.org/firefox/downloads/file/3698097/deutsch_de_language_pack-84.0buildid20201221152838-fx.xpi removing temp file C:\Users\mkaply\AppData\Local\Temp\tmp-psm.xpi

Will there still be a temp file even though there was no download?

This was discovered while investigating bug 1684501

Comment 1

[chandhu455]

Hi Shane Caraveo, I am new bie to the open source contribution. I wish to take up this issue... could you pls guide me?

Comment 2

[Caitlin Neiman [:caitmuenster]]

Hey chandhu, welcome! Please take a look at https://wiki.mozilla.org/WebExtensions/Contribution_Onramp to learn how to get set up for fixing this bug. :) Once your development environment is set up, Shane can provide more guidance.

Comment 3

[Shane Caraveo (:mixedpuppy)]

Hi Chandhu, after you look through the onramp and get started there, needinfo me again if you have any questions, as well then I can give some guidance. Thanks!

Comment 4

[phu.thai888]

Hello,

I am new to open source development and looking to improve my skills. Could I take on this project? I might need some guidance also.

thank you for your time,

Comment 5

[Caitlin Neiman [:caitmuenster]]

Hi Phu, thanks for your interest! Chandu recently expressed interest in working on this bug; let's give them another week to submit an initial patch before we open up this bug to other contributors. :)

In the meantime, would you be interested in working on one of our other good-first-bugs? You can find other good-first-bugs for this component at https://mzl.la/2VXBaqe and we also have a number of good-first-bugs for the WebExtensions API at https://mzl.la/2yq1XA8.

To get started, check out our onboarding documentation at https://wiki.mozilla.org/WebExtensions/Contribution_Onramp.

Comment 6

[Makenzie]

Hello! I am an Outreachy applicant interested in fixing this bug. My dev environment is set up and I can successfully commit, this will be the second bug I'm working on. May I be assigned? I may need some guidance but I'm ready to go. Thanks!

Comment 7

[Shane Caraveo (:mixedpuppy)]

Hi! Sorry for the delay. I've assigned the bug to you. You could start by looking into the code linked to in the initial post in this bug. It seems to me that that specific log entry would be better placed in downloadCompleted.

Comment 8

[Makenzie]

(In reply to Shane Caraveo (:mixedpuppy) from comment #7)

Hi! Sorry for the delay. I've assigned the bug to you. You could start by looking into the code linked to in the initial post in this bug. It seems to me that that specific log entry would be better placed in downloadCompleted.

Actually I just realized this is for toolkit, and I'm not sure I have the experience quite yet to work on this, so it might be best to unassign me for now. I appreciate your reply. Sorry about that!

Comment 9

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

This good-first-bug hasn't had any activity for 2 months, it is automatically unassigned.
For more information, please visit auto_nag documentation.

Comment 10

[Favour]

Hi Shane Caraveo, I am an Outreachy applicant interested in fixing this bug. I wish to take up this issue... could you pls guide me?

Comment 11

[chase.philpot]

Attachment: Bug 1689380 - Addons manager logging says that download completed when it actually failed. r?rpl

Comment 12

[chase.philpot]

I have posted a patch. Should you be the reviewer or should someone else? Thank you in advance!

Comment 13

[Luca Greco [:rpl] [:luca] [:lgreco]]

Thanks for looking into improving this XPIProvider debug log, I've just submitted an initial round of review comments (and redirected the review quested to myself).

Comment 14

[chase.philpot]

Attachment: Bug 1689380 - Addons manager logging says that download completed when it actually failed. r?rpl

Comment 15

[chase.philpot]

Hi Luca, I have submitted a revision. Please let me know if this needs further attention or if it is ready to land. Thank you in advance!

Comment 16

[Luca Greco [:rpl] [:luca] [:lgreco]]

(In reply to chase.philpot from comment #15)

Hi Luca, I have submitted a revision. Please let me know if this needs further attention or if it is ready to land. Thank you in advance!

The last version of the patch did look good to go, I just signed it off and queued to autoland.

Thanks for taking care of fixing it!

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/d05d898f1512
https://hg.mozilla.org/integration/autoland/rev/6f22269a56a3
Addons manager logging says that download completed when it actually failed. r=rpl

Comment 18

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/6f22269a56a3