vjs.hasData Error Spam

[Manbearpixel]

Developing a video playing website on AngularJS and using videoJS as the video player. The video player is contained within a controller, which gets destroyed and comes back upon a new video being loaded. In angular, I can run functions prior to a controller being destroyed, this is where I call videoObject.dispose()

Expectation:
I expected videoObject.dispose() to handle all the necessary removal of information related to the video player.

Reality:
It seems that if a video is exited (controller is destroyed) prior to the video fully loading, an error is thrown repeatedly from vjs.hasData (see screenshot below). This error seems to be thrown every second (assuming an event is still active somewhere that wasn't cleared) I tried tracking down the .dispose() function and commenting out this.trigger({ type: 'dispose', 'bubbles': false }); but that seems to have no effect. If I try checking that el is not null or wrapping it in a try/catch it still goes through but instead the error comes from vjs.trigger (see screenshot below)

At this point I am unsure how to debug this and am curious if anyone else has encountered this problem. I will try to create a plunkr that might replicate this issue since the site is currently not live.

vjs.hasData Screenshot

vjs.trigger Screenshot

videoPlayer.dispose() call

[gkatsev]

I think this is similar to the #1481 issue where some timer or something is still active and keeps trying to do something with the video after the fact.

[Manbearpixel]

@gkatsev Possibly, I commented on that issue but was informed to create a new one since it might not relate. In any case, I wish I had time to look into this more.

[gkatsev]

Yeah, it's a separate issue. Just commenting that the mechanism is similar.

[mmcc]

@gkatsev From our chat conversation, he was having the problem in 4.7 as well, so #1481 most likely is not related.

Edit: woops, must have been out of date when I commented because I didn't see that last one.

[mmcc]

@Manbearpixel when we left this in chat you were having trouble reproducing. Where are you now on this one?

[Manbearpixel]

@mmcc Unfortunately I have been unable to reproduce the problem in a plunkr.. However I do have a link to a now sandbox version of the web app I am working on where the problem still persists..

Steps to reproduce:

1. Go to http://tvisionprod21-14080601.elasticbeanstalk.com/ and open the console.log to watch for errors
2. Click on any video to begin playing
3. Once video begins to play simply navigate away from the video player (i.e. Go back in history, click another video, go to another page, etc..)

As you'll notice, the console.log will begin to flood with something along the lines of Uncaught TypeError: Cannot read property 'vdata1410215931241' of null

Site is using a local copy of VideoJS v4.8.0
Video player is getting destroyed upon exit with .dispose() method

I have noticed, however, that if I pause the video prior to exiting or navigating away from the video player that the error does not appear. Due to this, I have tried in the past to pause the video and then dispose but unfortunately that did no resolve the issue.

[mmcc]

I definitely see the problem there, but there's so much going on there on top of Video.js that there's simply no way we can help debug using the live example. We pushed out a patch release on Friday that included a fix for #1481, so you might want to update to that just in case the issue in 4.8.0 affected you for some reason.

[louisbl]

I had the same issue with a similar setup : the video player is contained within a Marionette.ItemView which is created/destroyed during the navigation between different page. Marionette View provide an onDestroy method where I call videoPlayer.dispose().

Sometimes this trigger a TypeError: el is undefined in vjs.hasData.

/**
 * Returns the cache object where data for an element is stored
 * @param  {Element} el Element to store data for.
 * @return {Object}
 * @private
 */
vjs.hasData = function (el) {
  var id = el[vjs.expando];
  return !(!id || vjs.isEmpty(vjs.cache[id]));
};

Following the callstack it came from the player triggering timeupdate from this setInterval:

vjs.MediaTechController.prototype.trackCurrentTime = function () {
  if (this.currentTimeInterval) {
    this.stopTrackingCurrentTime();
  }
  this.currentTimeInterval = setInterval(vjs.bind(this, function () {
    this.player().trigger('timeupdate');
  }), 250); // 42 = 24 fps // 250 is what Webkit uses // FF uses 15
};

I manage to workaround this issue by clearing manually the interval in the MediaTechController.dispose:

vjs.MediaTechController.prototype.dispose = function () {
  // Turn off any manual progress or timeupdate tracking
  if (this.manualProgress) {
    this.manualProgressOff();
  }

  if (this.manualTimeUpdates) {
    this.manualTimeUpdatesOff();
  }

 clearInterval(this.currentTimeInterval);
// ^^^^ here

  vjs.Component.prototype.dispose.call(this);
};

I have noticed, however, that if I pause the video prior to exiting or navigating away from the video player that the error does not appear. Due to this, I have tried in the past to pause the video and then dispose but unfortunately that did no resolve the issue.

I'm not really sure if this is related but it seems there is the same clearInterval triggered on pause event here:

this.player().on('pause', vjs.bind(this, this.stopTrackingCurrentTime));

with stopTrackingCurrentTime

vjs.MediaTechController.prototype.stopTrackingCurrentTime = function(){
clearInterval(this.currentTimeInterval);

[schmod]

Came in to say the same thing as @louisbl.

The fix that he proposes seems sane. If anybody wants a PR or concrete test case (ie. a jsfiddle), I can help out with that.

[Manbearpixel]

Saw the fix @louisbl had posted a couple days ago. Will be testing it out
later.
On Sep 15, 2014 10:08 AM, "Andrew Schmadel" [email protected]
wrote:

Came in to say the same thing as @louisbl https://github.com/louisbl.

The fix that he proposes seems sane. If anybody wants a PR or concrete
test case (ie. a jsfiddle), I can help out with that.

—
Reply to this email directly or view it on GitHub
#1484 (comment).

[Manbearpixel]

The fix suggested by @louisbl seems to have patched this issue. Changing the view in the middle of playback no longer triggers the spam of JS errors in the console. Seems pretty safe to say the variable currentTimeInterval is the culprit.

[techguydave]

The fix by @louisbl also worked for me on Angular.

[mmcc]

@heff and I discussed a way for us to avoid having to clear each interval we make on dispose in #1521, which if we're thinking things through correctly, should fix this issue as well.

TL;DR - We want to store a reference to each timeout on the player object so we can just clear them all on dispose rather than setting up a dispose listener for each one.

[jussi-kalliokoski]

@mmcc 👍

Aside from the error spam and memory leakage, this doesn't seem to have too terrible ramifications, but it's a bit annoying to develop with this bug active since I have "Break on uncaught exception" on in the devtools. :)

[heff]

Thanks for the fix @louisbl!

Looking at this more closely, if adding the extra clearInterval helps fix this then it appears that somehow this.manualTimeUpdates is reporting the wrong thing.

In the dispose function we have:

if (this.manualTimeUpdates) {
  this.manualTimeUpdatesOff();
}

Which internally would call clearInterval(this.currentTimeInterval) if this.manualTimeUpdates was true. And if those errors are happening then it should be true. I can't immediately see how that's happening.

So the fix @mmcc mentioned will help this, but somewhere the code must be wrong for this to be happening because we're at least already trying to clean up the interval.

[blimmer]

I'm seeing the same problem using 4.10.2 . Here's a JSBin using EmberJS where the problem can be reproduced every time.

EDIT: @schmod here's a JSBin with the fix recommended by @louisbl . It fixes the issue. It would be great to get a fix in for this.

[gary-feng]

I got the same problem in 4.10.2 with similar steps.
It seems like there is interval cannot be cleared up successfully.
Is there any update for this issue?

[mmcc]

Yes, there are multiple hotfixes out that address issues like this (the former addresses this one specifically). These should be pulled in later this week.

[benjipott]

new on this issue

[mmcc]

This shouldn't be an issue anymore, it should have been closed along with #1656 and #1642. @benjipott can you put together a reduced test case?

[gkatsev]

Generally if you are seeing this, it means there's some other unrelated bug further up the chain.

[saxena-gaurav]

this should have been fixed by #2125