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

Redirect URL not available from XHR on Chrome with service worker #1392

Closed
avelad opened this issue Apr 4, 2018 · 14 comments
Closed

Redirect URL not available from XHR on Chrome with service worker #1392

avelad opened this issue Apr 4, 2018 · 14 comments
Assignees
Labels
status: archived Archived and locked; will not be updated type: bug Something isn't working correctly
Milestone

Comments

@avelad
Copy link
Collaborator

avelad commented Apr 4, 2018

Have you read the FAQ and checked for duplicate open issues?:
Yes

What version of Shaka Player are you using?:
2.3.5

Can you reproduce the issue with our latest release version?:
Yes

Can you reproduce the issue with the latest code from master?:
Yes

Are you using the demo app or your own custom app?:
Demo app

If custom app, can you reproduce the issue using our demo app?:

What browser and OS are you using?:
Chrome 65 - Ubuntu 17.10 --> Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/65.0.3325.181 Safari/537.36
Firefox 59 - Ubuntu 17.10 --> Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:59.0) Gecko/20100101 Firefox/59.0

What are the manifest and license server URIs?:
https://shaka-player-demo.appspot.com/demo/#asset=https://balancer.demo.anevia.com/live/eds/Arte/dash/Arte.mpd;lang=es-ES;build=uncompiled

What did you do?
Load the previous url.

What did you expect to happen?
I expect the same behaviour of Firefox, the manifest redirect and the segments use the redirection.
any.mpd (balancer url) --> HTTP 302 --> edge url
1.mp4 --> edge url
2.mp2 --> edge url

Note: in 2.2.10 it works well --> https://v2-2-10-dot-shaka-player-demo.appspot.com/demo/#asset=https://balancer.demo.anevia.com/live/eds/Arte/dash/Arte.mpd;lang=es-ES

What actually happened?
In Firefox, the manifest redirect and the segments use the redirection.
any.mpd (balancer url) --> HTTP 302 --> edge url
1.mp4 --> edge url
2.mp2 --> edge url

In Chrome, the manifest redirect and the segments don't use the redirection.

Screenshots:
firefox
chrome

@joeyparrish
Copy link
Member

@avelad, I'm unable to reproduce with Shaka Player v2.3.5 and Chrome 65.0.3325.181 on Linux. I see segments requested from cdn.demo, not balancer.demo.

Can you please check again, and make sure your copy of Chrome is fully updated?

@joeyparrish joeyparrish added status: unable to reproduce Issue could not be reproduced by the team status: waiting on response Waiting on a response from the reporter(s) of the issue labels Apr 5, 2018
@avelad
Copy link
Collaborator Author

avelad commented Apr 6, 2018

@joeyparrish, I saw that I you have open dev tools with "Disable cache" checked (in network tab) and "Bypass for network" (in Application tab --> Service Workers), It works correctly. Are you tested with these fields unchecked?

@shaka-bot shaka-bot removed the status: waiting on response Waiting on a response from the reporter(s) of the issue label Apr 6, 2018
@joeyparrish
Copy link
Member

Interesting, let me try again.

@joeyparrish
Copy link
Member

It seems that the issue only appears when a service worker is used. This is unexpected, but it explains why v2.3 fails and v2.2 works: we didn't have a service worker in v2.2.

@joeyparrish joeyparrish added type: bug Something isn't working correctly and removed needs triage status: unable to reproduce Issue could not be reproduced by the team labels Apr 6, 2018
@joeyparrish joeyparrish added this to the v2.5 milestone Apr 6, 2018
@joeyparrish
Copy link
Member

I suspect this will turn into a bug filed against Chrome, but I need to investigate more deeply. Labelled "bug" for now, and assigned to the v2.5 milestone.

@joeyparrish joeyparrish changed the title Different behaviour on redirect on Chrome & Firefox. Redirect URL not available on Chrome with service worker Apr 6, 2018
@vaage
Copy link
Contributor

vaage commented Jul 23, 2018

@avelad Is the content you provided still available? I only get 404 errors when I try to request the content you provided in your original issue post.

@vaage vaage added the status: waiting on response Waiting on a response from the reporter(s) of the issue label Jul 23, 2018
@avelad
Copy link
Collaborator Author

avelad commented Jul 30, 2018

@vaage , i'm talking with our providers for provide a valid url again.

@shaka-bot shaka-bot removed the status: waiting on response Waiting on a response from the reporter(s) of the issue label Jul 30, 2018
@avelad
Copy link
Collaborator Author

