Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WebKit: in-memory FileReader failover, invalid Content-Range header. #145

Open
theazgra opened this issue Aug 4, 2024 · 2 comments
Open

Comments

@theazgra
Copy link

theazgra commented Aug 4, 2024

Hi,

this could be considered as follow-up to #134 , which was resolved by #138 .

We have discovered new bug, affecting same users, so Safari users. We have detected the error while uploading file of size cca 1.5GB.

The implemented failover is used, as this messege is logged to the console

Unable to read file of size 1503686620 bytes via a ReadableStream. Falling back to in-memory FileReader!

Then the upload continues, failing with the last chunk. That is, because the Content-Range header is invalid. The upper limit of the range is larger than file size.

Headers of the last chunk upload:

Accept: */*
Content-Range: bytes 1384120320-1509949439/1509949439
Content-Type: video/mp4
Origin: https://www.forendors.cz
Referer: https://www.forendors.cz/
Sec-Fetch-Dest: empty
Sec-Fetch-Mode: cors
Sec-Fetch-Site: cross-site
User-Agent: Mozilla/5.0 (iPhone; CPU iPhone OS 17_3 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/17.3 Mobile/15E148 Safari/604.1

Note that: 1509949439 >1503686620

HTTP error from Google API:

XMLHttpRequest cannot load https://storage.googleapis.com/video-storage-gcp-us-east4-vop1-uploads/EuWRHwC3NSg6Uen2EQttnC?Expires=1722829957&GoogleAccessId=uploads-gcp-us-east1-vop1%40mux-video-production.iam.gserviceaccount.com&Signature=D%2FppImf0Tff85CxU3k%2B1cSrScqe5spcH0TGE6kN3Ki62T1IGCeoj6k5f6EdFf1RnDp0032%2BIiX8eiYV0EJ0yidex1eq1SsZf2hfCyBJ7X83iz3%2B1%2FLrAXwyNRPeRzuCnKuUU%2BrexQL5R%2BITA%2BlAZILDkRNg9mEA0crJ7IhoPsMe6bthxxWW0ZLDoC0vcsJBlhOyeoznoSLSH%2B%2FY3cloI2ZeLM4%2FumkI0YaGWHAg5ziAJR0Juqzu2EYP%2B037w2TXPqfrPVr%2Bka71QQ9DzFintSQkHm8Aqc%2FE1kGpD72%2FlN7bo76NDw3I4OcqmiXMVsQo4dWUVwSEo%2F%2BE8kcuXBJPhlg%3D%3D&upload_id=AHxI1nNfUtlwKC7BncAsAKE5JwqECfs48Ouzhdz3lzrlfwu-SON7K-5M0a3kdYMcPdt9wbZZdxFeELWYNxFHVzjxa5G8fNKYl2Wrb7kUVrE4CuG2Jg due to access control checks.

error handler is the correctly invoked atleast :)

Interestingly, second time uploading the same file, fixes the problem. (while using the same UpChunk instance, page is not reloaded)

@theazgra
Copy link
Author

theazgra commented Aug 6, 2024

@cjpillsbury I will tag you directly, hope that is ok.

After looking at it more, the problem seems to be with nextChunkRangeStart property. Which is not reset correctly after fallback is applied.

nextChunkRangeStart is updated in successfulChunkUploadCb, then error occurs and readableStreamErrorCallback is called, nextChunkRangeStart is not reset-ed to 0.

This leads to nextChunkRangeStart being different in ChunkedFileIterable instance and UpChunk instance

ResettingnextChunkRangeStart in readableStreamErrorCallback seems to fix the problem.

if (options.useLargeFileWorkaround) {
      const readableStreamErrorCallback = (event: CustomEvent) => {
        // In this case, assume the error is a result of file reading via ReadableStream.
        // Retry using ChunkedFileIterable, which reads the file into memory instead
        // of a stream.
        if (this.chunkedIterable.error) {
          this.nextChunkRangeStart = 0; // <-- reset nextChunkRangeStart
          console.warn(
            `Unable to read file of size ${this.file.size} bytes via a ReadableStream. Falling back to in-memory FileReader!`
          );
          event.stopImmediatePropagation();

          // Re-set everything up with the fallback iterable and corresponding
          // iterator
          this.chunkedIterable = new ChunkedFileIterable(this.file, {
            ...options,
            defaultChunkSize: options.chunkSize,
          });
          this.chunkedIterator = this.chunkedIterable[Symbol.asyncIterator]();
          this.getEndpoint().then(() => {
            this.sendChunks();
          });
          this.off('error', readableStreamErrorCallback);
        }
      };
      this.on('error', readableStreamErrorCallback);
    }

@cjpillsbury
Copy link
Contributor

Hey @theazgra sorry for the delayed response! That looks right to me and all makes sense. Feel free to issue a pull request if you wanted to contribute!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants