throw error when plugin is missing

[thijstriemstra]

Improve error for missing plugin, refs #1928.

[gkatsev]

rather than using try/catch, we can see whether this[key] is a function and then log an error. This way it continues working even if a plugin is missing but we still have a better error log.

[thijstriemstra]

I was trying to figure out what the right way to go was here but, in my case, my video.js player is just an empty shell without the plugin and I want to abort further loading. It sounds like logging it would not help me determine if the video.js player (and required plugin) is broken?

[thijstriemstra]

So maybe log an error, fire an error event?

[gkatsev]

Use videojs.log.error()

[thijstriemstra]

Could anyone help me figure out why this build is dying?

[heff]

It looks like it doesn't like window.throws
https://travis-ci.org/videojs/video.js/builds/53280031#L466

[gkatsev]

throws needs to be added here https://github.com/videojs/video.js/blob/master/.jshintrc#L14-L45

[gkatsev]

and then you should use throws without referencing it from window.

[thijstriemstra]

Ok, thanks I'll check it out asap. I used the window reference because I saw this being used in text track tests, doesn't seem any other tests that check for a throw.

[thijstriemstra]

It doesn't fix my local jshint 'issues' though, but I guess that's a different PR.

[thijstriemstra]

Adding throws to jshint solved the problem in the jshint build step but not the minify:tests target:

test/unit/plugins.js:168: ERROR - Parse error. identifier is a reserved word
  throws(function() {
  ^

Since I'll just use videojs.log.error, and not be checking for a raised exception, I'm not going to bother looking into that error further.

[thijstriemstra]

Why are the tests minified anyway? This causes unexpected results where the key name is minified it seems:

    Expected: nonExistingPlugin
    Actual: Yf
        at /Users/foo/Sites/projects/video.js/node_modules/qunitjs/qunit/qunit.js:1540
        at /Users/foo/Sites/projects/video.js/build/files/test.minified.video.js:344
        at /Users/foo/Sites/projects/video.js/node_modules/qunitjs/qunit/qunit.js:1303
        at /Users/foo/Sites/projects/video.js/node_modules/qunitjs/qunit/qunit.js:1463
        at process (/Users/foo/Sites/projects/video.js/node_modules/qunitjs/qunit/qunit.js:1016)
        at /Users/foo/Sites/projects/video.js/node_modules/qunitjs/qunit/qunit.js:186

I guess making it a string key in the test fixes it.

[thijstriemstra]

Updated pull request and now a plugin that does not exist logs an error.

[heff]

We don't need throws in the globals any more. But otherwise looks good to me.