avelad commented Jul 31, 2018

The previous url is working again.

@joeyparrish
Copy link
Member

I used the following code in the JS console to test this:

xhrRedirect = async (url) => {
  const x = new XMLHttpRequest();
  await new Promise((done) => {
    x.onload = done;
    x.open('GET', url, true);
    x.send();
  });
  return x.responseURL;
};

fetchRedirect = async (url) => {
  const r = await fetch(url);
  return r.url;
};

url = 'https://balancer.demo.anevia.com/live/eds/Arte/dash/Arte.mpd';

To test with our service worker, I ran this in the context of the Shaka Player demo. To test without a service worker, I ran this in the context of about:blank. To test with service workers generally, I ran this in the context of a separate service worker demo. For the SW test, I made sure that the "Bypass for network" checkbox in the "Application" tab of the Chrome debugger was unchecked. Reload after changing this setting to make sure it is in effect.

Here are my results in Chrome 68:

await xhrRedirect(url) await fetchRedirect(url)
Shaka demo "https://balancer.demo..." "https://cdn.demo..."
about:blank "https://cdn.demo..." "https://cdn.demo..."
SW demo "https://cdn.demo..." "https://cdn.demo..."

So it seems that we are at fault, although I don't yet understand why. Here's the working SW code for comparison: https://googlechrome.github.io/samples/service-worker/basic/service-worker.js

@joeyparrish
Copy link
Member

I found the trigger in our service worker. Although this effect is triggered by our service worker, it seems that the bad behavior is still in the browser, and it only affects XHR. When fetch is used, everything seems to work correctly.

The difference between our SW and the SW demo I tried is that ours uses event.respondWith on all requests. We then asynchronously open the cache and decide if we have a cached copy, and if not, we decide whether or not we should cache one now. If we should not cache a copy, we just call fetch and pass the results directly through.

The big difference is that the SW demo only calls event.respondWith on specific URLs, and it makes that decision synchronously.

During installation, our SW will store a bunch of demo app assets by relative URLs. Later, at fetch time, we are presented with an absolute URL to make a decision with. We can't compare that to the relative URL, so instead we check the cache. Checking the cache is async, so we have to take responsibility for the request (event.respondWith()) early.

When we take responsibility for all requests, we trigger what I suspect is a Chrome bug: the redirect URL from the fetch() call in the service worker is not propagated through to the XHR in the application.

We could/should stop managing all requests, but this will be tricky. If we stop using relative URLs, the SW in our demo app will not work in arbitrary locations. So we would need (I think) to map them to absolute URLs during installation/activation, then be able to look them up synchronously at fetch-time.

I also think it's worth tracking down the bug in Chrome if possible.

For now, as a workaround, you can simply upgrade to v2.4.4. Starting with v2.4.0, fetch is preferred over XHR.

@joeyparrish
Copy link
Member

Because this is working correctly with Fetch in v2.4, I'm going to move this issue to the backlog. I'd like to fix it, but it's not critical now.

@joeyparrish joeyparrish modified the milestones: v2.5, Backlog Aug 30, 2018
@joeyparrish joeyparrish changed the title Redirect URL not available on Chrome with service worker Redirect URL not available from XHR on Chrome with service worker Aug 30, 2018
@joeyparrish joeyparrish added the type: external An issue with an external dependency; not our issue; sometimes kept open for tracking label Aug 30, 2018
@avelad
Copy link
Collaborator Author

avelad commented Sep 11, 2018

With the commit d6720cc I see that Fetch is used for fallback, then the issue will appear again in v2.5

@joeyparrish
Copy link
Member

I believe we have a change coming soon to add the new functionality to Fetch, as well.

@joeyparrish joeyparrish removed the type: external An issue with an external dependency; not our issue; sometimes kept open for tracking label Apr 29, 2019
@joeyparrish joeyparrish self-assigned this Apr 29, 2019
@joeyparrish joeyparrish removed this from the Backlog milestone Apr 29, 2019
@joeyparrish
Copy link
Member

I believe we can fix our demo's service worker to avoid this browser behavior. I hope to have an update out soon.

@shaka-bot shaka-bot added this to the v2.5 milestone Apr 29, 2019
@shaka-project shaka-project locked and limited conversation to collaborators Jun 29, 2019
@shaka-bot shaka-bot added the status: archived Archived and locked; will not be updated label Apr 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status: archived Archived and locked; will not be updated type: bug Something isn't working correctly
Projects
None yet
Development

No branches or pull requests

4 participants