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

Audio/Video element with preload can lead to precaching getting a 206 response and crashing #3288

Open
segevfiner opened this issue Jan 11, 2024 · 8 comments

Comments

@segevfiner
Copy link

Library Affected:
workbox-precaching

Browser & Platform:
Google Chrome 120.0.6099.199 Desktop

Issue or Feature Request Description:
On a page with an audio/video elements with preload="auto", where workbox is also set to precache the audio/video src file, sometimes the service worker ends up getting a 206 in its install event for its precache request, which leads to the following error:

workbox-0f22832a.js:2064 Uncaught (in promise) TypeError: Failed to execute 'put' on 'Cache': Partial response (status code 206) is unsupported
    at StrategyHandler.cachePut (workbox-0f22832a.js:2064:23)
    at async PrecacheStrategy._handleInstall (workbox-0f22832a.js:3072:27)
    at async PrecacheStrategy._handle (workbox-0f22832a.js:3005:18)
    at async PrecacheStrategy._getResponse (workbox-0f22832a.js:2402:22)
cachePut @ workbox-0f22832a.js:2064

I'd assume it's taking the request to preload from the browser which has a Range request, before the pre-caching is done, and sending it to the network, trying to pre-cache based on it, instead of simply dropping the uncachable partial response, and still sending a normal pre-cache request separately.

I did follow https://developer.chrome.com/docs/workbox/serving-cached-audio-and-video (P.S. I guess the cacheableResponse plugin for this is redundant nowadays?)

When reporting bugs, please include relevant JavaScript Console logs and links to public URLs at which the issue could be reproduced.
The site is private, I will create a standalone repro later on.

@segevfiner
Copy link
Author

Hmm, actually I probably need to add the RangeRequests plugin to the precaching handler... So that runtimeCachine example from the linked article isn't helpful for that...

@segevfiner
Copy link
Author

Doesn't seem to help even if I do that though...

@segevfiner
Copy link
Author

I guess a workaround will be to do preload="none" and hope the user doesn't start any media before pre-caching ends... There doesn't seem to be an obvious way to add plugins to workbox-precaching in generateSW mode of workbox-build which I opened a separate request for #3289.

I guess a fix will be to just let such requests through to the network, without trying to precache from their response, letting the normal pre-caching still take place, or alternatively to outright to dump the Range header when getting a request that has it during the install phase in the PrecachingStrategy, and maybe also pass the response through plugins so the ranges plugin can modify the complete response to only return the requested range even in that case.

@segevfiner
Copy link
Author

Maybe we want something like this in the range requests plugin or inside workbox-precaching:

// requestWillFetch() if from a plugin
    if (handler.event && handler.event.type === 'install') {
      request.headers.delete("Range")
    }

@segevfiner
Copy link
Author

Doesn't work. The headers received in the request object seem empty for this case...

@segevfiner
Copy link
Author

We probably need to add 206 to be ignored here:

if (!response || response.status >= 400) {

@segevfiner
Copy link
Author

Doesn't help, still leads to bad-precaching-response.

@segevfiner
Copy link
Author

segevfiner commented Jan 31, 2024

So basically the browser sets a Range: bytes=0- header with preload="auto" and gets a full response with a 206 status, when the service worker starts after that request is complete, and tries to prefetch the file, the browser returns the 206 full content response (Racy, due to chromium coalescing the requests) which fails to be inserted into the cache with the described error. Something like this can possibly fix this:

    async cacheWillUpdate({ response }) {
      // The default will update logic of workbox-precaching
      if (!response || response.status >= 400) {
        return null;
      }

      if (response.status === 206 /* && fullBody */) {
        console.log('Patching 206 response');
        response = await copyResponse(response, (responseInit) => {
          responseInit.status = 200;
          return responseInit;
        })
      }

      return response;
    },

But I think copyResponse will fail if the request is cross origin...

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

1 participant