From 68081c4f6a6c0429a75890d5764a93b7e3d68164 Mon Sep 17 00:00:00 2001
From: Shigma <shigma10826@gmail.com>
Date: Tue, 14 Nov 2023 02:34:48 +0800
Subject: [PATCH] fix: catch dispose error, fix koishijs/koishi#1254

---
 src/scope.ts          |  4 +++-
 tests/dispose.spec.ts | 30 +++++++++++++++++++++++++-----
 2 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/src/scope.ts b/src/scope.ts
index 6c35aaf..65ad903 100644
--- a/src/scope.ts
+++ b/src/scope.ts
@@ -134,7 +134,9 @@ export abstract class EffectScope<C extends Context = Context> {
     this.isActive = false
     this.disposables = this.disposables.splice(0).filter((dispose) => {
       if (this.uid !== null && dispose[Context.static] === this) return true
-      dispose()
+      ;(async () => dispose())().catch((reason) => {
+        this.context.emit('internal/warning', reason)
+      })
     })
   }
 
diff --git a/tests/dispose.spec.ts b/tests/dispose.spec.ts
index c7bc1b4..a46eb24 100644
--- a/tests/dispose.spec.ts
+++ b/tests/dispose.spec.ts
@@ -61,18 +61,38 @@ describe('Disposables', () => {
 
   it('dispose event', () => {
     const root = new Context()
-    const callback = jest.fn(noop)
+    const dispose = jest.fn(noop)
     const plugin = (ctx: Context) => {
-      ctx.on('dispose', callback)
+      ctx.on('dispose', dispose)
     }
 
     root.plugin(plugin)
-    expect(callback.mock.calls).to.have.length(0)
+    expect(dispose.mock.calls).to.have.length(0)
     expect(root.dispose(plugin)).to.be.ok
-    expect(callback.mock.calls).to.have.length(1)
+    expect(dispose.mock.calls).to.have.length(1)
     // callback should only be called once
     expect(root.dispose(plugin)).to.be.not.ok
-    expect(callback.mock.calls).to.have.length(1)
+    expect(dispose.mock.calls).to.have.length(1)
+  })
+
+  it('dispose event', async () => {
+    const root = new Context()
+    const warn = jest.fn()
+    const dispose = jest.fn(() => {
+      throw new Error('test')
+    })
+    root.on('internal/warning', warn)
+    const plugin = (ctx: Context) => {
+      ctx.on('dispose', dispose)
+    }
+
+    root.plugin(plugin)
+    expect(dispose.mock.calls).to.have.length(0)
+    expect(root.dispose(plugin)).to.be.ok
+    // warning is asynchronous
+    await new Promise((resolve) => setTimeout(resolve, 0))
+    expect(dispose.mock.calls).to.have.length(1)
+    expect(warn.mock.calls).to.have.length(1)
   })
 
   it('root dispose', async () => {