diff --git a/spec/unit/ReEmitter.spec.ts b/spec/unit/ReEmitter.spec.ts index d1c853fe6..0f139427f 100644 --- a/spec/unit/ReEmitter.spec.ts +++ b/spec/unit/ReEmitter.spec.ts @@ -23,6 +23,10 @@ class EventSource extends EventEmitter { doTheThing() { this.emit(EVENTNAME, "foo", "bar"); } + + doAnError() { + this.emit('error'); + } } class EventTarget extends EventEmitter { @@ -46,4 +50,23 @@ describe("ReEmitter", function() { // also the source object of the event which re-emitter adds expect(handler).toHaveBeenCalledWith("foo", "bar", src); }); + + it("Doesn't throw if no handler for 'error' event", function() { + const src = new EventSource(); + const tgt = new EventTarget(); + + const reEmitter = new ReEmitter(tgt); + reEmitter.reEmit(src, ['error']); + + // without the workaround in ReEmitter, this would throw + src.doAnError(); + + const handler = jest.fn(); + tgt.on('error', handler); + + src.doAnError(); + + // Now we've attached an error handler, it should be called + expect(handler).toHaveBeenCalled(); + }); }); diff --git a/src/ReEmitter.ts b/src/ReEmitter.ts index f65912aa4..b1060cbb8 100644 --- a/src/ReEmitter.ts +++ b/src/ReEmitter.ts @@ -31,6 +31,17 @@ export class ReEmitter { // such as read receipt listeners on the client class which won't have the context // of the room. const forSource = (...args) => { + // EventEmitter special cases 'error' to make the emit function throw if no + // handler is attached, which sort of makes sense for making sure that something + // handles an error, but for re-emitting, there could be a listener on the original + // source object so the test doesn't really work. We *could* try to replicate the + // same logic and throw if there is no listener on either the source or the target, + // but this behaviour is fairly undesireable for us anyway: the main place we throw + // 'error' events is for calls, where error events are usually emitted some time + // later by a different part of the code where 'emit' throwing because the app hasn't + // added an error handler isn't terribly helpful. (A better fix in retrospect may + // have been to just avoid using the event name 'error', but backwards compat...) + if (eventName === 'error' && this.target.listenerCount('error') === 0) return; this.target.emit(eventName, ...args, source); }; source.on(eventName, forSource);