ChannelDownloader fails to download from servers that do not support resuming

RESOLVED FIXED in Firefox 58

Status

()

enhancement
P1
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: mhowell, Assigned: mhowell)

Tracking

(Blocks 1 bug)

Trunk
mozilla58
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox58blocking fixed)

Details

Attachments

(1 attachment)

In bug 1348087 we started using nsIResumableChannel to resume update downloads, but I didn't handle the case where the server sends Accept-Ranges: none to explicitly disable resuming; in that case, accessing the entityID property of the request will throw NS_ERROR_NOT_RESUMABLE, which we should be catching.

The download manager does this at:
https://searchfox.org/mozilla-central/rev/30ead7d1ae5bf95b8bc0fd67b950cd46ca05e32c/toolkit/components/jsdownloads/src/DownloadCore.jsm#2013

My plan for this bug is to copy over that logic.
Assignee

Updated

2 years ago
Assignee: nobody → mhowell
Status: NEW → ASSIGNED
Priority: -- → P1
Marking this as a blocker for 58.
From discussion on irc, sounds like this affected b1 users (on the dev edition channel only) so we have a good chance to fix it if we land a patch before Monday morning, when we plan to build 58.0b3 for the beta channel.
Assignee

Updated

2 years ago
Attachment #8927395 - Flags: review?(dothayer)

Comment 4

2 years ago
mozreview-review
Comment on attachment 8927395 [details]
Bug 1416295 - Support downloading updates from servers that don't support resuming.

https://reviewboard.mozilla.org/r/198696/#review203812

LGTM

::: toolkit/mozapps/update/nsUpdateService.js:3606
(Diff revision 1)
> +      // Reading the entityID can throw if the server doesn't allow resuming.
> +      try {
> -      this._patch.setProperty("entityID", request.entityID);
> +        this._patch.setProperty("entityID", request.entityID);
> +      } catch (ex) {
> +        if (!(ex instanceof Components.Exception) ||
> +              ex.result != Cr.NS_ERROR_NOT_RESUMABLE) {

Nit: align with the first paren, not the second, since this is excluded from the second paren's clause.
Attachment #8927395 - Flags: review?(dothayer) → review+
Comment hidden (mozreview-request)
The Mac jobs on try haven't started yet, but everything else passed, so I'm landing this.

Comment 7

2 years ago
Pushed by mhowell@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/df5703f27971
Support downloading updates from servers that don't support resuming. r=dthayer

Comment 8

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/df5703f27971
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
You need to log in before you can comment on or make changes to this bug.