Skip to content

Commit

Permalink
fix(evented): log an error on invalid type (#7067)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
gkatsev authored Jan 26, 2021
1 parent 22b5535 commit 85575db
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 12 deletions.
11 changes: 9 additions & 2 deletions src/js/mixins/evented.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import * as Events from '../utils/events';
import * as Fn from '../utils/fn';
import * as Obj from '../utils/obj';
import EventTarget from '../event-target';
import log from '../utils/log';

const objName = (obj) => {
if (typeof obj.name === 'function') {
Expand Down Expand Up @@ -444,8 +445,14 @@ const EventedMixin = {
const type = event && typeof event !== 'string' ? event.type : event;

if (!isValidEventType(type)) {
throw new 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.');
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.';

if (event) {
(this.log || log).error(error);
} else {
throw new Error(error);
}
}
return Events.trigger(this.eventBusEl_, event, hash);
}
Expand Down
41 changes: 31 additions & 10 deletions test/unit/mixins/evented.test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/* eslint-env qunit */
import sinon from 'sinon';
import evented from '../../../src/js/mixins/evented';
import log from '../../../src/js/utils/log';
import * as Dom from '../../../src/js/utils/dom';
import * as Obj from '../../../src/js/utils/obj';

Expand Down Expand Up @@ -64,26 +65,46 @@ QUnit.test('evented() with custom element', function(assert) {

QUnit.test('trigger() errors', function(assert) {
class Test {}
const targeta = evented({});

const tester = new Test();
const targeta = evented(tester);
const targetb = 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';

[targeta, targetb, targetc].forEach((target) => {
const createTest = (lg) => (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(' '), triggerError, 'expected error');
assert.throws(() => target.trigger({}), triggerError, 'expected error');
assert.throws(() => target.trigger({type: ''}), triggerError, 'expected error');
assert.throws(() => target.trigger({type: ' '}), triggerError, 'expected error');
assert.throws(() => target.trigger(), /^Error: Invalid event type/, 'threw an error when tried to trigger without an event');

target.trigger(' ');
target.trigger({});
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_;

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) {
Expand Down

0 comments on commit 85575db

Please sign in to comment.