From 55a7ca82f7595cfe126e41e16f4e76ee54f7e1d1 Mon Sep 17 00:00:00 2001 From: Him188 Date: Tue, 22 Dec 2020 13:15:02 +0800 Subject: [PATCH] Wrap exceptions thrown in EventHandler with relevant event so as to allow obtaining event instance in SimpleListenerHost.handleException. Fix #533 --- .../kotlin/event/JvmMethodListeners.kt | 93 +++++++++++++------ .../kotlin/event/JvmMethodEventsTest.kt | 47 ++++++++-- 2 files changed, 101 insertions(+), 39 deletions(-) diff --git a/mirai-core-api/src/commonMain/kotlin/event/JvmMethodListeners.kt b/mirai-core-api/src/commonMain/kotlin/event/JvmMethodListeners.kt index 4c38c0dc9..7e57ed596 100644 --- a/mirai-core-api/src/commonMain/kotlin/event/JvmMethodListeners.kt +++ b/mirai-core-api/src/commonMain/kotlin/event/JvmMethodListeners.kt @@ -15,6 +15,7 @@ package net.mamoe.mirai.event import kotlinx.coroutines.* import net.mamoe.mirai.utils.EventListenerLikeJava +import net.mamoe.mirai.utils.castOrNull import java.lang.reflect.Method import kotlin.coroutines.CoroutineContext import kotlin.coroutines.EmptyCoroutineContext @@ -189,6 +190,8 @@ public abstract class SimpleListenerHost /** * 处理事件处理中未捕获的异常. 在构造器中的 [coroutineContext] 未提供 [CoroutineExceptionHandler] 情况下必须继承此函数. + * + * [exception] 通常是 [ExceptionInEventHandlerException]. 可以获取事件: [ExceptionInEventHandlerException.event] */ public open fun handleException(context: CoroutineContext, exception: Throwable) { throw IllegalStateException( @@ -207,8 +210,39 @@ public abstract class SimpleListenerHost public fun cancelAll() { this.cancel() } + + protected companion object { + /** + * 获取 [ExceptionInEventHandlerException.event] + */ + @JvmStatic + protected val Throwable.event: Event? + get() = this.castOrNull()?.event + + /** + * 递归获取 [Throwable.cause], 无 `cause` 时返回 `this` + */ + @JvmStatic + protected val Throwable.rootCause: Throwable + get() = generateSequence(this) { it.cause }.last() + } } +/** + * [EventHandler] 标记的函数在处理事件时产生异常时包装异常并重新抛出 + */ +public class ExceptionInEventHandlerException( + /** + * 当时正在处理的事件 + */ + public val event: Event, + override val message: String = "Exception in EventHandler", + /** + * 原异常 + */ + override val cause: Throwable +) : IllegalStateException() + /** * 反射得到所有标注了 [EventHandler] 的函数 (Java 为方法), 并注册为事件监听器 * @@ -248,21 +282,12 @@ private fun Method.isKotlinFunction(): Boolean { return declaringClass.getDeclaredAnnotation(kotlin.Metadata::class.java) != null } -private fun Method.invokeWithErrorReport(self: Any?, vararg args: Any?): Any? = try { - invoke(self, *args) -} catch (exception: IllegalArgumentException) { - throw IllegalArgumentException( - "Internal Error: $exception, method=${this}, this=$self, arguments=$args, please report to https://github.com/mamoe/mirai", - exception - ) -} - @Suppress("UNCHECKED_CAST") private fun Method.registerEvent( owner: Any, scope: CoroutineScope, annotation: EventHandler, - coroutineContext: CoroutineContext + coroutineContext: CoroutineContext, ): Listener { this.isAccessible = true val kotlinFunction = kotlin.runCatching { this.kotlinFunction }.getOrNull() @@ -311,6 +336,8 @@ private fun Method.registerEvent( } catch (e: IllegalCallableAccessException) { listener.completeExceptionally(e) return ListeningStatus.STOPPED + } catch (e: Throwable) { + throw ExceptionInEventHandlerException(event, cause = e) } } require(!kotlinFunction.returnType.isMarkedNullable) { @@ -357,6 +384,30 @@ private fun Method.registerEvent( check(this.parameterCount == 1 && Event::class.java.isAssignableFrom(paramType)) { "Illegal method parameter. Required one exact Event subclass. found ${this.parameters.contentToString()}" } + suspend fun callMethod(event: Event): Any? { + fun Method.invokeWithErrorReport(self: Any?, vararg args: Any?): Any? = try { + invoke(self, *args) + } catch (exception: IllegalArgumentException) { + throw IllegalArgumentException( + "Internal Error: $exception, method=${this}, this=$self, arguments=$args, please report to https://github.com/mamoe/mirai", + exception + ) + } catch (e: Throwable) { + throw ExceptionInEventHandlerException(event, cause = e) + } + + + return if (annotation.ignoreCancelled) { + if (event.castOrNull()?.isCancelled != true) { + withContext(Dispatchers.IO) { + this@registerEvent.invokeWithErrorReport(owner, event) + } + } else ListeningStatus.LISTENING + } else withContext(Dispatchers.IO) { + this@registerEvent.invokeWithErrorReport(owner, event) + } + } + when (this.returnType) { Void::class.java, Void.TYPE, Nothing::class.java -> { scope.subscribeAlways( @@ -365,15 +416,7 @@ private fun Method.registerEvent( concurrency = annotation.concurrency, coroutineContext = coroutineContext ) { - if (annotation.ignoreCancelled) { - if ((this as? CancellableEvent)?.isCancelled != true) { - withContext(Dispatchers.IO) { - this@registerEvent.invokeWithErrorReport(owner, this@subscribeAlways) - } - } - } else withContext(Dispatchers.IO) { - this@registerEvent.invokeWithErrorReport(owner, this@subscribeAlways) - } + callMethod(this) } } ListeningStatus::class.java -> { @@ -383,16 +426,8 @@ private fun Method.registerEvent( concurrency = annotation.concurrency, coroutineContext = coroutineContext ) { - if (annotation.ignoreCancelled) { - if ((this as? CancellableEvent)?.isCancelled != true) { - withContext(Dispatchers.IO) { - this@registerEvent.invokeWithErrorReport(owner, this@subscribe) as ListeningStatus - } - } else ListeningStatus.LISTENING - } else withContext(Dispatchers.IO) { - this@registerEvent.invokeWithErrorReport(owner, this@subscribe) as ListeningStatus - } - + callMethod(this) as ListeningStatus? + ?: error("Java method EventHandler cannot return `null`: $this") } } else -> error("Illegal method return type. Required Void or ListeningStatus, but found ${this.returnType.canonicalName}") diff --git a/mirai-core-api/src/jvmTest/kotlin/event/JvmMethodEventsTest.kt b/mirai-core-api/src/jvmTest/kotlin/event/JvmMethodEventsTest.kt index e14070ffd..01096e051 100644 --- a/mirai-core-api/src/jvmTest/kotlin/event/JvmMethodEventsTest.kt +++ b/mirai-core-api/src/jvmTest/kotlin/event/JvmMethodEventsTest.kt @@ -12,10 +12,10 @@ package net.mamoe.mirai.event import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.runBlocking -import net.mamoe.mirai.event.* import org.jetbrains.annotations.NotNull import org.junit.jupiter.api.Test import java.util.concurrent.atomic.AtomicInteger +import kotlin.coroutines.CoroutineContext import kotlin.coroutines.EmptyCoroutineContext import kotlin.test.assertEquals @@ -100,6 +100,33 @@ internal class JvmMethodEventsTest { } } + @Test + fun testExceptionHandle() { + class MyException : RuntimeException() + + class TestClass : SimpleListenerHost() { + override fun handleException(context: CoroutineContext, exception: Throwable) { + assert(exception is ExceptionInEventHandlerException) + assert(exception.event is TestEvent) + assert(exception.rootCause is MyException) + } + + @Suppress("unused") + @EventHandler + private suspend fun TestEvent.test() { + throw MyException() + } + } + + TestClass().run { + this.registerEvents() + + runBlocking { + TestEvent().broadcast() + } + } + } + @Test fun testIntercept() { class TestClass : ListenerHost, CoroutineScope by CoroutineScope(EmptyCoroutineContext) { @@ -122,14 +149,14 @@ internal class JvmMethodEventsTest { } } -// TestClass().run { -// this.registerEvents() -// -// runBlocking { -// TestEvent().broadcast() -// } -// -// assertEquals(1, this.getCalled()) -// } + TestClass().run { + this.registerEvents() + + runBlocking { + TestEvent().broadcast() + } + + assertEquals(1, this.getCalled()) + } } }