"Error: Unexpected response, download incomplete." in Firefox

[pschichtel]

In Firefox: When I click download on any object I get "Unexpected response, download incomplete" in the "Uploads / Downloads" widget (close to the end of the progress bar, > 90%). In the network tab the only request that I see (https://console..../api/v1/buckets/.../objects/download?...) has status 200. Nothing in the minio log. Nothing in the browser console.

When I do the same in Chromium instead of Firefox, then the file downloads normally.
What I noticed: In firefox the download response payload seems to be base64 encoded, in chromium it isn't. (probably just a display thing in firefox' console)

Expected Behavior

Downloads work identically in Firefox and Chromium.

Current Behavior

In Firefox it fails.

Steps to Reproduce (for bugs)

This is the only Minio cluster of mine that has this behavior and I have no idea when it started. Other clusters I operate running the same version of Minio work fine.

Context

minio/minio#19091

Your Environment

• Version used (minio --version):

  minio version RELEASE.2024-02-17T01-15-57Z (commit-id=b6e98aed0161bb526aaf11fa1672d30eb89dca4b)
  Runtime: go1.21.7 linux/amd64
  License: GNU AGPLv3 <https://www.gnu.org/licenses/agpl-3.0.html>
  Copyright: 2015-2024 MinIO, Inc.
  

• Server setup and configuration: It's deployed in a kubernetes 1.29.2 (k0s) using the minio operator (5.0.12). It's single node (both k8s and minio) and single pool (openebs-hostpath volume on xfs).
• Operating System and version (uname -a): Linux ... 6.7.5-arch1-1 #1 SMP PREEMPT_DYNAMIC Sat, 17 Feb 2024 14:02:33 +0000 x86_64 x86_64 x86_64 GNU/Linux

[pschichtel]

The preview also works without issues, for shorter documents it displays the entire thing without issues.

[alexisdondon]

is it fixed #3214 ?

[pschichtel]

I'll check in the evening

[pschichtel]

I just updated to RELEASE.2024-02-26T09-33-48Z, but the issue persists.

[cesnietor]

I've debugged this and seems that Firefox for some reason doesn't report the downloaded size as others do.
so the let completeDownload = bytesLoaded === fileSize; done here is false.
From a successfull download the corresponding values are:

fileSize 1048576000
evt.loaded 1048656025

And we expect them to be the same.
We'll need to dig more on how to fix this case for Firefox. We can't just remove the completeDownload condition cause we don't want incomplete downloads to happen.

[pschichtel]

interesting, that's quite the difference. first thing that came to my mind was "compressed size", but I don't think the difference is big enough to account for gzip.

[cesnietor]

Yes, and the difference changes depending on the filesize so we can't account for a fixed offset.
If you find something related on this issue for Firefox that could improve the solution please let us know. Or feel free to send a PR @pschichtel

[pschichtel]

@cesnietor I googled around a bit and my suspicion is currently, that "progress" might not consistently fire for the last bytes. It seems the spec is only requiring it to be fired at least once: https://xhr.spec.whatwg.org/#dom-progressevent-loaded

I think you want to also mount a handler to the load event (https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest/load_event) which will always be the final progress event on success. alternatively the loadend event to cover error cases as well, but I think that's covered in the logic already. I think the simplest fix might be bind the same function to progress and load.

[pschichtel]

Non of my assumptions turned out to be true. I even had a case where loaded reported more bytes than fileSize.

[cesnietor]

Thanks for investigating, was a nice start, I also tried some stuff and indeed the loaded can be bigger or smaller. Found an old issue for Firefox. But haven't found more. We'll have to spend more time on this one.

Please use RELEASE.2024-02-14T21-36-02Z or older versions while this is taken care of.

[pschichtel]

I just confirmed: The size reported by firefox is the gzipped size of the file. I counted the bytes from curl's output with and without the --compressed option, they line up exactly.

[pschichtel]

The intent of the size check is to detect incomplete downloads, right? In what scenario would a download be incompletely while not triggering any of the error/failure mechanism of XHR? i.e.: is the check like this really necessary?

[cesnietor]

I just confirmed: The size reported by firefox is the gzipped size of the file. I counted the bytes from curl's output with and without the --compressed option, they line up exactly.

ah interesting 🤔 .

The intent of the size check is to detect incomplete downloads, right? In what scenario would a download be incompletely while not triggering any of the error/failure mechanism of XHR? i.e.: is the check like this really necessary?

So this is the original issue.
The thing is that the request was coming as a 200 and everything seemed to be fine in the backend (no errors thrown) but the file was not fully downloaded (whenever there was a server disconnected) [test steps on the PR].
So we added a filesize validation just to be sure everything was right.

[cesnietor]

We could use the loadend and load events to achieve this. We'll need to verify the k8s scenario.

[pschichtel]

loadend and load behaved identical to progress.

[cesnietor]

oh but we wouldn't need to read the loaded data anymore since load is sent when:

load | Progression is successful.

[pschichtel]

Ah so instead of tracking the size you assume that when load is fired the download really was complete? Does the browser have enough information to decide this given that minio is doing chunked transfer?

[cesnietor]

I hope so :D I'm doing some testing.

[pschichtel]

I considered migrating the code over to fetch, but it seems that is has the exact same problem: anthumchris/fetch-progress-indicators#13 (comment)

So regarding checking the size we are pretty much all out of luck.

[cesnietor]

Ok these are my findings so far:

• Browser is sending the Accept-Encoding and similarly to --compress on curl, it will set it and if the server the server can handle them it will do so [We have a middleware that handles this here] (this edits some headers). In this case we are setting the header content-encoding: gzip so then the browser compresses it to optimize the download, hence we get different sizes (and we don't have that value in the backend). For some reason Chrome doesn't do that.
• We could go through the path of trying to figure out the compressed size in advance and send the value in a header as suggested here but that would be quite inefficient for us since it would be compressed many times (server and client).

Thinking about the initial problem we were trying to solve, is to not download a file when it's incomplete and surface the proper error in the UI. So the best solution would be to handle the error properly when the io.Copy fails see Who should handle the error. I tried in a first proposal to do panic but that's not the best way, also worth mentioning that we can't set a header cause once you write the initial headers and start writing the body, you can't send the headers again. So we'll investigate a better way to return the error.
cc @bayasdev

[harshavardhana]

@cesnietor we can ignore accept-encoding for "GETs" because this can come in the way, causing wrong size being sent to client.

[pschichtel]

@harshavardhana @cesnietor I just had a very simple idea for a fix and it works, see #3249