mark plugin disabled when it throw exception during enable process (#2022)

* Fix bootstrap when no env specified

* add PluginWithExceptionTest

* Add exceptedExceptionMessage in MCIT(Throwable or Exception may not work?)

* Using red color show disabled plugins in /status command

* Catch exception during enable process

* Count plugins without disabled plugins

* Revert "Fix bootstrap when no env specified"

This reverts commit ff9bb180fd6687c5da27b8a38da2658a1b26ad93.

* format

* format

* halt plugin enable process when mirai failed to enable it dependencies.

* Update mirai-console/backend/mirai-console/src/internal/plugin/PluginManagerImpl.kt hint

Co-authored-by: Him188 <Him188@mamoe.net>

* rename tmp to dependsOn

* Revert changes of PluginManagerImpl

* Plugin callback executions assertions

* Improve & Fix Logic

* Fix PluginDependOnErrorPlugin

* Update err msg

* move dependencies check from JvmPluginInternal.kt to BuiltInJvmPluginLoaderImpl.kt

* typo

* don't throw err caused by dependencies fail

Co-authored-by: Him188 <Him188@mamoe.net>
Co-authored-by: Karlatemp <kar@kasukusakura.com>

mirai-console/backend/integration-test/src/AbstractTestPointAsPlugin.kt

@@ -1,5 +1,5 @@
 /*
- * Copyright 2019-2021 Mamoe Technologies and contributors.
+ * Copyright 2019-2022 Mamoe Technologies and contributors.
  *
  * 此源代码的使用受 GNU AFFERO GENERAL PUBLIC LICENSE version 3 许可证的约束, 可以在以下链接找到该许可证.
  * Use of this source code is governed by the GNU AGPLv3 license that can be found through the following link.
@@ -29,6 +29,22 @@ public abstract class AbstractTestPointAsPlugin : AbstractTestPoint() {
     protected open fun KotlinPlugin.onEnable0() {}
     protected open fun KotlinPlugin.onDisable0() {}
 
+    protected open fun exceptionHandler(exception: Throwable, step: JvmPluginExecutionStep, instance: KotlinPlugin) {
+        IntegrationTestBootstrapContext.failures.add(this.javaClass)
+    }
+
+    private fun callEH(exception: Throwable, step: JvmPluginExecutionStep, instance: KotlinPlugin) {
+        try {
+            exceptionHandler(exception, step, instance)
+        } catch (e: Throwable) {
+            forceFail(cause = e)
+        }
+    }
+
+    protected enum class JvmPluginExecutionStep {
+        OnEnable, OnDisable, OnLoad
+    }
+
 
     @Suppress("unused")
     @PublishedApi
@@ -51,7 +67,7 @@ public abstract class AbstractTestPointAsPlugin : AbstractTestPoint() {
             try {
                 impl.apply { onDisable0() }
             } catch (e: Throwable) {
-                IntegrationTestBootstrapContext.failures.add(impl.javaClass)
+                impl.callEH(e, JvmPluginExecutionStep.OnDisable, this)
                 throw e
             }
         }
@@ -60,7 +76,7 @@ public abstract class AbstractTestPointAsPlugin : AbstractTestPoint() {
             try {
                 impl.apply { onEnable0() }
             } catch (e: Throwable) {
-                IntegrationTestBootstrapContext.failures.add(impl.javaClass)
+                impl.callEH(e, JvmPluginExecutionStep.OnEnable, this)
                 throw e
             }
         }
@@ -69,7 +85,7 @@ public abstract class AbstractTestPointAsPlugin : AbstractTestPoint() {
             try {
                 impl.apply { onLoad0(this@onLoad) }
             } catch (e: Throwable) {
-                IntegrationTestBootstrapContext.failures.add(impl.javaClass)
+                impl.callEH(e, JvmPluginExecutionStep.OnLoad, this@TestPointPluginImpl)
                 throw e
             }
         }

mirai-console/backend/integration-test/src/IntegrationTestBootstrap.kt

@@ -22,9 +22,7 @@ import net.mamoe.mirai.console.terminal.MiraiConsoleTerminalLoader
 import net.mamoe.mirai.utils.cast
 import net.mamoe.mirai.utils.sha1
 import net.mamoe.mirai.utils.toUHexString
-import org.objectweb.asm.ClassWriter
-import org.objectweb.asm.Opcodes
-import org.objectweb.asm.Type
+import org.objectweb.asm.*
 import java.io.File
 import java.io.FileDescriptor
 import java.io.FileOutputStream
@@ -215,6 +213,28 @@ private fun AbstractTestPointAsPlugin.generatePluginJar() {
             superName,
             null
         )
+
+        // region Copy class annotations
+        this.javaClass.getResourceAsStream(javaClass.simpleName + ".class")!!.use {
+            ClassReader(it)
+        }.accept(object : ClassVisitor(Opcodes.ASM9) {
+            override fun visitAnnotation(descriptor: String, visible: Boolean): AnnotationVisitor? {
+                if ("kotlin/Metadata" in descriptor) return null
+                return classWriter.visitAnnotation(descriptor, visible)
+            }
+
+            override fun visitTypeAnnotation(
+                typeRef: Int,
+                typePath: TypePath,
+                descriptor: String,
+                visible: Boolean
+            ): AnnotationVisitor? {
+                if ("kotlin/Metadata" in descriptor) return null
+                return classWriter.visitTypeAnnotation(typeRef, typePath, descriptor, visible)
+            }
+        }, ClassReader.SKIP_CODE)
+        // endregion
+
         classWriter.visitMethod(
             Opcodes.ACC_PUBLIC,
             "<init>", "()V", null, null

mirai-console/backend/integration-test/src/utils.kt

@@ -9,6 +9,7 @@
 
 package net.mamoe.console.integrationtest
 
+import net.mamoe.mirai.console.internal.plugin.ConsoleJvmPluginTestFailedError
 import org.junit.jupiter.api.fail
 import org.objectweb.asm.ClassReader
 import org.objectweb.asm.ClassWriter
@@ -52,6 +53,13 @@ public fun assertClassSame(expected: Class<*>?, actually: Class<*>?) {
                 "Class actually: ${vt(actually)}"
     }
 }
+
+public fun forceFail(
+    msg: String? = null,
+    cause: Throwable? = null,
+): Nothing {
+    throw ConsoleJvmPluginTestFailedError(msg, cause)
+}
 // endregion
 
 // region JVM Utils

mirai-console/backend/integration-test/test/testpoints/plugin/PluginDependOnErrorPlugin.kt

@@ -0,0 +1,66 @@
+/*
+ * Copyright 2019-2021 Mamoe Technologies and contributors.
+ *
+ * 此源代码的使用受 GNU AFFERO GENERAL PUBLIC LICENSE version 3 许可证的约束, 可以在以下链接找到该许可证.
+ * Use of this source code is governed by the GNU AGPLv3 license that can be found through the following link.
+ *
+ * https://github.com/mamoe/mirai/blob/dev/LICENSE
+ */
+
+package net.mamoe.console.integrationtest.testpoints.plugin
+
+import net.mamoe.console.integrationtest.AbstractTestPointAsPlugin
+import net.mamoe.mirai.console.extension.PluginComponentStorage
+import net.mamoe.mirai.console.internal.plugin.ConsoleJvmPluginFuncCallbackStatus
+import net.mamoe.mirai.console.internal.plugin.ConsoleJvmPluginFuncCallbackStatusExcept
+import net.mamoe.mirai.console.plugin.PluginManager
+import net.mamoe.mirai.console.plugin.id
+import net.mamoe.mirai.console.plugin.jvm.JvmPluginDescription
+import net.mamoe.mirai.console.plugin.jvm.KotlinPlugin
+import org.junit.jupiter.api.Assertions.assertFalse
+import kotlin.test.fail
+
+@ConsoleJvmPluginFuncCallbackStatusExcept.OnEnable(ConsoleJvmPluginFuncCallbackStatus.FAILED)
+internal object PluginDependOnErrorPlugin : AbstractTestPointAsPlugin() {
+    private var isOnEnabledExecuted: Boolean = false
+
+    override fun newPluginDescription(): JvmPluginDescription {
+        return JvmPluginDescription(
+            id = "net.mamoe.testpoint.plugin-depend-on-error-plugin",
+            version = "1.0.0",
+            name = "PluginDependOnErrorPlugin",
+        ) {
+            dependsOn("net.mamoe.testpoint.plugin-with-exception-test")
+        }
+    }
+
+    override fun beforeConsoleStartup() {
+        isOnEnabledExecuted = false
+    }
+
+    override fun KotlinPlugin.onLoad0(storage: PluginComponentStorage) {
+
+    }
+
+    override fun KotlinPlugin.onEnable0() {
+        // unreachable
+        isOnEnabledExecuted = true
+        fail("net.mamoe.testpoint.plugin-depend-on-error-plugin enabled")
+    }
+
+    override fun onConsoleStartSuccessfully() {
+        assertFalse { isOnEnabledExecuted }
+        assertFalse {
+            PluginManager
+                .plugins
+                .first { it.id == "net.mamoe.testpoint.plugin-with-exception-test" }
+                .isEnabled
+        }
+        assertFalse {
+            PluginManager
+                .plugins
+                .first { it.id == "net.mamoe.testpoint.plugin-depend-on-error-plugin" }
+                .isEnabled
+        }
+    }
+}

mirai-console/backend/integration-test/test/testpoints/plugin/PluginWithExceptionTest.kt

@@ -0,0 +1,59 @@
+/*
+ * Copyright 2019-2021 Mamoe Technologies and contributors.
+ *
+ * 此源代码的使用受 GNU AFFERO GENERAL PUBLIC LICENSE version 3 许可证的约束, 可以在以下链接找到该许可证.
+ * Use of this source code is governed by the GNU AGPLv3 license that can be found through the following link.
+ *
+ * https://github.com/mamoe/mirai/blob/dev/LICENSE
+ */
+
+package net.mamoe.console.integrationtest.testpoints.plugin
+
+import net.mamoe.console.integrationtest.AbstractTestPointAsPlugin
+import net.mamoe.mirai.console.extension.PluginComponentStorage
+import net.mamoe.mirai.console.internal.plugin.ConsoleJvmPluginFuncCallbackStatus
+import net.mamoe.mirai.console.internal.plugin.ConsoleJvmPluginFuncCallbackStatusExcept
+import net.mamoe.mirai.console.plugin.PluginManager
+import net.mamoe.mirai.console.plugin.id
+import net.mamoe.mirai.console.plugin.jvm.JvmPluginDescription
+import net.mamoe.mirai.console.plugin.jvm.KotlinPlugin
+import net.mamoe.mirai.utils.debug
+import org.junit.jupiter.api.Assertions.assertFalse
+import kotlin.test.assertEquals
+import kotlin.test.assertIs
+
+@ConsoleJvmPluginFuncCallbackStatusExcept.OnEnable(ConsoleJvmPluginFuncCallbackStatus.FAILED)
+internal object PluginWithExceptionTest : AbstractTestPointAsPlugin() {
+
+    override fun newPluginDescription(): JvmPluginDescription {
+        return JvmPluginDescription(
+            id = "net.mamoe.testpoint.plugin-with-exception-test",
+            version = "1.0.0",
+            name = "PluginWithExceptionTest",
+        )
+    }
+
+    override fun exceptionHandler(exception: Throwable, step: JvmPluginExecutionStep, instance: KotlinPlugin) {
+        instance.logger.debug { "PluginWithExceptionTestExceptionTest" }
+        assertIs<Exception>(exception)
+        assertEquals("PluginWithExceptionTestExceptionTest", exception.message)
+
+    }
+
+    override fun KotlinPlugin.onLoad0(storage: PluginComponentStorage) {
+
+    }
+
+    override fun KotlinPlugin.onEnable0() {
+        throw Exception("PluginWithExceptionTestExceptionTest")
+    }
+
+    override fun onConsoleStartSuccessfully() {
+        assertFalse {
+            PluginManager
+                .plugins
+                .first { it.id == "net.mamoe.testpoint.plugin-with-exception-test" }
+                .isEnabled
+        }
+    }
+}

mirai-console/backend/mirai-console/src/command/BuiltInCommands.kt

@@ -618,7 +618,11 @@ public object BuiltInCommands {
                     gray().append("<none>")
                 } else {
                     MiraiConsole.pluginManagerImpl.resolvedPlugins.joinTo(this) { plugin ->
-                        green().append(plugin.name).reset().append(" v").gold()
+                        if (plugin.isEnabled) {
+                            green().append(plugin.name).reset().append(" v").gold()
+                        } else {
+                            red().append(plugin.name).append("(disabled)").reset().append(" v").gold()
+                        }
                         plugin.version.toString()
                     }
                 }

mirai-console/backend/mirai-console/src/internal/MiraiConsoleImplementationBridge.kt

@@ -238,7 +238,7 @@ internal class MiraiConsoleImplementationBridge(
                 registeredCommand.permission // init
             }
 
-            mainLogger.info { "${pluginManager.plugins.size} plugin(s) enabled." }
+            mainLogger.info { "${pluginManager.plugins.count { it.isEnabled }} plugin(s) enabled." }
         }
 
         phase("auto-login bots") {

mirai-console/backend/mirai-console/src/internal/plugin/BuiltInJvmPluginLoaderImpl.kt

@@ -18,6 +18,7 @@ import net.mamoe.mirai.console.data.PluginDataStorage
 import net.mamoe.mirai.console.internal.util.PluginServiceHelper.findServices
 import net.mamoe.mirai.console.internal.util.PluginServiceHelper.loadAllServices
 import net.mamoe.mirai.console.plugin.PluginManager
+import net.mamoe.mirai.console.plugin.dependencies
 import net.mamoe.mirai.console.plugin.id
 import net.mamoe.mirai.console.plugin.jvm.*
 import net.mamoe.mirai.console.plugin.loader.AbstractFilePluginLoader
@@ -263,6 +264,16 @@ internal class BuiltInJvmPluginLoaderImpl(
         ensureActive()
         runCatching {
             logger.verbose { "Enabling plugin ${plugin.description.smartToString()}" }
+
+            val loadedPlugins = PluginManager.plugins
+            val failedDependencies = plugin.dependencies.asSequence().mapNotNull { dep ->
+                loadedPlugins.firstOrNull { it.id == dep.id }
+            }.filterNot { it.isEnabled }.toList()
+            if (failedDependencies.isNotEmpty()) {
+                logger.error("Failed to enable '${plugin.name}' because dependencies not enabled: " + failedDependencies.joinToString { "'${it.name}'" })
+                return
+            }
+
             if (plugin is JvmPluginInternal) {
                 plugin.internalOnEnable()
             } else plugin.onEnable()
@@ -270,7 +281,7 @@ internal class BuiltInJvmPluginLoaderImpl(
             // Extra space for logging align
             logger.verbose { "Enabled  plugin ${plugin.description.smartToString()}" }
         }.getOrElse {
-            throw PluginLoadException("Exception while loading ${plugin.description.name}", it)
+            throw PluginLoadException("Exception while enabling ${plugin.description.name}", it)
         }
     }
 

mirai-console/backend/mirai-console/src/internal/plugin/JvmPluginInternal.kt

@@ -34,7 +34,7 @@ import net.mamoe.mirai.utils.safeCast
 import java.io.File
 import java.io.InputStream
 import java.nio.file.Path
-import java.util.Objects
+import java.util.*
 import java.util.concurrent.locks.ReentrantLock
 import kotlin.coroutines.CoroutineContext
 
@@ -101,10 +101,14 @@ internal abstract class JvmPluginInternal(
             onSuccess = {
                 cancel(CancellationException("plugin disabled"))
             },
-            onFailure = {
-                cancel(CancellationException("Exception while disabling plugin", it))
+            onFailure = { err ->
+                cancel(CancellationException("Exception while disabling plugin", err))
+
+                // @TestOnly
+                if (err is ConsoleJvmPluginTestFailedError) throw err
+
                 if (MiraiConsoleImplementation.getInstance().consoleLaunchOptions.crashWhenPluginLoadFailed) {
-                    throw it
+                    throw err
                 }
             }
         )
@@ -122,18 +126,35 @@ internal abstract class JvmPluginInternal(
         parentPermission
         if (!firstRun) refreshCoroutineContext()
 
+        val except = javaClass.getDeclaredAnnotation(ConsoleJvmPluginFuncCallbackStatusExcept.OnEnable::class.java)
         kotlin.runCatching {
             onEnable()
         }.fold(
             onSuccess = {
+                if (except?.excepted == ConsoleJvmPluginFuncCallbackStatus.FAILED) {
+                    val msg = "Test point '${javaClass.name}' assets failed but onEnable() invoked successfully"
+                    cancel(msg)
+                    logger.error(msg)
+                    throw AssertionError(msg)
+                }
                 isEnabled = true
                 return true
             },
-            onFailure = {
-                cancel(CancellationException("Exception while enabling plugin", it))
-                logger.error(it)
+            onFailure = { err ->
+                cancel(CancellationException("Exception while enabling plugin", err))
+                logger.error(err)
+
+                // @TestOnly
+                if (err is ConsoleJvmPluginTestFailedError) throw err
+
+                when (except?.excepted) {
+                    ConsoleJvmPluginFuncCallbackStatus.SUCCESS -> throw err
+                    ConsoleJvmPluginFuncCallbackStatus.FAILED -> return false
+                    else -> {}
+                }
+
                 if (MiraiConsoleImplementation.getInstance().consoleLaunchOptions.crashWhenPluginLoadFailed) {
-                    throw it
+                    throw err
                 }
                 return false
             }
@@ -147,7 +168,9 @@ internal abstract class JvmPluginInternal(
         val classloader = javaClass.classLoader.safeCast<JvmPluginClassLoaderN>() ?: return
         val desc = try {
             Objects.requireNonNull(description)
-        } catch (ignored: NullPointerException) { return }
+        } catch (ignored: NullPointerException) {
+            return
+        }
         if (desc.dependencies.isEmpty()) {
             classloader.linkPluginLibraries(logger)
         }

mirai-console/backend/mirai-console/src/internal/plugin/JvmPluginTesting.kt

@@ -0,0 +1,37 @@
+/*
+ * Copyright 2019-2022 Mamoe Technologies and contributors.
+ *
+ * 此源代码的使用受 GNU AFFERO GENERAL PUBLIC LICENSE version 3 许可证的约束, 可以在以下链接找到该许可证.
+ * Use of this source code is governed by the GNU AGPLv3 license that can be found through the following link.
+ *
+ * https://github.com/mamoe/mirai/blob/dev/LICENSE
+ */
+
+package net.mamoe.mirai.console.internal.plugin
+
+import net.mamoe.mirai.utils.MiraiInternalApi
+
+/**
+ * 仅用于 Console 测试, 标记期望方法执行结果应该是 success 还是 failed
+ */
+@MiraiInternalApi
+public annotation class ConsoleJvmPluginFuncCallbackStatusExcept {
+    @MiraiInternalApi
+    @Target(AnnotationTarget.CLASS)
+    public annotation class OnEnable(
+        val excepted: ConsoleJvmPluginFuncCallbackStatus,
+    )
+}
+
+@MiraiInternalApi
+public enum class ConsoleJvmPluginFuncCallbackStatus {
+    SUCCESS, FAILED
+}
+
+@MiraiInternalApi
+public class ConsoleJvmPluginTestFailedError : Error {
+    public constructor() : super()
+    public constructor(cause: Throwable?) : super(cause)
+    public constructor(msg: String?, cause: Throwable?) : super(msg, cause)
+    public constructor(msg: String?) : super(msg)
+}