diff --git a/mirai-core/src/commonMain/kotlin/network/handler/selector/AbstractKeepAliveNetworkHandlerSelector.kt b/mirai-core/src/commonMain/kotlin/network/handler/selector/AbstractKeepAliveNetworkHandlerSelector.kt index a970a1966..17a29b6b7 100644 --- a/mirai-core/src/commonMain/kotlin/network/handler/selector/AbstractKeepAliveNetworkHandlerSelector.kt +++ b/mirai-core/src/commonMain/kotlin/network/handler/selector/AbstractKeepAliveNetworkHandlerSelector.kt @@ -10,10 +10,7 @@ package net.mamoe.mirai.internal.network.handler.selector import kotlinx.atomicfu.atomic -import kotlinx.coroutines.currentCoroutineContext -import kotlinx.coroutines.delay -import kotlinx.coroutines.isActive -import kotlinx.coroutines.yield +import kotlinx.coroutines.* import net.mamoe.mirai.internal.network.handler.NetworkHandler import net.mamoe.mirai.internal.network.handler.NetworkHandlerFactory import net.mamoe.mirai.internal.network.handler.logger @@ -85,30 +82,50 @@ internal abstract class AbstractKeepAliveNetworkHandlerSelector= maxAttempts) { logIfEnabled { "Max attempt $maxAttempts reached." } - throw exceptionCollector.getLast()?.apply { addSuppressed(MaxAttemptsReachedException(null)) } - ?: MaxAttemptsReachedException(null) + throw MaxAttemptsReachedException(exceptionCollector.getLast()) +// throw exceptionCollector.getLast()?.apply { addSuppressed(MaxAttemptsReachedException(null)) } +// ?: MaxAttemptsReachedException(null) } if (!currentCoroutineContext().isActive) { logIfEnabled { "Cancellation detected." } yield() // throw canonical CancellationException if cancelled + throw CancellationException("Cancellation detected.") } val current = getCurrentInstanceOrNull() lastNetwork = current - suspend fun H.resumeInstanceCatchingException() { + /** + * @return `false` if failed + */ + suspend fun H.resumeInstanceCatchingException(): Boolean { logIfEnabled { "Try resumeConnection" } try { resumeConnection() // once finished, it should has been LOADING or OK + return true } catch (e: LoginFailedException) { logIfEnabled { "... failed with LoginFailedException $e" } logIfEnabled { "CLOSING SELECTOR" } close(e) logIfEnabled { "... CLOSED" } + exceptionCollector.collect(e) if (e is RetryLaterException) { - return + return false } // LoginFailedException is not resumable - exceptionCollector.collectThrow(e) + exceptionCollector.throwLast() + } catch (e: NetworkException) { + logIfEnabled { "... failed with NetworkException $e" } + logIfEnabled { "... recoverable=${e.recoverable}" } + exceptionCollector.collect(e) + if (e.recoverable) { + logIfEnabled { "IGNORING" } + // allow recoverable NetworkException, see #1361 + } else { + logIfEnabled { "CLOSING SELECTOR" } + close(e) + logIfEnabled { "... CLOSED" } + } + return false } catch (e: Exception) { logIfEnabled { "... failed with $e" } logIfEnabled { "CLOSING SELECTOR" } @@ -116,6 +133,7 @@ internal abstract class AbstractKeepAliveNetworkHandlerSelector { - current.resumeInstanceCatchingException() - return runImpl() // does not count for an attempt. + if (!current.resumeInstanceCatchingException()) { + attempted += 1 + } + return runImpl() } NetworkHandler.State.LOADING -> { logIfEnabled { "RETURN" } diff --git a/mirai-core/src/commonTest/kotlin/network/handler/KeepAliveNetworkHandlerSelectorRealTest.kt b/mirai-core/src/commonTest/kotlin/network/handler/KeepAliveNetworkHandlerSelectorRealTest.kt index e821c4854..8de57c46e 100644 --- a/mirai-core/src/commonTest/kotlin/network/handler/KeepAliveNetworkHandlerSelectorRealTest.kt +++ b/mirai-core/src/commonTest/kotlin/network/handler/KeepAliveNetworkHandlerSelectorRealTest.kt @@ -14,38 +14,62 @@ package net.mamoe.mirai.internal.network.handler import io.netty.channel.Channel import net.mamoe.mirai.internal.network.framework.AbstractNettyNHTest import net.mamoe.mirai.internal.network.framework.TestNettyNH +import net.mamoe.mirai.internal.network.handler.selector.MaxAttemptsReachedException +import net.mamoe.mirai.internal.network.handler.selector.NetworkException import net.mamoe.mirai.internal.test.runBlockingUnit import net.mamoe.mirai.utils.TestOnly import org.junit.jupiter.api.assertThrows -import java.net.SocketAddress import kotlin.test.Test +import kotlin.test.assertIs internal class KeepAliveNetworkHandlerSelectorRealTest : AbstractNettyNHTest() { internal class FakeFailOnCreatingConnection : AbstractNettyNHTest() { private class MyException : Exception() - override val factory: NetworkHandlerFactory = object : NetworkHandlerFactory { - override fun create(context: NetworkHandlerContext, address: SocketAddress): TestNettyNH { - return object : TestNettyNH(bot, context, address) { + private lateinit var throwException: () -> Nothing + + override val factory: NetworkHandlerFactory = + NetworkHandlerFactory { context, address -> + object : TestNettyNH(bot, context, address) { override suspend fun createConnection(decodePipeline: PacketDecodePipeline): Channel = - throw MyException() + throwException() } } - } @Test - fun `should not tolerant any exception thrown by states`() = runBlockingUnit { + fun `should not tolerant any exception except NetworkException thrown by states`() = runBlockingUnit { // selector should not tolerant any exception during state initialization, or in the Jobs launched in states. + throwException = { + throw MyException() + } val selector = TestSelector(3) { createHandler() } assertThrows { selector.awaitResumeInstance() } } @Test - fun `should unwrap exception`() = runBlockingUnit { + fun `should tolerant NetworkException thrown by states`() = runBlockingUnit { + // selector should not tolerant any exception during state initialization, or in the Jobs launched in states. + throwException = { + throw object : NetworkException(true) {} + } + val selector = TestSelector(3) { createHandler() } - assertThrows { selector.awaitResumeInstance() } + assertThrows { selector.awaitResumeInstance() }.let { + assertIs(it.cause) + } + } + + @Test + fun `throws MaxAttemptsReachedException with cause of original`() = runBlockingUnit { + throwException = { + throw MyException() + } + val selector = TestSelector(3) { createHandler() } + assertThrows { selector.awaitResumeInstance() }.let { + assertIs(it.cause) + } } }