1
0
mirror of https://github.com/videojs/video.js.git synced 2024-12-10 11:10:19 +02:00

fix(evented): log an error on invalid type (#7067)

Follow up from #6982. We previously threw an error, but we've seen it
happen unexpectedly. Instead, we should log an error.
We will still throw an error if the event is undefined or null.

Here, if we have a `log` object on the current object, we should use it,
otherwise, we use a default `log` object.
This commit is contained in:
Gary Katsevman 2021-01-26 13:56:41 -05:00 committed by GitHub
parent 22b5535187
commit 85575dbaba
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 40 additions and 12 deletions

View File

@ -8,6 +8,7 @@ import * as Events from '../utils/events';
import * as Fn from '../utils/fn'; import * as Fn from '../utils/fn';
import * as Obj from '../utils/obj'; import * as Obj from '../utils/obj';
import EventTarget from '../event-target'; import EventTarget from '../event-target';
import log from '../utils/log';
const objName = (obj) => { const objName = (obj) => {
if (typeof obj.name === 'function') { if (typeof obj.name === 'function') {
@ -444,8 +445,14 @@ const EventedMixin = {
const type = event && typeof event !== 'string' ? event.type : event; const type = event && typeof event !== 'string' ? event.type : event;
if (!isValidEventType(type)) { if (!isValidEventType(type)) {
throw new Error(`Invalid event type for ${objName(this)}#trigger; ` + const error = `Invalid event type for ${objName(this)}#trigger; ` +
'must be a non-empty string or object with a type key that has a non-empty value.'); 'must be a non-empty string or object with a type key that has a non-empty value.';
if (event) {
(this.log || log).error(error);
} else {
throw new Error(error);
}
} }
return Events.trigger(this.eventBusEl_, event, hash); return Events.trigger(this.eventBusEl_, event, hash);
} }

View File

@ -1,6 +1,7 @@
/* eslint-env qunit */ /* eslint-env qunit */
import sinon from 'sinon'; import sinon from 'sinon';
import evented from '../../../src/js/mixins/evented'; import evented from '../../../src/js/mixins/evented';
import log from '../../../src/js/utils/log';
import * as Dom from '../../../src/js/utils/dom'; import * as Dom from '../../../src/js/utils/dom';
import * as Obj from '../../../src/js/utils/obj'; import * as Obj from '../../../src/js/utils/obj';
@ -64,26 +65,46 @@ QUnit.test('evented() with custom element', function(assert) {
QUnit.test('trigger() errors', function(assert) { QUnit.test('trigger() errors', function(assert) {
class Test {} class Test {}
const targeta = evented({});
const tester = new Test();
const targeta = evented(tester);
const targetb = evented(new Test()); const targetb = evented(new Test());
const targetc = evented(new Test()); const targetc = evented(new Test());
const targetd = evented({});
tester.log = log.createLogger('tester');
sinon.stub(log, 'error');
sinon.stub(tester.log, 'error');
targetc.name_ = 'foo'; targetc.name_ = 'foo';
[targeta, targetb, targetc].forEach((target) => { const createTest = (lg) => (target) => {
const objName = target.name_ || target.constructor.name || typeof target; const objName = target.name_ || target.constructor.name || typeof target;
const triggerError = errors.trigger(objName);
assert.throws(() => target.trigger(), triggerError, 'expected error'); assert.throws(() => target.trigger(), /^Error: Invalid event type/, 'threw an error when tried to trigger without an event');
assert.throws(() => target.trigger(' '), triggerError, 'expected error');
assert.throws(() => target.trigger({}), triggerError, 'expected error'); target.trigger(' ');
assert.throws(() => target.trigger({type: ''}), triggerError, 'expected error'); target.trigger({});
assert.throws(() => target.trigger({type: ' '}), triggerError, 'expected error'); target.trigger({type: ''});
target.trigger({type: ' '});
assert.ok(lg.error.called, 'error was called');
assert.equal(lg.error.callCount, 4, 'log.error called 4 times');
assert.ok(lg.error.calledWithMatch(new RegExp(`^Invalid event type for ${objName}#trigger`)), 'error called with expected message');
delete target.eventBusEl_; delete target.eventBusEl_;
assert.throws(() => target.trigger({type: 'foo'}), errors.target(objName, 'trigger'), 'expected error'); assert.throws(() => target.trigger({type: 'foo'}), new RegExp(`^Error: Invalid target for ${objName}#trigger`), 'expected error');
});
lg.error.reset();
};
createTest(targeta.log)(targeta);
[targetb, targetc, targetd].forEach(createTest(log));
targeta.log.error.restore();
log.error.restore();
}); });
QUnit.test('on(), one(), and any() errors', function(assert) { QUnit.test('on(), one(), and any() errors', function(assert) {