video.js icon indicating copy to clipboard operation
video.js copied to clipboard

Player Keeps Spewing Errors on CORS Failure

Open nikolaosinlight opened this issue 3 years ago • 21 comments

I have a project using video.min.js version 7.11.7 that is playing AWS signed URL content through Cloudfront.

For the most part everything works very well except when a signed URL expires.

e.g. say I do the following:

  • Set a URL to expire in 15 seconds (to speed up the issue)
  • Load a video page containing the player and wait 20 seconds
  • Hit the play button
  • I start seeing rapidly increasing warnings, errors and user messages

After just 20 seconds of playback I have (in the browser console):

  • 1790 error messages = 644 user messages + 1141 errors

At about 100 message/sec this starts making the browser very unresponsive and I cannot copy and paste the error but rather can only provide a screen shot.

I would be happy with just trapping the error and redirecting the user to a page telling them that the link expired but the following code does not do the job (even though I tried multiple variants of error handling:

    var player = videojs("vod");
    const playerElement = document.querySelector("#vod")
    playerElement.addEventListener("error", (event) => {
      window.location = 'https://www.google.com';
    })
    player.tech().on('error', function(e) {
      window.location = 'https://www.google.com';
    });
    player.on('error', function() {
      window.location = 'https://www.google.com';
    });
    player.ready(function() {
      const cacheLink = '{{{videoUrl}}}'
      videojs.Vhs.xhr.beforeRequest = function(options) {
        // This is done to get the GET params of MPEG-DASH signed URL to be added to the metadata URLs
        const requestURL = new URL(options.uri);
        const baseURL = new URL(cacheLink);
        requestURL.search = baseURL.search;
        options.uri = requestURL.href
        return options;
      };
      player.src({
        src: '{{{videoUrl}}}',
        type: '{{videoUrlType}}',
        withCredentials: true
      });
    });

A screen shot of the errors are here

A screen shot of the warnings are here

Ultimately, I would be happy to just redirect the user to another page on these errors.

Hoping someone has ideas.

Thank You,

--Nikolaos

nikolaosinlight avatar May 12 '21 04:05 nikolaosinlight

👋 Thanks for opening your first issue here! 👋

If you're reporting a 🐞 bug, please make sure you include steps to reproduce it. We get a lot of issues on this repo, so please be patient and we will get back to you as soon as we can. To help make it easier for us to investigate your issue, please follow the contributing guidelines.

welcome[bot] avatar May 12 '21 04:05 welcome[bot]

Anyone have any thoughts on trapping the feed errors for this issue?

Once the errors starts spewing the browser becomes unresponsive.

--Nikolaos

nikolaosinlight avatar May 18 '21 06:05 nikolaosinlight

Based on your explanation it looks very similar to this issue https://github.com/videojs/http-streaming/issues/975 which should've been fixed in https://github.com/videojs/http-streaming/pull/1038, which should be available in Video.js 7.11.7. Would you be able to share a test stream? If you can't share publicly but can share privately, feel free to ping me on Slack.

gkatsev avatar May 19 '21 00:05 gkatsev

Sorry for the late reply so I can provide a feed to test privately to your email however I am not sure if the issue I am describing is clear. One part is the spewing of errors which I believe the above addresses but the other part is if the user opens a video stream that is expired then the resulting 403 should be caught by the player, it should stop attempting to play and an error event should be triggered. That way we can present the user with a message that the video has expired or whatever. You can see that I have tried 3 different ways to catch the error and none work. What am I missing?

nikolaosinlight avatar May 21 '21 21:05 nikolaosinlight

Just took a look and I am using the version that has the fix: * Video.js 7.11.7 <http://videojs.com/> Also did some more tests and I think I have a better idea of the issue - here are 3 scenarios and under each I will explain what I think is going on:

  1. If I load the page with an expired video stream then indeed an error event is triggered and the page redirects to Google.
  • This makes total sense as an error event is clearly raised and redirecting to Google occurs....
  1. If I load the page with a video stream that expires say in 15 seconds and start playing before then there are no issues. At least none that I can see with a small video file.
  • This too makes sense as the metadata .mpd file and the video streams contained within all are non-expired....
  1. If I load the page with a video stream that say expires say in 15 seconds and then wait 20 seconds and try to play it the video stream start to play and the errors spew.
  • What I think is happening is that when the page loads the manifest .mpd file is non-expired and loads fine however the video stream URLs contained within are not loaded yet and when they are loaded those expired URLs cause errors to spew.

The video player needs to be able to trap 403 errors in not just loading the manifest file but in the video streams contained within vs. having those continue to spew errors. Does this make sense... is there a solution to this problem?

--Nikolaos

nikolaosinlight avatar May 22 '21 00:05 nikolaosinlight

@gkatsev are you still interested in trying to work to resolve this issue?

I sent you an email privately and can provide a stream but need to know what you want me to provide for your testing....

--Nikolaos

nikolaosinlight avatar Jun 03 '21 18:06 nikolaosinlight

Yes! Sorry, we've just been super busy with other things that I haven't had a chance to take a look.

gkatsev avatar Jun 03 '21 18:06 gkatsev

This could probably a Chrome issue. We got it squashed by upgrading Chrome to the lastest version, Version 91.0.4472.106 (Build officiel) (64 bits) as of time writing. EDIT We had to upgrade manually since it was not pushed by Chrome.

arsene-glanum avatar Jun 17 '21 11:06 arsene-glanum

Maybe, we also recently finally added maxPlaylistRetries https://github.com/videojs/http-streaming#maxplaylistretries which limits how long we keep retrying when a playlist errors out. The default is Infinity, which is the current behavior.

gkatsev avatar Jun 24 '21 19:06 gkatsev

I am on Chrome 92.0.4515.131 and this issue is still happening with video.js version 2.10.1

aaronpk avatar Aug 18 '21 20:08 aaronpk

I am on Mac OS (Catalina) using Chrome Version 95.0.4638.69 (Official Build) (x86_64) and I have set:

    player.nuevo({
      maxPlaylistRetries: 5,
    });

and I am sorry to say that the issue just appeared (had not tested it for many months so I don't think it ever went away): Screen Shot 2021-10-31 at 4 55 11 AM

This is extremely problematic as this will run up our CDN bill to no end when say a session expires....

Hopefully there is a solution / work around....

nikolaosinlight avatar Oct 31 '21 09:10 nikolaosinlight

maxPlaylistRetries is a VHS option. You'd need to include it in the player setup options, e.g. videojs('myPlayer', {html5: {vhs: {maxPlaylistRetries: 5}}})

mister-ben avatar Oct 31 '21 18:10 mister-ben

@mister-ben ... Thanks for your reply. A few simple questions:

1.) I am using a paid plugin and not sure if that option works. Is there a way to pass in this option but not in setup? Something like: videojs.Vhs.maxPlaylistRetries = 5;

2.) Does this count "consecutive" retries OR just "total number" of retries. I hope it is the former and if so is 5 a sensible value or would you recommend another value?

3.) Assuming this works (need to test)... any reason why there isn't a default value OOB vs. "unlimited" you might be aware of? I have to think that video feeds are largely protected and this is quite a common problem.

4.) lastly, I have been thinking about alternative approaches so perhaps I should disclose the issue. The issue is that the signed cookie expires and then video fails with CORS errors. I was thinking if I keep track of the expiration... perhaps I could do something like add a check on the "progress" event and dispose of the player and throw a popup saying the video has expired to avoid hitting the CORS error. Basically I am looking for an event that as more of the video feed is being loaded I can cut it off prematurely especially if I know it will fail.

Makes sense? Any alternative suggestions? Much Appreciated.

nikolaosinlight avatar Oct 31 '21 19:10 nikolaosinlight

  1. You can set it via videojs.options.html5 = { vhs: {maxPlaylistRetries: 5}} or something like that.
  2. It counts consecutive, so, if a playlist recovers, it should reset the retries.
  3. The reason it's unlimited by default is that it was the previous behavior. Plus, we can't really know whether the stream is having issues due to network issues and will recover, or because the stream has ended. Generally, servers should end a stream properly (for example, adding an end-list tag to HLS playlist) before making it be unavailable on the server.
  4. With a maxPlaylistRetries, we should stop requesting playlists eventually, so, you shouldn't need to do that. However, you can definitely do that by following this:
  player.tech().on('retryplaylist', () => {
      do_something()
  });

gkatsev avatar Nov 01 '21 20:11 gkatsev

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Jan 08 '22 23:01 stale[bot]

@gkatsev

I added the following implementation.... Essentially if there are 20 errors in a 4 second window then likely that the signed cookies that the user had have expired so reload the page and will either need to sign in or get updated cookies.

Problem is retryplaylist is never hit. Here is the code I used:

    let player = videojs("vod");
    let playerRetries = 0;
    let playerRetriesReset = setInterval(playerRetriesTimer, 4000);
    function playerRetriesTimer() {
      playerRetries = 0;
    }

    player.ready(function() {
      player.tech().on('retryplaylist', () => {
        playerRetries++;
        if (playerRetries > 20) {
          window.location.reload();
        }
      });
      videojs.Vhs.xhr.beforeRequest = function(options) {
        options.withCredentials = true;  {{!-- needed for 2nd+ request (link inside manifest) to send CORS request header --}}
      }
      player.src({
        src: videoUrl,
        type: videoType,
        withCredentials: true  {{!-- needed for initial request to send CORS request header --}}
      });
      player.loadTracks( { kind:"chapters", src:"{{{videoVtt}}}", srclang:"en"} );

I also tried adding the retryplaylist function outside the player.ready function and still nothing gets hit in retryplaylist.

I also tried setting the following to no avail: videojs('myPlayer', {html5: {vhs: {maxPlaylistRetries: 5}}})

I am going to guess that retryplaylist will only get triggered once the stream is already playing BUT not at the start when the manifest gets loaded. Either that or for some reason something in the player that I am using is limiting this but I don't have any other issues with any other videojs errors.

I don't know why videojs cannot just catch errors and stop playing OR ideally allow users to reload the page if there are many errors. Instead it just happily spews 10's of thousands of errors in a very short time frame with no upper bound :-(

Any thoughts / ideas?

--Nikolaos

nikolaosinlight avatar Jan 29 '22 06:01 nikolaosinlight

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Apr 28 '22 04:04 stale[bot]

  1. You can set it via videojs.options.html5 = { vhs: {maxPlaylistRetries: 5}} or something like that.
  2. It counts consecutive, so, if a playlist recovers, it should reset the retries.
  3. The reason it's unlimited by default is that it was the previous behavior. Plus, we can't really know whether the stream is having issues due to network issues and will recover, or because the stream has ended. Generally, servers should end a stream properly (for example, adding an end-list tag to HLS playlist) before making it be unavailable on the server.
  4. With a maxPlaylistRetries, we should stop requesting playlists eventually, so, you shouldn't need to do that. However, you can definitely do that by following this:
  player.tech().on('retryplaylist', () => {
      do_something()
  });

@gkatsev player.tech() is undefined for me. Is there still a proper way to tap into retryplaylist event in current version 7.15? image

mattcwebster avatar May 11 '22 15:05 mattcwebster

@mattcwebster yes, player.tech() is the way. The only time when it's possible for tech() to be undefined is if the player isn't ready yet. Try wrapping changing that line to something like:

this.vjs.ready(function() {
  console.log(this.vjs.tech());
})

You can also pass in a ready function as the 3rd argument to videojs():

this.vjs = videojs('my-player', optionsObject, function() {
  // you can either use this.vjs here
  // or `this` is a reference to the player as well
})

Hope this helps.

gkatsev avatar May 11 '22 16:05 gkatsev

Thanks @gkatsev ill give that a shot

mattcwebster avatar May 11 '22 17:05 mattcwebster

@mattcwebster yes, player.tech() is the way. The only time when it's possible for tech() to be undefined is if the player isn't ready yet. Try wrapping changing that line to something like:

this.vjs.ready(function() {
  console.log(this.vjs.tech());
})

You can also pass in a ready function as the 3rd argument to videojs():

this.vjs = videojs('my-player', optionsObject, function() {
  // you can either use this.vjs here
  // or `this` is a reference to the player as well
})

Hope this helps.

@gkatsev that did it image

mattcwebster avatar May 11 '22 23:05 mattcwebster