[EventEmitter] Preserve `this` in event emitter callbacks

10 files changed, 130 insertions, 30 deletions

diff --git a/src/bun.js/api/bun.zig b/src/bun.js/api/bun.zig
index ad015e0ba..91d106b65 100644
--- a/src/bun.js/api/bun.zig
+++ b/src/bun.js/api/bun.zig
@@ -3906,8 +3906,8 @@ pub const EnvironmentVariables = struct {
     }
 };
 
-export fn Bun__reportError(_: *JSGlobalObject, err: JSC.JSValue) void {
-    JSC.VirtualMachine.get().runErrorHandler(err, null);
+export fn Bun__reportError(globalObject: *JSGlobalObject, err: JSC.JSValue) void {
+    JSC.VirtualMachine.runErrorHandlerWithDedupe(globalObject.bunVM(), err, null);
 }
 
 comptime {
diff --git a/src/bun.js/bindings/ZigGlobalObject.cpp b/src/bun.js/bindings/ZigGlobalObject.cpp
index 11b67b433..58497cad4 100644
--- a/src/bun.js/bindings/ZigGlobalObject.cpp
+++ b/src/bun.js/bindings/ZigGlobalObject.cpp
@@ -496,7 +496,7 @@ WebCore::ScriptExecutionContext* GlobalObject::scriptExecutionContext() const
 void GlobalObject::reportUncaughtExceptionAtEventLoop(JSGlobalObject* globalObject,
     JSC::Exception* exception)
 {
-    Bun__reportError(globalObject, JSValue::encode(JSValue(exception)));
+    Bun__reportUnhandledError(globalObject, JSValue::encode(JSValue(exception)));
 }
 
 void GlobalObject::promiseRejectionTracker(JSGlobalObject* obj, JSC::JSPromise* promise,
@@ -2119,7 +2119,6 @@ public:
 
 const ClassInfo BunPrimordialsObject::s_info = { "Primordials"_s, &Base::s_info, nullptr, nullptr, CREATE_METHOD_TABLE(BunPrimordialsObject) };
 
-extern "C" void Bun__reportUnhandledError(JSGlobalObject*, EncodedJSValue);
 JSC_DEFINE_HOST_FUNCTION(jsFunctionPerformMicrotask, (JSGlobalObject * globalObject, CallFrame* callframe))
 {
     auto& vm = globalObject->vm();
@@ -2141,22 +2140,26 @@ JSC_DEFINE_HOST_FUNCTION(jsFunctionPerformMicrotask, (JSGlobalObject * globalObj
     WTF::NakedPtr<JSC::Exception> exceptionPtr;
 
     size_t argCount = callframe->argumentCount();
-    if (argCount > 1) {
-        if (JSValue arg0 = callframe->argument(1)) {
-            arguments.append(arg0);
-        }
-
-        if (argCount > 2) {
-            if (JSValue arg1 = callframe->argument(2)) {
-                arguments.append(arg1);
-            }
-
-            if (argCount > 3) {
-                if (JSValue arg2 = callframe->argument(3)) {
-                    arguments.append(arg2);
-                }
-            }
-        }
+    switch (argCount) {
+    case 1: {
+        break;
+    }
+    case 2: {
+        arguments.append(callframe->uncheckedArgument(1));
+        break;
+    }
+    case 3: {
+        arguments.append(callframe->uncheckedArgument(1));
+        arguments.append(callframe->uncheckedArgument(2));
+        break;
+    }
+    case 4: {
+        arguments.append(callframe->uncheckedArgument(1));
+        arguments.append(callframe->uncheckedArgument(2));
+        arguments.append(callframe->uncheckedArgument(3));
+    }
+    default:
+        break;
     }
 
     JSC::call(globalObject, job, callData, jsUndefined(), arguments, exceptionPtr);
@@ -3323,10 +3326,10 @@ void GlobalObject::installAPIGlobals(JSClassRef* globals, int count, JSC::VM& vm
             dnsObject->putDirectNativeFunction(vm, this, JSC::Identifier::fromString(vm, "lookup"_s), 2, Bun__DNSResolver__lookup, ImplementationVisibility::Public, NoIntrinsic,
                 JSC::PropertyAttribute::Function | JSC::PropertyAttribute::DontDelete | 0);
             dnsObject->putDirectNativeFunction(vm, this, JSC::Identifier::fromString(vm, "resolveSrv"_s), 2, Bun__DNSResolver__resolveSrv, ImplementationVisibility::Public, NoIntrinsic,
-                JSC::PropertyAttribute::Function | JSC::PropertyAttribute::DontDelete | 0); 
+                JSC::PropertyAttribute::Function | JSC::PropertyAttribute::DontDelete | 0);
             object->putDirect(vm, PropertyName(identifier), dnsObject, JSC::PropertyAttribute::ReadOnly | JSC::PropertyAttribute::DontDelete | 0);
         }
-        
+
         {
             JSC::Identifier identifier = JSC::Identifier::fromString(vm, "plugin"_s);
             JSFunction* pluginFunction = JSFunction::create(vm, this, 1, String("plugin"_s), jsFunctionBunPlugin, ImplementationVisibility::Public, NoIntrinsic);
diff --git a/src/bun.js/bindings/ZigGlobalObject.h b/src/bun.js/bindings/ZigGlobalObject.h
index 3b8f4c342..98377e160 100644
--- a/src/bun.js/bindings/ZigGlobalObject.h
+++ b/src/bun.js/bindings/ZigGlobalObject.h
@@ -44,6 +44,7 @@ class SubtleCrypto;
 }
 
 extern "C" void Bun__reportError(JSC__JSGlobalObject*, JSC__JSValue);
+extern "C" void Bun__reportUnhandledError(JSGlobalObject*, EncodedJSValue);
 // defined in ModuleLoader.cpp
 extern "C" JSC::EncodedJSValue jsFunctionOnLoadObjectResultResolve(JSC::JSGlobalObject* globalObject, JSC::CallFrame* callFrame);
 extern "C" JSC::EncodedJSValue jsFunctionOnLoadObjectResultReject(JSC::JSGlobalObject* globalObject, JSC::CallFrame* callFrame);
diff --git a/src/bun.js/bindings/webcore/EventEmitter.cpp b/src/bun.js/bindings/webcore/EventEmitter.cpp
index 640e7d158..02770be68 100644
--- a/src/bun.js/bindings/webcore/EventEmitter.cpp
+++ b/src/bun.js/bindings/webcore/EventEmitter.cpp
@@ -81,6 +81,7 @@ bool EventEmitter::removeAllListeners()
     auto& map = data->eventListenerMap;
     bool any = !map.isEmpty();
     map.clear();
+    this->m_thisObject.clear();
     return any;
 }
 
@@ -196,6 +197,9 @@ void EventEmitter::innerInvokeEventListeners(const Identifier& eventType, Simple
     auto& context = *scriptExecutionContext();
     VM& vm = context.vm();
 
+    auto* thisObject = protectedThis->m_thisObject.get();
+    JSC::JSValue thisValue = thisObject ? JSC::JSValue(thisObject) : JSC::jsUndefined();
+
     for (auto& registeredListener : listeners) {
         if (UNLIKELY(registeredListener->wasRemoved()))
             continue;
@@ -217,13 +221,13 @@ void EventEmitter::innerInvokeEventListeners(const Identifier& eventType, Simple
                 continue;
 
             WTF::NakedPtr<JSC::Exception> exceptionPtr;
-            JSC::call(jsFunction->globalObject(), jsFunction, callData, JSC::jsUndefined(), arguments, exceptionPtr);
+            JSC::call(jsFunction->globalObject(), jsFunction, callData, thisValue, arguments, exceptionPtr);
             if (auto* exception = exceptionPtr.get()) {
                 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__reportError(lexicalGlobalObject, JSValue::encode(JSValue(exception)));
+                    Bun__reportUnhandledError(lexicalGlobalObject, JSValue::encode(JSValue(exception)));
                 } else if (hasErrorListener) {
                     MarkedArgumentBuffer expcep;
                     JSValue errorValue = exception->value();
diff --git a/src/bun.js/bindings/webcore/EventEmitter.h b/src/bun.js/bindings/webcore/EventEmitter.h
index 21a7dd579..b46bcff5d 100644
--- a/src/bun.js/bindings/webcore/EventEmitter.h
+++ b/src/bun.js/bindings/webcore/EventEmitter.h
@@ -83,6 +83,15 @@ public:
 
     IdentifierEventListenerMap& eventListenerMap() { return ensureEventEmitterData().eventListenerMap; }
 
+    void setThisObject(JSC::JSValue thisObject)
+    {
+        m_thisObject.clear();
+
+        if (thisObject.isCell()) {
+            m_thisObject = JSC::Weak<JSC::JSObject>(thisObject.getObject());
+        }
+    }
+
 private:
     EventEmitter(ScriptExecutionContext& context)
         : ContextDestructionObserver(&context)
@@ -99,6 +108,8 @@ private:
 
     EventEmitterData m_eventTargetData;
     unsigned m_maxListeners { 10 };
+
+    mutable JSC::Weak<JSC::JSObject> m_thisObject { nullptr };
 };
 
 inline const EventEmitterData* EventEmitter::eventTargetData() const
diff --git a/src/bun.js/bindings/webcore/JSEventEmitter.cpp b/src/bun.js/bindings/webcore/JSEventEmitter.cpp
index 28a18c31c..8a4c3fe6b 100644
--- a/src/bun.js/bindings/webcore/JSEventEmitter.cpp
+++ b/src/bun.js/bindings/webcore/JSEventEmitter.cpp
@@ -247,6 +247,7 @@ static inline JSC::EncodedJSValue addListener(JSC::JSGlobalObject* lexicalGlobal
     RETURN_IF_EXCEPTION(throwScope, encodedJSValue());
 
     vm.writeBarrier(&static_cast<JSObject&>(*castedThis), argument1.value());
+    impl.setThisObject(actualThis);
     RELEASE_AND_RETURN(throwScope, JSValue::encode(actualThis));
 }
 
@@ -348,6 +349,7 @@ static inline JSC::EncodedJSValue jsEventEmitterPrototypeFunction_removeListener
     auto result = JSValue::encode(toJS<IDLUndefined>(*lexicalGlobalObject, throwScope, [&]() -> decltype(auto) { return impl.removeListenerForBindings(WTFMove(eventType), WTFMove(listener)); }));
     RETURN_IF_EXCEPTION(throwScope, encodedJSValue());
     vm.writeBarrier(&static_cast<JSObject&>(*castedThis), argument1.value());
+    impl.setThisObject(actualThis);
     RELEASE_AND_RETURN(throwScope, JSValue::encode(actualThis));
 }
 
diff --git a/src/bun.js/bindings/webcore/JSEventEmitterCustom.cpp b/src/bun.js/bindings/webcore/JSEventEmitterCustom.cpp
index 15284fe6c..e6e15b874 100644
--- a/src/bun.js/bindings/webcore/JSEventEmitterCustom.cpp
+++ b/src/bun.js/bindings/webcore/JSEventEmitterCustom.cpp
@@ -62,6 +62,7 @@ JSEventEmitter* jsEventEmitterCastFast(VM& vm, JSC::JSGlobalObject* lexicalGloba
     auto scope = DECLARE_CATCH_SCOPE(vm);
     auto* globalObject = reinterpret_cast<Zig::GlobalObject*>(lexicalGlobalObject);
     auto impl = EventEmitter::create(*globalObject->scriptExecutionContext());
+    impl->setThisObject(thisObject);
 
     auto throwScope = DECLARE_THROW_SCOPE(vm);
     auto result = toJSNewlyCreated<IDLInterface<EventEmitter>>(*lexicalGlobalObject, *globalObject, throwScope, WTFMove(impl));
diff --git a/src/bun.js/test/jest.zig b/src/bun.js/test/jest.zig
index f43c97a82..1ae0bd51a 100644
--- a/src/bun.js/test/jest.zig
+++ b/src/bun.js/test/jest.zig
@@ -80,6 +80,10 @@ pub const TestRunner = struct {
 
     has_pending_tests: bool = false,
     pending_test: ?*TestRunnerTask = null,
+
+    /// This silences TestNotRunningError when expect() is used to halt a running test.
+    did_pending_test_fail: bool = false,
+
     pub const Drainer = JSC.AnyTask.New(TestRunner, drain);
 
     pub fn enqueue(this: *TestRunner, task: *TestRunnerTask) void {
@@ -100,6 +104,7 @@ pub const TestRunner = struct {
         if (this.queue.readItem()) |task| {
             this.pending_test = task;
             this.has_pending_tests = true;
+            this.did_pending_test_fail = false;
             if (!task.run()) {
                 this.has_pending_tests = false;
                 this.pending_test = null;
@@ -273,7 +278,9 @@ pub const Expect = struct {
         const value = arguments[0];
 
         if (Jest.runner.?.pending_test == null) {
-            globalObject.throw("expect() must be called inside a test", .{});
+            const err = globalObject.createErrorInstance("expect() must be called in a test", .{});
+            err.put(globalObject, ZigString.static("name"), ZigString.init("TestNotRunningError").toValueGC(globalObject));
+            globalObject.throwValue(err);
             return .zero;
         }
 
@@ -1454,6 +1461,7 @@ pub const TestScope = struct {
         if (comptime is_bindgen) return undefined;
         var vm = VirtualMachine.get();
         var callback = this.callback;
+        Jest.runner.?.did_pending_test_fail = false;
         defer {
             js.JSValueUnprotect(vm.global, callback);
             this.callback = null;
@@ -1480,7 +1488,11 @@ pub const TestScope = struct {
         }
 
         if (initial_value.isAnyError()) {
-            vm.runErrorHandler(initial_value, null);
+            if (!Jest.runner.?.did_pending_test_fail) {
+                Jest.runner.?.did_pending_test_fail = true;
+                vm.runErrorHandler(initial_value, null);
+            }
+
             return .{ .fail = active_test_expectation_counter.actual };
         }
 
@@ -1498,7 +1510,11 @@ pub const TestScope = struct {
             }
             switch (promise.status(vm.global.vm())) {
                 .Rejected => {
-                    vm.runErrorHandler(promise.result(vm.global.vm()), null);
+                    if (!Jest.runner.?.did_pending_test_fail) {
+                        Jest.runner.?.did_pending_test_fail = true;
+                        vm.runErrorHandler(promise.result(vm.global.vm()), null);
+                    }
+
                     return .{ .fail = active_test_expectation_counter.actual };
                 },
                 .Pending => {
@@ -1692,6 +1708,10 @@ pub const DescribeScope = struct {
             const pending_test = Jest.runner.?.pending_test;
             // forbid `expect()` within hooks
             Jest.runner.?.pending_test = null;
+            const orig_did_pending_test_fail = Jest.runner.?.did_pending_test_fail;
+
+            Jest.runner.?.did_pending_test_fail = false;
+
             const vm = VirtualMachine.get();
             var result: JSC.JSValue = if (cb.getLengthOfArray(ctx) > 0) brk: {
                 this.done = false;
@@ -1718,6 +1738,7 @@ pub const DescribeScope = struct {
             }
 
             Jest.runner.?.pending_test = pending_test;
+            Jest.runner.?.did_pending_test_fail = orig_did_pending_test_fail;
             if (result.isAnyError()) return result;
 
             if (comptime hook == .beforeAll or hook == .afterAll) {
@@ -1971,7 +1992,19 @@ pub const TestRunnerTask = struct {
         fulfilled,
     };
 
-    pub fn onUnhandledRejection(jsc_vm: *VirtualMachine, _: *JSC.JSGlobalObject, rejection: JSC.JSValue) void {
+    pub fn onUnhandledRejection(jsc_vm: *VirtualMachine, global: *JSC.JSGlobalObject, rejection: JSC.JSValue) void {
+        if (Jest.runner) |runner| {
+            if (runner.did_pending_test_fail and rejection.isException(global.vm())) {
+                if (rejection.toError()) |err| {
+                    if (err.get(global, "name")) |name| {
+                        if (name.isString() and name.getZigString(global).eqlComptime("TestNotRunningError")) {
+                            return;
+                        }
+                    }
+                }
+            }
+        }
+
         if (jsc_vm.last_reported_error_for_dedupe == rejection and rejection != .zero) {
             jsc_vm.last_reported_error_for_dedupe = .zero;
         } else {
@@ -2042,6 +2075,9 @@ pub const TestRunnerTask = struct {
     }
 
     pub fn handleResult(this: *TestRunnerTask, result: Result, comptime from: @Type(.EnumLiteral)) void {
+        if (result == .fail)
+            Jest.runner.?.did_pending_test_fail = true;
+
         switch (comptime from) {
             .promise => {
                 std.debug.assert(this.promise_state == .pending);
diff --git a/test/bun.js/event-emitter.test.ts b/test/bun.js/event-emitter.test.ts
index c9bef0ff4..1752af1f5 100644
--- a/test/bun.js/event-emitter.test.ts
+++ b/test/bun.js/event-emitter.test.ts
@@ -123,9 +123,9 @@ for (let create of waysOfCreating) {
     .trim()} should work`, () => {
     var myEmitter = create();
     var called = false;
-
-    myEmitter.once("event", () => {
+    myEmitter.once("event", function () {
       called = true;
+      expect(this as any).toBe(myEmitter);
     });
     var firstEvents = myEmitter._events;
     expect(myEmitter.listenerCount("event")).toBe(1);
diff --git a/test/bun.js/test-test.test.ts b/test/bun.js/test-test.test.ts
index 78a701f45..7f1918a94 100644
--- a/test/bun.js/test-test.test.ts
+++ b/test/bun.js/test-test.test.ts
@@ -1,4 +1,8 @@
+import { spawnSync } from "bun";
 import { describe, expect, it, test } from "bun:test";
+import { bunEnv } from "bunEnv";
+import { bunExe } from "bunExe";
+import { mkdirSync, realpathSync, rmSync, writeFileSync } from "fs";
 
 test("toStrictEqual() vs toEqual()", () => {
   expect([1, , 3]).toEqual([1, , 3]);
@@ -2009,3 +2013,41 @@ describe("throw in describe scope doesn't enqueue tests after thrown", () => {
 it("a describe scope throwing doesn't cause all other tests in the file to fail", () => {
   expect(true).toBe(true);
 });
+
+test("test throwing inside an EventEmitter fails the test", () => {
+  const code = `
+  import {test, expect} from 'bun:test';
+  import {EventEmitter} from 'events';
+  test('test throwing inside an EventEmitter fails the test', () => {
+    const emitter = new EventEmitter();
+    emitter.on('event', () => {
+      throw new Error('THIS TEST HAS PASSED');
+    });
+    emitter.emit('event');
+  });
+  
+  `;
+
+  rmSync("/tmp/test-throwing-bun/test-throwing-eventemitter.test.js", {
+    force: true,
+  });
+
+  try {
+    mkdirSync("/tmp/test-throwing-bun", { recursive: true });
+  } catch (e) {}
+  writeFileSync(
+    "/tmp/test-throwing-bun/test-throwing-eventemitter.test.js",
+    code,
+  );
+
+  const { stdout, stderr, exitCode } = spawnSync(
+    [bunExe(), "wiptest", "test-throwing-eventemitter"],
+    {
+      cwd: realpathSync("/tmp/test-throwing-bun"),
+      env: bunEnv,
+    },
+  );
+
+  expect(exitCode).toBe(1);
+  expect(stderr!.toString()).toContain("THIS TEST HAS PASSED");
+});