From 85575dbababf461463902b52a95891518b53b6b7 Mon Sep 17 00:00:00 2001
From: Gary Katsevman <git@gkatsev.com>
Date: Tue, 26 Jan 2021 13:56:41 -0500
Subject: [PATCH] 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.
---
 src/js/mixins/evented.js         | 11 +++++++--
 test/unit/mixins/evented.test.js | 41 ++++++++++++++++++++++++--------
 2 files changed, 40 insertions(+), 12 deletions(-)

diff --git a/src/js/mixins/evented.js b/src/js/mixins/evented.js
index 29eae816e..045493d37 100644
--- a/src/js/mixins/evented.js
+++ b/src/js/mixins/evented.js
@@ -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') {
@@ -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);
   }
diff --git a/test/unit/mixins/evented.test.js b/test/unit/mixins/evented.test.js
index 4ecc4ce94..54d3e30b2 100644
--- a/test/unit/mixins/evented.test.js
+++ b/test/unit/mixins/evented.test.js
@@ -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';
 
@@ -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) {