1578955 - downloads API: Allow extensions a way to download ignoring HTTP errors

Status: RESOLVED FIXED

(WebExtensions :: General, enhancement)

Description

[Nils Maier [:nmaier]]

Bug #1576333 will bring Firefox to parity with Chrome in regards to handling HTTP errors and cancelling downloads on HTTP errors.

A concern arose that some extensions might in fact want to continue with downloads even though the server returned an error such as 404 Not Found.

This is currently unsupported in Chrome, and implementing this in Firefox would be a new feature.

I therefore propose the following changes:

1. Add a new, optional boolean flag allowHttpErrors to the options of download.download(), which defaults to false.
2. When the flag is false (the default) in calls to download.download(), then there should be no change in behavior, and downloads should continue to be cancelled upon encountering HTTP errors.
3. When the flag is true in calls to download.download(), then upon encountering errors the download should be allowed to proceed regardless, but DownloadItem.error should be set to the appropriate InterruptReason, such as SERVER_BAD_CONTENT, as usual, and special care should be taken that any .onChanged listeners are notified of the changed DownloadItem.error value.
4. When the flag is true in calls to download.download(), when the download finishes and changes its state to complete, the DownloadItem.error should still reflect the InterruptReason corresponding to HTTP error code encountered earlier.
5. When the flag is true in calls to download.download() and there is a later error, such as writing the file to disk failed, then the DownloadItem.error should reflect the InterruptReason of this later error.

Comment 1

[Nils Maier [:nmaier]]

Attachment: 0001-Bug-1578955-Add-allowHttpErrors-feature-to-downloads.patch

Preliminary patch, demonstrating the idea, tho I am not married to the details and open to suggestions.

It is supposed to stack on the patches from bug 1576333 of course.

Comment 2

[Nils Maier [:nmaier]]

Attachment: Bug 1578955 - Add allowHttpErrors feature to downloads API r?robwu

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2d12f40be9fc
Add allowHttpErrors feature to downloads API r=robwu

Comment 4

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/2d12f40be9fc

Comment 5

[Richard Bloor]

The following documentation has been updated:

• compatibility table data, the addition of theallowHttpErrors property, see pull request https://github.com/mdn/browser-compat-data/pull/5042.
• downloads.download() added documentation of the allowHttpErrors property.
• Release notes noted the addition of the allowHttpErrors property with basic description of features.

Please let me know if there are any changes or additions needed. Thank you!

Comment 6

[Nils Maier [:nmaier]]

Seems something went wrong with your downloads.dowload() change?
https://wiki.developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/downloads/download$compare?locale=en-US&to=1588216&from=1588068

Comment 7

[Richard Bloor]

Not sure what went wrong there, but fortunately I had my changes preserved in the preview page: downloads.dowload() Is now as I intended.

Comment 8

[Nils Maier [:nmaier]]

We eventually opted to not report at all HTTP errors when allowHttpErrors is true, i.e. the errors that bug 1576333 added are ignored. Other errors, such as FILE_* and NETWORK_* errors will be reported.

I'd replace the following phrase and the listing that follows:

Errors encountered during the download are reported: [...]

with something like:

HTTP server errors such as SERVER_BAD_CONTENT will be ignored entirely and are not reported, and the download is allowed to proceed, however other errors unrelated to HTTP server errors - such as FILE_FAILED or NETWORK_FAILED - still get reported and fail the download, as usual. Using this flag e.g. allows to deliberately download server error pages.

And I'd remove the following sentence entirely:

All .onChanged listeners are notified of the changed DownloadItem.error value.

Sorry, I forgot to mention this before and created extra work.

Comment 9

[Richard Bloor]

No worries, I have updated downloads.download. Please let me know if this is OK. Thanks!

Comment 10

[Nils Maier [:nmaier]]

Looks good to me.
Thanks

Comment 11

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

(clearing needinfo, Nils already provided feedback on the updated MDN api docs).