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

fix: Provide a fallback to GET request when HEAD request fails #5986

Merged
merged 2 commits into from Dec 5, 2023

Conversation

avelad
Copy link
Collaborator

@avelad avelad commented Dec 4, 2023

Fixes #5959

@avelad avelad added type: bug Something isn't working correctly priority: P2 Smaller impact or easy workaround labels Dec 4, 2023
@avelad avelad added this to the v5.0 milestone Dec 4, 2023
@shaka-bot
Copy link
Collaborator

shaka-bot commented Dec 4, 2023

Incremental code coverage: 75.00%

lib/net/networking_utils.js Show resolved Hide resolved
@avelad avelad requested a review from theodab December 5, 2023 07:15
@avelad avelad merged commit 1af93e6 into shaka-project:main Dec 5, 2023
11 of 16 checks passed
@avelad avelad deleted the mimetype-fallback-get branch December 5, 2023 07:21
@Robloche
Copy link
Contributor

Robloche commented Dec 5, 2023

Thanks for the quick fix & merge.

Now, the HEAD request (don't know why there're 2 of them) is followed be a GET request.
But in my specific case, the response status is 302 Found, meaning that the resource has temporarily moved.
And it looks like the redirection is not followed:
image

image

If I manually enter the URL https://ntg-zeop-content.netgemplatform.net/api/channels?id=coeurocean&deviceId=1d46bbfc-ffa4-55e0-cc80-b4c9a8c92478209062a1&ip=52.166.120.253%3A4352 in my browser, the redirection is followed and the .m3u8 manifest is correctly downloaded.

What do you think? Is it really a redirection issue?

@Robloche
Copy link
Contributor

Robloche commented Dec 6, 2023

@avelad Do you have any idea about my issue? Is it a problem on my side or in Shaka Player?

@Robloche
Copy link
Contributor

Robloche commented Dec 6, 2023

OK, my bad. We forgot to set Access-Control-Allow-Origin on our server.
It's now done but the GET request is not mage any longer...
I still see the two 404 errors for the HEAD request, though.

@Robloche
Copy link
Contributor

Robloche commented Dec 7, 2023

OK, once again it's on our side. HEAD request is not allowed on our server.
I asked someone to allow it.
When it's done, I'll check again and let you know...

@Robloche
Copy link
Contributor

Robloche commented Dec 7, 2023

OK, with the following 3 things, everything now works correctly:

  • your fix,
  • the setting of Access-Control-Allow-Origin,
  • allowing HEAD method.

Thanks a lot!

@shaka-bot shaka-bot added the status: archived Archived and locked; will not be updated label Feb 3, 2024
@shaka-project shaka-project locked as resolved and limited conversation to collaborators Feb 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
priority: P2 Smaller impact or easy workaround status: archived Archived and locked; will not be updated type: bug Something isn't working correctly
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow no-cors on fetch request
4 participants