Right now Firefox only ever fails downloads with the WE downloads API with an InterruptReason of either NETWORK_FAILED or FILE_FAILED.
Downloads where the server returned a HTTP error such as 404 Not Found, 504 Gateway Timeout are reported back to the extension as done, with no way for the extension to determine if there was an error or not.

Chrome on the other hand supports a wide range of errors, some of which correspond to HTTP Errors.
See: https://cs.chromium.org/chromium/src/components/download/internal/common/download_utils.cc?rcl=529b59b7d8ed908a9897be40d65f01be9469e349&l=107

This bug is about parity/compatibility with Chrome in regards to HTTP errors, in that Firefox should cancel the download and report an error when encountering certain HTTP codes:

• SERVER_BAD_CONTENT: 404
• SERVER_FORBIDDEN: 403
• SERVER_UNAUTHORIZED: 402 and Proxy 407
• SERVER_FAILED: Anything else above 400

The lack of errors is really a MAJOR inconvenience one of my extensions (DownThemAll!)

Maybe :evilpie wants to take a looksy after he thankfully fixed the referrer header limitation (bug 1367626)

Actually, I got a patch now. Let's see if I can figure out this moz-phab stuff

Depends on D43342

Depends on D43343

Thanks for the bug and patches Nils, it all looks ok from the quick look, but I wouldn't be the right person to review Downloads Core code.

I've added :mak for the first one, and removed my self from the others to get them out of my queue. Please request review again after you get r+ for part 1 dependency.

I am new to this phab stuff... Did me updating add you for review again? Or what else should I do/should I have done?

You're all good, though it might take me a couple of days to get to these, I apologize in advance.

Ping pong... Will you get a chance to review this soon-ish, or should I look for another reviewer (and any suggestions)?

Pinging for reviews within 24 hours is rarely helpful.

Pushed by apavel@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/22f66ae3fa5a
Part 1: Allow Downloads users to inspect HTTP codes r=mak
https://hg.mozilla.org/integration/autoland/rev/5407ffcafcb2
Part 2: Make WE downloads API cancel on HTTP errors r=zombie
https://hg.mozilla.org/integration/autoland/rev/a84b2fedfb3c
Part 3: Fix broken test_inprogress r=zombie

https://hg.mozilla.org/mozilla-central/rev/22f66ae3fa5a
https://hg.mozilla.org/mozilla-central/rev/5407ffcafcb2
https://hg.mozilla.org/mozilla-central/rev/a84b2fedfb3c

Thanks Nils, for future reference, you don't need to rename commits after getting reviews, r? gets converted into r= as appropriate during landing.

Hey Philipp, this (along with bug 1578955) should probably be mentioned in the "what's new" api blog post for 71.

Thanks Nils, for future reference, you don't need to rename commits after getting reviews, r? gets converted into r= as appropriate during landing.

Good to know, thanks, but I had to address the one remaining nit anyway and phab again ;)

Hello,
So far I've been able to see some error messages while testing on Windows 10 with 64-bit on both Firefox Nightly 70.0a1 (Build ID:20190901094958) and Firefox Nightly 71.0a1 (Build id:20190906094324). See the "Down Them all error messages" image.
However, as this was supposed to be a Fx71 fix, I am not convinced Down them All Manager is where the fix should have been visible.
Could you confirm if it is and if not could you provide some steps to reproduce and test the issue?
Also, if this issue requires manual testing please assign it the "qe+ verify" flag, Otherwise set it as "qe-verify-".
Thank you

I am the author of DownThemAll!, in case you didn't know.

If you test with DownThemAll! 4 against a FX71 with this fix, then you'll see it in action.
However, DownThemAll! will in some cases, based on some heuristics, perform regular fetch requests before performing an actual download with this API, to work around an unrelated issue. This includes URLs containing for example .php or .asp, or contain a query string. So since it does these fetch requests anyway, it will look at their status codes itself and set the error appropriately. This is why, for some downloads, you'll see it report errors even though you're using a Firefox without the fix.

Internal Browser Error (which is InterruptReason = CRASH) and Network Error (which is InterruptReason = NETWORK_FAILED) are unrelated to the fix from this bug and are present in all Firefox versions. The CRASH one is probably a result of bug 1576348.

If you reliably want to manually verify this fix with DownThemAll!, you will have use version 4.0.8 or earlier, as 4.0.9 added those fetch requests.

Also, if this issue requires manual testing please assign it the "qe+ verify" flag, Otherwise set it as "qe-verify-".

There are xpcshell unit tests for all the new error codes Firefox supports now with this fix, so I'm guessing no manual testing is required? I am not too familar with the policies around that...

Tested fix using Firefox 71.0a1/ 20190909095540 on Windows 10 64-bit, using the 4.0.8 DownThemAll! extension and triggered only the Not Found and Server Error errors. On Ubuntu 18.04.3 LTS FF 71.0a1 20190909095540 all requests were marked as "Done" so far.
If you could provide some sample links that when downloaded trigger the other types of errors then this could be manually verified.
If a bug can be tested through a series of reproducible steps, manually, then it's useful to have it assigned as "qe+ verify". If it cannot be done so, or it's covered by another set of tests, then you can mark it as "qe-verify-" from the Detail section of the bug.

If a bug can be tested through a series of reproducible steps, manually, then it's useful to have it assigned as "qe+ verify". If it cannot be done so, or it's covered by another set of tests, then you can mark it as "qe-verify-" from the Detail section of the bug.

It's covered by the xpcshell tests, so qe-verify- it is, I guess.

I've put this and the other bug mentioned on my list for Firefox 71

Could you clarify the documentation/compatibility table updates required? It would appear that these error messages are documented on downloads.InterruptReason as errors returned from downloads.download() and have been since the page was created in 2016. It would, therefore, appear that no documentation updates are needed, or am I missing a nuance of the requirements?

The InterruptReasons had been documented (having been copied from the Chrome docs), but were not actually supported.

What really needs updating is the compat table I'd guess: Add a note on when The InterruptReasons added by this bug because available, and that previously such errors were ignored.

I'm thinking that the appropriate documentation is a comment in the release notes, which is done.

In my opinion, it should documented on the page for the API(s) as well, at least in the compatibility matrix table(s) because 68 ESR will be around for quite a while without these changes, so authors might want to know about it, and e.g. do workarounds when running in an ESR or specify an appropriate minversion or at least be aware of this when users report bugs.