wip(node:events): fix emit when no `error` listener attached to EEderrick/fix/event-emitter-emit-throw-native

Note: Deprecated branch, rewriting `EE.prototype.emit` in JS

7 files changed, 87 insertions, 26 deletions

diff --git a/src/bun.js/bindings/JSReadableHelper.cpp b/src/bun.js/bindings/JSReadableHelper.cpp
index 271bfb358..c6204dc0a 100644
--- a/src/bun.js/bindings/JSReadableHelper.cpp
+++ b/src/bun.js/bindings/JSReadableHelper.cpp
@@ -37,7 +37,10 @@ static bool callRead(JSValue stream, JSFunction* read, JSC::MarkedArgumentBuffer
     WTF::NakedPtr<JSC::Exception> exceptionPtr;
     JSC::CallData callData = JSC::getCallData(read);
     JSValue ret = JSC::call(lexicalGlobalObject, read, callData, JSValue(stream), WTFMove(args), exceptionPtr);
-    if (auto* exception = exceptionPtr.get()) {
+    auto throwScope = DECLARE_THROW_SCOPE(vm);
+
+    auto* exception = exceptionPtr.get();
+    if (UNLIKELY(exception)) {
         JSC::Identifier errorEventName = JSC::Identifier::fromString(vm, "error"_s);
         if (emitter.hasEventListeners(errorEventName)) {
             args.clear();
@@ -46,7 +49,15 @@ static bool callRead(JSValue stream, JSFunction* read, JSC::MarkedArgumentBuffer
                 val = jsUndefined();
             }
             args.append(val);
-            emitter.emitForBindings(errorEventName, args);
+
+            auto catchScope = DECLARE_CATCH_SCOPE(vm);
+            auto result = emitter.emitForBindings(lexicalGlobalObject, errorEventName, args);
+            auto* exception = catchScope.exception();
+            if (UNLIKELY(exception)) {
+                catchScope.clearException();
+                reportException(lexicalGlobalObject, exception);
+                return false;
+            }
         } else {
             reportException(lexicalGlobalObject, exception);
         }
@@ -149,7 +160,14 @@ JSC_DEFINE_HOST_FUNCTION(jsReadable_resume, (JSGlobalObject * lexicalGlobalObjec
     auto eventType = clientData->builtinNames().resumePublicName();
     MarkedArgumentBuffer args;
 
-    emitter.emitForBindings(eventType, args);
+    auto catchScope = DECLARE_CATCH_SCOPE(vm);
+    auto result = emitter.emitForBindings(lexicalGlobalObject, eventType, args);
+    auto* exception = catchScope.exception();
+    if (UNLIKELY(exception)) {
+        catchScope.clearException();
+        JSC::throwVMError(lexicalGlobalObject, throwScope, exception);
+        return JSValue::encode(JSValue {});
+    }
 
     flow(lexicalGlobalObject, stream, state);
 
@@ -185,7 +203,15 @@ EncodedJSValue emitReadable_(JSGlobalObject* lexicalGlobalObject, JSObject* stre
             throwTypeError(lexicalGlobalObject, throwScope, "Stream must be an EventEmitter"_s);
             return JSValue::encode(JSValue {});
         }
-        emitter->wrapped().emitForBindings(eventType, args);
+
+        auto catchScope = DECLARE_CATCH_SCOPE(vm);
+        auto result = emitter->wrapped().emitForBindings(lexicalGlobalObject, eventType, args);
+        auto* exception = catchScope.exception();
+        if (UNLIKELY(exception)) {
+            catchScope.clearException();
+            JSC::throwVMError(lexicalGlobalObject, throwScope, exception);
+            return JSValue::encode(JSValue {});
+        }
 
         state->setBool(JSReadableState::emittedReadable, false);
     }
diff --git a/src/bun.js/bindings/headers-cpp.h b/src/bun.js/bindings/headers-cpp.h
index dafdaed17..da0c916c5 100644
--- a/src/bun.js/bindings/headers-cpp.h
+++ b/src/bun.js/bindings/headers-cpp.h
@@ -1,4 +1,4 @@
-//-- AUTOGENERATED FILE -- 1679200292
+//-- AUTOGENERATED FILE -- 1679890150
 // clang-format off
 #pragma once
 
diff --git a/src/bun.js/bindings/headers.h b/src/bun.js/bindings/headers.h
index 5c727f397..6c5d61bc4 100644
--- a/src/bun.js/bindings/headers.h
+++ b/src/bun.js/bindings/headers.h
@@ -1,5 +1,5 @@
 // clang-format off
-//-- AUTOGENERATED FILE -- 1679530947
+//-- AUTOGENERATED FILE -- 1679890150
 #pragma once
 
 #include <stddef.h>
diff --git a/src/bun.js/bindings/webcore/EventEmitter.cpp b/src/bun.js/bindings/webcore/EventEmitter.cpp
index 4ea10587e..83cd28c03 100644
--- a/src/bun.js/bindings/webcore/EventEmitter.cpp
+++ b/src/bun.js/bindings/webcore/EventEmitter.cpp
@@ -104,18 +104,24 @@ bool EventEmitter::hasActiveEventListeners(const Identifier& eventType) const
     return data && data->eventListenerMap.containsActive(eventType);
 }
 
-bool EventEmitter::emitForBindings(const Identifier& eventType, const MarkedArgumentBuffer& arguments)
+bool EventEmitter::emitForBindings(JSC::JSGlobalObject* lexicalGlobalObject, const Identifier& eventType, const MarkedArgumentBuffer& arguments)
 {
     if (!scriptExecutionContext())
         return false;
 
-    emit(eventType, arguments);
+    auto result = emit(eventType, arguments);
+    if (UNLIKELY(result)) {
+        auto throwScope = DECLARE_THROW_SCOPE(lexicalGlobalObject->vm());
+        JSC::throwVMError(lexicalGlobalObject, throwScope, &*result);
+        return false;
+    }
     return true;
 }
 
-void EventEmitter::emit(const Identifier& eventType, const MarkedArgumentBuffer& arguments)
+std::optional<JSC::Exception> EventEmitter::emit(const Identifier& eventType, const MarkedArgumentBuffer& arguments)
 {
-    fireEventListeners(eventType, arguments);
+    auto result = fireEventListeners(eventType, arguments);
+    return result;
 }
 
 void EventEmitter::uncaughtExceptionInEventHandler()
@@ -169,28 +175,29 @@ Vector<JSObject*> EventEmitter::getListeners(const Identifier& eventType)
 }
 
 // https://dom.spec.whatwg.org/#concept-event-listener-invoke
-void EventEmitter::fireEventListeners(const Identifier& eventType, const MarkedArgumentBuffer& arguments)
+std::optional<JSC::Exception> EventEmitter::fireEventListeners(const Identifier& eventType, const MarkedArgumentBuffer& arguments)
 {
     ASSERT_WITH_SECURITY_IMPLICATION(ScriptDisallowedScope::isEventAllowedInMainThread());
 
     auto* data = eventTargetData();
     if (!data)
-        return;
+        return std::nullopt;
 
     auto* listenersVector = data->eventListenerMap.find(eventType);
     if (UNLIKELY(!listenersVector))
-        return;
+        return std::nullopt;
 
     bool prevFiringEventListeners = data->isFiringEventListeners;
     data->isFiringEventListeners = true;
-    innerInvokeEventListeners(eventType, *listenersVector, arguments);
+    auto result = innerInvokeEventListeners(eventType, *listenersVector, arguments);
     data->isFiringEventListeners = prevFiringEventListeners;
+    return result;
 }
 
 // Intentionally creates a copy of the listeners vector to avoid event listeners added after this point from being run.
 // Note that removal still has an effect due to the removed field in RegisteredEventListener.
 // https://dom.spec.whatwg.org/#concept-event-listener-inner-invoke
-void EventEmitter::innerInvokeEventListeners(const Identifier& eventType, SimpleEventListenerVector listeners, const MarkedArgumentBuffer& arguments)
+std::optional<JSC::Exception> EventEmitter::innerInvokeEventListeners(const Identifier& eventType, SimpleEventListenerVector listeners, const MarkedArgumentBuffer& arguments)
 {
     Ref<EventEmitter> protectedThis(*this);
     ASSERT(!listeners.isEmpty());
@@ -235,8 +242,11 @@ void EventEmitter::innerInvokeEventListeners(const Identifier& eventType, Simple
             auto errorIdentifier = JSC::Identifier::fromString(vm, eventNames().errorEvent);
             auto hasErrorListener = this->hasActiveEventListeners(errorIdentifier);
             if (!hasErrorListener || eventType == errorIdentifier) {
-                // If the event type is error, report the exception to the console.
-                Bun__reportUnhandledError(lexicalGlobalObject, JSValue::encode(JSValue(exception)));
+                // To match Node behavior, we need to throw when we can't emit an error event because there are no listeners or otherwise
+                // So we bubble it up
+                return std::optional<JSC::Exception> { *exception };
+                // // If the event type is error, report the exception to the console.
+                // Bun__reportUnhandledError(lexicalGlobalObject, JSValue::encode(JSValue(exception)));
             } else if (hasErrorListener) {
                 MarkedArgumentBuffer expcep;
                 JSValue errorValue = exception->value();
@@ -244,10 +254,17 @@ void EventEmitter::innerInvokeEventListeners(const Identifier& eventType, Simple
                     errorValue = JSC::jsUndefined();
                 }
                 expcep.append(errorValue);
-                fireEventListeners(errorIdentifier, WTFMove(expcep));
+                auto result = fireEventListeners(errorIdentifier, WTFMove(expcep));
+
+                // We bubble any exceptions which are not emitted as an error event
+                if (UNLIKELY(result)) {
+                    return result;
+                }
             }
         }
     }
+
+    return std::nullopt;
 }
 
 Vector<Identifier> EventEmitter::eventTypes()
diff --git a/src/bun.js/bindings/webcore/EventEmitter.h b/src/bun.js/bindings/webcore/EventEmitter.h
index b46bcff5d..972a8be62 100644
--- a/src/bun.js/bindings/webcore/EventEmitter.h
+++ b/src/bun.js/bindings/webcore/EventEmitter.h
@@ -3,7 +3,6 @@
 #include "root.h"
 
 #include "IdentifierEventListenerMap.h"
-#include "ExceptionOr.h"
 #include "ContextDestructionObserver.h"
 #include "ScriptWrappable.h"
 #include <memory>
@@ -49,13 +48,13 @@ public:
     WEBCORE_EXPORT void addListenerForBindings(const Identifier& eventType, RefPtr<EventListener>&&, bool, bool);
     WEBCORE_EXPORT void removeListenerForBindings(const Identifier& eventType, RefPtr<EventListener>&&);
     WEBCORE_EXPORT void removeAllListenersForBindings(const Identifier& eventType);
-    WEBCORE_EXPORT bool emitForBindings(const Identifier&, const MarkedArgumentBuffer&);
+    WEBCORE_EXPORT bool emitForBindings(JSC::JSGlobalObject* lexicalGlobalObject, const Identifier&, const MarkedArgumentBuffer&);
 
     WEBCORE_EXPORT bool addListener(const Identifier& eventType, Ref<EventListener>&&, bool, bool);
     WEBCORE_EXPORT bool removeListener(const Identifier& eventType, EventListener&);
     WEBCORE_EXPORT bool removeAllListeners(const Identifier& eventType);
 
-    WEBCORE_EXPORT void emit(const Identifier&, const MarkedArgumentBuffer&);
+    WEBCORE_EXPORT std::optional<JSC::Exception> emit(const Identifier&, const MarkedArgumentBuffer&);
     WEBCORE_EXPORT void uncaughtExceptionInEventHandler();
 
     WEBCORE_EXPORT Vector<Identifier> getEventNames();
@@ -74,7 +73,7 @@ public:
     Vector<Identifier> eventTypes();
     const SimpleEventListenerVector& eventListeners(const Identifier& eventType);
 
-    void fireEventListeners(const Identifier& eventName, const MarkedArgumentBuffer& arguments);
+    std::optional<JSC::Exception> fireEventListeners(const Identifier& eventName, const MarkedArgumentBuffer& arguments);
     bool isFiringEventListeners() const;
 
     void invalidateJSEventListeners(JSC::JSObject*);
@@ -103,7 +102,7 @@ private:
     EventEmitterData& ensureEventEmitterData() { return m_eventTargetData; }
     void eventListenersDidChange() {}
 
-    void innerInvokeEventListeners(const Identifier&, SimpleEventListenerVector, const MarkedArgumentBuffer& arguments);
+    std::optional<JSC::Exception> innerInvokeEventListeners(const Identifier&, SimpleEventListenerVector, const MarkedArgumentBuffer& arguments);
     void invalidateEventListenerRegions();
 
     EventEmitterData m_eventTargetData;
diff --git a/src/bun.js/bindings/webcore/JSEventEmitter.cpp b/src/bun.js/bindings/webcore/JSEventEmitter.cpp
index 003313b95..fe62b0352 100644
--- a/src/bun.js/bindings/webcore/JSEventEmitter.cpp
+++ b/src/bun.js/bindings/webcore/JSEventEmitter.cpp
@@ -243,8 +243,14 @@ static inline JSC::EncodedJSValue addListener(JSC::JSGlobalObject* lexicalGlobal
     args.append(argument0.value());
     args.append(argument1.value());
 
-    auto result2 = JSValue::encode(toJS<IDLBoolean>(*lexicalGlobalObject, throwScope, [&]() -> decltype(auto) { return impl.emitForBindings(WTFMove(newListenerEventType), WTFMove(args)); }));
-    RETURN_IF_EXCEPTION(throwScope, encodedJSValue());
+    auto catchScope = DECLARE_CATCH_SCOPE(vm);
+    auto result = impl.emitForBindings(lexicalGlobalObject, WTFMove(newListenerEventType), WTFMove(args));
+    auto* exception = catchScope.exception();
+    if (UNLIKELY(exception)) {
+        catchScope.clearException();
+        JSC::throwVMError(lexicalGlobalObject, throwScope, exception);
+        return JSValue::encode(JSValue {});
+    }
 
     vm.writeBarrier(&static_cast<JSObject&>(*castedThis), argument1.value());
     impl.setThisObject(actualThis);
@@ -399,7 +405,15 @@ static inline JSC::EncodedJSValue jsEventEmitterPrototypeFunction_emitBody(JSC::
     for (size_t i = 1; i < argumentCount; ++i) {
         args.append(callFrame->uncheckedArgument(i));
     }
-    RELEASE_AND_RETURN(throwScope, JSValue::encode(toJS<IDLBoolean>(*lexicalGlobalObject, throwScope, impl.emitForBindings(eventType, args))));
+    auto catchScope = DECLARE_CATCH_SCOPE(vm);
+    bool result = impl.emitForBindings(lexicalGlobalObject, WTFMove(eventType), WTFMove(args));
+    auto* exception = catchScope.exception();
+    if (exception) {
+        catchScope.clearException();
+        throwVMError(lexicalGlobalObject, throwScope, exception);
+        return encodedJSValue();
+    }
+    RELEASE_AND_RETURN(throwScope, JSValue::encode(toJS<IDLBoolean>(*lexicalGlobalObject, throwScope, result)));
 }
 
 JSC_DEFINE_HOST_FUNCTION(jsEventEmitterPrototypeFunction_emit, (JSGlobalObject * lexicalGlobalObject, CallFrame* callFrame))
diff --git a/test/js/node/events/event-emitter.test.ts b/test/js/node/events/event-emitter.test.ts
index 401ccf605..9f049e11e 100644
--- a/test/js/node/events/event-emitter.test.ts
+++ b/test/js/node/events/event-emitter.test.ts
@@ -162,3 +162,8 @@ test("EventEmitter GCs", async () => {
 
   await expectMaxObjectTypeCount(expect, "EventEmitter", startCount);
 });
+
+test("EventEmitter error without listener", () => {
+  var myEmitter = new EventEmitter();
+  expect(() => myEmitter.emit("error", new Error("whoops"))).toThrow("whoops");
+});