From 1c7e3bc5a1e8accfd18d479f5224e48cb84dc6c9 Mon Sep 17 00:00:00 2001 From: Him188 Date: Fri, 13 May 2022 15:01:45 +0100 Subject: [PATCH] Revise exception handling in NetworkHandler, involving: - HeartbeatProcessor - HeartbeatFailedException: IOException is not recoverable, since this is not even thrown --- .../network/components/HeartbeatProcessor.kt | 32 ++++++++++++------- .../network/components/HeartbeatScheduler.kt | 8 +++-- .../kotlin/network/handler/NetworkHandler.kt | 9 ++++++ .../network/handler/NetworkHandlerSupport.kt | 5 +++ .../impl/netty/HeartbeatFailedException.kt | 12 +++---- .../network/protocol/packet/OutgoingPacket.kt | 2 +- .../AbstractRealNetworkHandlerTest.kt | 10 +++--- .../network/handler/SelectorRecoveryTest.kt | 27 ++++++++-------- .../impl/netty/NettyBotNormalLoginTest.kt | 5 ++- .../impl/netty/NettyHandlerEventTest.kt | 1 + 10 files changed, 71 insertions(+), 40 deletions(-) diff --git a/mirai-core/src/commonMain/kotlin/network/components/HeartbeatProcessor.kt b/mirai-core/src/commonMain/kotlin/network/components/HeartbeatProcessor.kt index 811d8fdac..5119961d8 100644 --- a/mirai-core/src/commonMain/kotlin/network/components/HeartbeatProcessor.kt +++ b/mirai-core/src/commonMain/kotlin/network/components/HeartbeatProcessor.kt @@ -1,14 +1,16 @@ /* - * 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. + * 此源代码的使用受 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/master/LICENSE + * https://github.com/mamoe/mirai/blob/dev/LICENSE */ package net.mamoe.mirai.internal.network.components +import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.TimeoutCancellationException import net.mamoe.mirai.internal.network.component.ComponentKey import net.mamoe.mirai.internal.network.handler.NetworkHandler import net.mamoe.mirai.internal.network.protocol.packet.login.Heartbeat @@ -16,21 +18,31 @@ import net.mamoe.mirai.internal.network.protocol.packet.login.StatSvc import net.mamoe.mirai.internal.network.protocol.packet.sendAndExpect internal interface HeartbeatProcessor { - - @Throws(Exception::class) + /** + * @throws TimeoutCancellationException if timed out waiting for response. + * @throws CancellationException if [networkHandler] closed. + * @throws Exception any other exceptions considered critical internal error and will stop SSO (i.e. stop Bot). + */ suspend fun doAliveHeartbeatNow(networkHandler: NetworkHandler) - @Throws(Exception::class) + /** + * @throws TimeoutCancellationException if timed out waiting for response. + * @throws CancellationException if [networkHandler] closed. + * @throws Exception any other exceptions considered critical internal error and will stop SSO (i.e. stop Bot). + */ suspend fun doStatHeartbeatNow(networkHandler: NetworkHandler) - @Throws(Exception::class) + /** + * @throws TimeoutCancellationException if timed out waiting for response. + * @throws CancellationException if [networkHandler] closed. + * @throws Exception any other exceptions considered critical internal error and will stop SSO (i.e. stop Bot). + */ suspend fun doRegisterNow(networkHandler: NetworkHandler): StatSvc.Register.Response companion object : ComponentKey } internal class HeartbeatProcessorImpl : HeartbeatProcessor { - @Throws(Exception::class) override suspend fun doStatHeartbeatNow(networkHandler: NetworkHandler) { StatSvc.SimpleGet(networkHandler.context.bot.client).sendAndExpect( networkHandler, @@ -39,7 +51,6 @@ internal class HeartbeatProcessorImpl : HeartbeatProcessor { ) } - @Throws(Exception::class) override suspend fun doAliveHeartbeatNow(networkHandler: NetworkHandler) { Heartbeat.Alive(networkHandler.context.bot.client).sendAndExpect( networkHandler, @@ -48,7 +59,6 @@ internal class HeartbeatProcessorImpl : HeartbeatProcessor { ) } - @Throws(Exception::class) override suspend fun doRegisterNow(networkHandler: NetworkHandler): StatSvc.Register.Response { return networkHandler.context[SsoProcessor].sendRegister(networkHandler) } diff --git a/mirai-core/src/commonMain/kotlin/network/components/HeartbeatScheduler.kt b/mirai-core/src/commonMain/kotlin/network/components/HeartbeatScheduler.kt index 6f3290cb5..8cc5398a5 100644 --- a/mirai-core/src/commonMain/kotlin/network/components/HeartbeatScheduler.kt +++ b/mirai-core/src/commonMain/kotlin/network/components/HeartbeatScheduler.kt @@ -13,11 +13,15 @@ import kotlinx.coroutines.* import net.mamoe.mirai.internal.network.component.ComponentKey import net.mamoe.mirai.internal.network.component.ComponentStorage import net.mamoe.mirai.internal.network.handler.NetworkHandlerSupport +import net.mamoe.mirai.internal.network.handler.selector.NetworkException import net.mamoe.mirai.internal.network.handler.selector.PacketTimeoutException import net.mamoe.mirai.utils.BotConfiguration.HeartbeatStrategy.* import net.mamoe.mirai.utils.MiraiLogger import net.mamoe.mirai.utils.info +/** + * Accepts any kinds of exceptions. A [NetworkException] can control whether this error is recoverable, while any other ones are regarded as unexpected failure. + */ internal typealias HeartbeatFailureHandler = (name: String, e: Throwable) -> Unit /** @@ -127,13 +131,13 @@ internal class TimeBasedHeartbeatSchedulerImpl( name, PacketTimeoutException( "$coroutineName: Timeout receiving action response", - cause + cause // cause is TimeoutCancellationException from `action` ) // This is a NetworkException that is recoverable ) return@async } } catch (e: Throwable) { - // catch other errors + // catch other errors in `action`, should not happen onHeartFailure( name, IllegalStateException("$coroutineName: Internal error: caught unexpected exception", e) diff --git a/mirai-core/src/commonMain/kotlin/network/handler/NetworkHandler.kt b/mirai-core/src/commonMain/kotlin/network/handler/NetworkHandler.kt index 83e10f51a..7f6eb2356 100644 --- a/mirai-core/src/commonMain/kotlin/network/handler/NetworkHandler.kt +++ b/mirai-core/src/commonMain/kotlin/network/handler/NetworkHandler.kt @@ -9,7 +9,9 @@ package net.mamoe.mirai.internal.network.handler +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.TimeoutCancellationException import kotlinx.coroutines.channels.ReceiveChannel import kotlinx.coroutines.flow.collect import kotlinx.coroutines.flow.consumeAsFlow @@ -131,6 +133,10 @@ internal interface NetworkHandler : CoroutineScope { * Coroutine suspension may happen if connection is not yet available however, * [IllegalStateException] is thrown if [NetworkHandler] is already in [State.CLOSED] since closure is final. * + * @throws TimeoutCancellationException if timeout has been reached. + * @throws CancellationException if the [NetworkHandler] is closed, with the last cause for closure. + * @throws IllegalArgumentException if [timeout] or [attempts] are invalid. + * * @param attempts ranges `1..INFINITY` */ suspend fun sendAndExpect(packet: OutgoingPacket, timeout: Long = 5000, attempts: Int = 2): Packet? @@ -141,6 +147,9 @@ internal interface NetworkHandler : CoroutineScope { * Response is still being processed but not passed as a return value of this function, so it does not suspends this function (due to awaiting for the response). * However, coroutine is still suspended if connection is not yet available, * and [IllegalStateException] is thrown if [NetworkHandler] is already in [State.CLOSED] since closure is final. + * legalStateException] is thrown if [NetworkHandler] is already in [State.CLOSED] since closure is final. + * + * @throws CancellationException if the [NetworkHandler] is closed, with the last cause for closure. */ suspend fun sendWithoutExpect(packet: OutgoingPacket) diff --git a/mirai-core/src/commonMain/kotlin/network/handler/NetworkHandlerSupport.kt b/mirai-core/src/commonMain/kotlin/network/handler/NetworkHandlerSupport.kt index ce04e7319..9050c1f16 100644 --- a/mirai-core/src/commonMain/kotlin/network/handler/NetworkHandlerSupport.kt +++ b/mirai-core/src/commonMain/kotlin/network/handler/NetworkHandlerSupport.kt @@ -43,6 +43,11 @@ internal abstract class NetworkHandlerSupport( .plus(CoroutineExceptionHandler.fromMiraiLogger(logger)) protected abstract fun initialState(): BaseStateImpl + + /** + * It's not guaranteed whether this function sends the packet in-place or launches a coroutine for it. + * Caller should not rely on this property. + */ protected abstract suspend fun sendPacketImpl(packet: OutgoingPacket) protected fun collectUnknownPacket(raw: RawIncomingPacket) { diff --git a/mirai-core/src/commonMain/kotlin/network/impl/netty/HeartbeatFailedException.kt b/mirai-core/src/commonMain/kotlin/network/impl/netty/HeartbeatFailedException.kt index bcdc97b6c..14858d619 100644 --- a/mirai-core/src/commonMain/kotlin/network/impl/netty/HeartbeatFailedException.kt +++ b/mirai-core/src/commonMain/kotlin/network/impl/netty/HeartbeatFailedException.kt @@ -1,22 +1,20 @@ /* - * 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. + * 此源代码的使用受 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/master/LICENSE + * https://github.com/mamoe/mirai/blob/dev/LICENSE */ package net.mamoe.mirai.internal.network.impl.netty import net.mamoe.mirai.internal.network.handler.selector.NetworkException -import net.mamoe.mirai.utils.unwrapCancellationException -import java.io.IOException internal class HeartbeatFailedException( private val name: String, // kind of HB override val cause: Throwable, - recoverable: Boolean = cause.unwrapCancellationException() is IOException || cause is NetworkException && cause.recoverable, + recoverable: Boolean = cause is NetworkException && cause.recoverable, ) : NetworkException(recoverable) { override val message: String = "Exception in $name job" override fun toString(): String = "HeartbeatFailedException: $name, recoverable=$recoverable, cause=$cause" diff --git a/mirai-core/src/commonMain/kotlin/network/protocol/packet/OutgoingPacket.kt b/mirai-core/src/commonMain/kotlin/network/protocol/packet/OutgoingPacket.kt index 44fd5c0f7..bb7adab50 100644 --- a/mirai-core/src/commonMain/kotlin/network/protocol/packet/OutgoingPacket.kt +++ b/mirai-core/src/commonMain/kotlin/network/protocol/packet/OutgoingPacket.kt @@ -49,7 +49,7 @@ internal class IncomingPacket private constructor( val commandName: String, val sequenceId: Int, - val result: Either + val result: Either // exception will be the same as caught from PacketFactory.decode. So they can be ISE, NPE, etc. ) { companion object { operator fun invoke(commandName: String, sequenceId: Int, data: Packet?) = diff --git a/mirai-core/src/commonTest/kotlin/network/framework/AbstractRealNetworkHandlerTest.kt b/mirai-core/src/commonTest/kotlin/network/framework/AbstractRealNetworkHandlerTest.kt index 2efe8f036..8e1c8ec62 100644 --- a/mirai-core/src/commonTest/kotlin/network/framework/AbstractRealNetworkHandlerTest.kt +++ b/mirai-core/src/commonTest/kotlin/network/framework/AbstractRealNetworkHandlerTest.kt @@ -1,10 +1,10 @@ /* - * 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. + * 此源代码的使用受 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/master/LICENSE + * https://github.com/mamoe/mirai/blob/dev/LICENSE */ package net.mamoe.mirai.internal.network.framework @@ -117,6 +117,7 @@ internal sealed class AbstractRealNetworkHandlerTest : Abstr override suspend fun init() { nhEvents.add(NHEvent.Init) networkLogger.debug { "BotInitProcessor.init" } + bot.components[SsoProcessor].firstLoginResult.value = FirstLoginResult.PASSED } }) set(ServerList, ServerListImpl()) @@ -161,6 +162,7 @@ internal sealed class AbstractRealNetworkHandlerTest : Abstr } val eventDispatcher get() = bot.components[EventDispatcher] + val firstLoginResult: FirstLoginResult? get() = bot.components[SsoProcessor].firstLoginResult.value } internal fun AbstractRealNetworkHandlerTest<*>.setSsoProcessor(action: suspend SsoProcessor.(handler: NetworkHandler) -> Unit) { diff --git a/mirai-core/src/commonTest/kotlin/network/handler/SelectorRecoveryTest.kt b/mirai-core/src/commonTest/kotlin/network/handler/SelectorRecoveryTest.kt index e489e6a0d..490488b99 100644 --- a/mirai-core/src/commonTest/kotlin/network/handler/SelectorRecoveryTest.kt +++ b/mirai-core/src/commonTest/kotlin/network/handler/SelectorRecoveryTest.kt @@ -18,14 +18,26 @@ import net.mamoe.mirai.internal.network.framework.AbstractNettyNHTestWithSelecto import net.mamoe.mirai.internal.network.impl.netty.HeartbeatFailedException import net.mamoe.mirai.internal.network.impl.netty.NettyChannelException import net.mamoe.mirai.internal.test.runBlockingUnit +import org.junit.jupiter.api.AfterEach +import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test -import java.io.IOException +import org.junit.jupiter.api.TestInfo import kotlin.test.assertFails /** * Test whether the selector can recover the connection after first successful login. */ internal class SelectorRecoveryTest : AbstractNettyNHTestWithSelector() { + @BeforeEach + fun beforeTest(info: TestInfo) { + println("=".repeat(30) + "BEGIN: ${info.displayName}" + "=".repeat(30)) + } + + @AfterEach + fun afterTest(info: TestInfo) { + println("=".repeat(31) + "END: ${info.displayName}" + "=".repeat(31)) + } + @Test fun `stop on manual close`() = runBlockingUnit { network.resumeConnection() @@ -33,19 +45,6 @@ internal class SelectorRecoveryTest : AbstractNettyNHTestWithSelector() { assertFails { network.resumeConnection() } } - /** - * Emulates system hibernation and network failure. - * @see HeartbeatFailedException - */ - @Test - fun `can recover on heartbeat failure with IOException`() = runBlockingUnit { - // We allow IOException to cause a reconnect. - testRecoverWhenHeartbeatFailWith { IOException("test IO ex") } - - // BotOfflineMonitor immediately launches a recovery which is UNDISPATCHED, so connection is immediately recovered. - assertState(NetworkHandler.State.CONNECTING, NetworkHandler.State.LOADING, NetworkHandler.State.OK) - } - /** * Emulates system hibernation and network failure. * @see HeartbeatFailedException diff --git a/mirai-core/src/commonTest/kotlin/network/impl/netty/NettyBotNormalLoginTest.kt b/mirai-core/src/commonTest/kotlin/network/impl/netty/NettyBotNormalLoginTest.kt index 4defc4983..b6aa65035 100644 --- a/mirai-core/src/commonTest/kotlin/network/impl/netty/NettyBotNormalLoginTest.kt +++ b/mirai-core/src/commonTest/kotlin/network/impl/netty/NettyBotNormalLoginTest.kt @@ -13,6 +13,7 @@ import kotlinx.coroutines.delay import kotlinx.coroutines.isActive import net.mamoe.mirai.internal.network.components.BotOfflineEventMonitor import net.mamoe.mirai.internal.network.components.BotOfflineEventMonitorImpl +import net.mamoe.mirai.internal.network.components.FirstLoginResult import net.mamoe.mirai.internal.network.framework.AbstractNettyNHTest import net.mamoe.mirai.internal.network.framework.TestNettyNH import net.mamoe.mirai.internal.network.framework.setSsoProcessor @@ -27,6 +28,7 @@ import net.mamoe.mirai.utils.cast import org.junit.jupiter.api.AfterEach import org.junit.jupiter.api.Test import java.io.IOException +import kotlin.test.assertEquals import kotlin.test.assertFailsWith import kotlin.test.assertFalse @@ -93,8 +95,9 @@ internal class NettyBotNormalLoginTest : AbstractNettyNHTest() { // } @Test - fun `test resume after MsfOffline received`() = runBlockingUnit { + fun `test resume after MsfOffline received after first login`() = runBlockingUnit { bot.login() + assertEquals(FirstLoginResult.PASSED, firstLoginResult) bot.network.close(StatSvc.ReqMSFOffline.MsfOfflineToken(0, 0, 0)) eventDispatcher.joinBroadcast() diff --git a/mirai-core/src/commonTest/kotlin/network/impl/netty/NettyHandlerEventTest.kt b/mirai-core/src/commonTest/kotlin/network/impl/netty/NettyHandlerEventTest.kt index 34becd928..92bf9d3d9 100644 --- a/mirai-core/src/commonTest/kotlin/network/impl/netty/NettyHandlerEventTest.kt +++ b/mirai-core/src/commonTest/kotlin/network/impl/netty/NettyHandlerEventTest.kt @@ -145,6 +145,7 @@ internal class NettyHandlerEventTest : AbstractNettyNHTest() { fun `BotOffline from OK TO CLOSED by bot close`() = runBlockingUnit { bot.login() assertState(OK) + assertEquals(FirstLoginResult.PASSED, firstLoginResult) eventDispatcher.joinBroadcast() // `login` launches a job which broadcasts the event assertEventBroadcasts(1) { assertTrue { bot.isActive }