From 3b5ec941b350639b6e8ba6f38201243e6d82123a Mon Sep 17 00:00:00 2001
From: Him188 <Him188@mamoe.net>
Date: Fri, 23 Jul 2021 13:19:28 +0800
Subject: [PATCH] Always throw `MaxAttemptsReachedException` when for
consistency; Correctly count failure attempts; Tolerant `NetworkException`
thrown while `resumeInstanceCatchingException`, fix #1361
---
...AbstractKeepAliveNetworkHandlerSelector.kt | 42 ++++++++++++++-----
...KeepAliveNetworkHandlerSelectorRealTest.kt | 42 +++++++++++++++----
2 files changed, 64 insertions(+), 20 deletions(-)
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<H : NetworkHandl
private tailrec suspend fun runImpl(): H {
if (attempted >= 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<H : NetworkHandl
logIfEnabled { "... CLOSED" }
// exception will be collected by `exceptionCollector.collectException(current.getLastFailure())`
// so that duplicated exceptions are ignored in logging
+ return false
}
}
@@ -141,8 +159,10 @@ internal abstract class AbstractKeepAliveNetworkHandlerSelector<H : NetworkHandl
NetworkHandler.State.CONNECTING,
NetworkHandler.State.INITIALIZED,
-> {
- 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<TestNettyNH> = object : NetworkHandlerFactory<TestNettyNH> {
- override fun create(context: NetworkHandlerContext, address: SocketAddress): TestNettyNH {
- return object : TestNettyNH(bot, context, address) {
+ private lateinit var throwException: () -> Nothing
+
+ override val factory: NetworkHandlerFactory<TestNettyNH> =
+ NetworkHandlerFactory<TestNettyNH> { 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<Throwable> { 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<MyException> { selector.awaitResumeInstance() }
+ assertThrows<MaxAttemptsReachedException> { selector.awaitResumeInstance() }.let {
+ assertIs<NetworkException>(it.cause)
+ }
+ }
+
+ @Test
+ fun `throws MaxAttemptsReachedException with cause of original`() = runBlockingUnit {
+ throwException = {
+ throw MyException()
+ }
+ val selector = TestSelector(3) { createHandler() }
+ assertThrows<MaxAttemptsReachedException> { selector.awaitResumeInstance() }.let {
+ assertIs<MyException>(it.cause)
+ }
}
}