diff options
4 files changed, 43 insertions, 5 deletions
diff --git a/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/converter/RomajiConversionHelper.kt b/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/converter/RomajiConversionHelper.kt index 8c656ec..4da017f 100644 --- a/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/converter/RomajiConversionHelper.kt +++ b/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/converter/RomajiConversionHelper.kt @@ -1,5 +1,6 @@ package dev.m1sk9.lunaticChat.paper.converter +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.runBlocking import kotlinx.coroutines.withTimeoutOrNull @@ -17,11 +18,18 @@ suspend fun convertWithRomaji( converter: RomanjiConverter, timeoutMs: Long = 1000, ): String = - runCatching { + try { withTimeoutOrNull(timeoutMs) { converter.convert(message) }?.let { "$message §e($it)" } ?: message - }.getOrElse { message } + } catch (e: CancellationException) { + // Rethrown rather than degraded to the original message: this runs on the delivery queue, so + // swallowing it would let a message be delivered after the plugin scope has been cancelled. + // The conversion's own timeout is handled by withTimeoutOrNull and does not reach here. + throw e + } catch (_: Exception) { + message + } /** * Blocking form of [convertWithRomaji], for callers that cannot suspend. diff --git a/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/converter/RomanjiConverter.kt b/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/converter/RomanjiConverter.kt index 7908824..530d3dc 100644 --- a/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/converter/RomanjiConverter.kt +++ b/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/converter/RomanjiConverter.kt @@ -2,6 +2,7 @@ package dev.m1sk9.lunaticChat.paper.converter import dev.m1sk9.lunaticChat.engine.converter.GoogleIMEClient import dev.m1sk9.lunaticChat.engine.converter.KanaConverter +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.async import kotlinx.coroutines.awaitAll import kotlinx.coroutines.coroutineScope @@ -85,6 +86,11 @@ class RomanjiConverter( val converted = try { apiClient.convert(hiragana) + } catch (e: CancellationException) { + // Not an API failure: the caller's timeout fired. Caching the hiragana here would + // pin every word of the message to its unconverted form for good, because the words + // are converted concurrently and the timeout cancels all of them at once. + throw e } catch (e: Exception) { logger.warning("Failed to convert $hiragana: ${e.message}") hiragana // Use hiragana if API fails diff --git a/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/velocity/CrossServerDirectMessageManager.kt b/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/velocity/CrossServerDirectMessageManager.kt index 307ca8c..ccd8d40 100644 --- a/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/velocity/CrossServerDirectMessageManager.kt +++ b/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/velocity/CrossServerDirectMessageManager.kt @@ -7,6 +7,7 @@ import dev.m1sk9.lunaticChat.paper.chat.handler.DirectMessageHandler import dev.m1sk9.lunaticChat.paper.config.LunaticChatConfiguration import dev.m1sk9.lunaticChat.paper.i18n.LanguageManager import dev.m1sk9.lunaticChat.paper.i18n.MessageFormatter +import kotlinx.coroutines.CancellationException import org.bukkit.entity.Player import org.bukkit.plugin.Plugin import java.util.UUID @@ -35,9 +36,10 @@ class CrossServerDirectMessageManager( /** * Sends a direct message to a player on another server through Velocity. * - * Must be called on the main server thread. The sender-side display, spy - * notification and reply recording are handled by [DirectMessageHandler]; - * the (possibly romaji-converted) body is what gets relayed. + * Runs on the sender's delivery queue, off the tick thread, because the romaji conversion it + * goes through may wait on the Google IME API. The reply target is recorded by the command + * before the work is queued; the sender-side display and the spy notification are handled by + * [DirectMessageHandler], and the (possibly romaji-converted) body is what gets relayed. */ suspend fun sendCrossServerMessage( sender: Player, @@ -73,6 +75,10 @@ class CrossServerDirectMessageManager( "Sent direct message to Velocity: messageId=$messageId, " + "target=$targetName@$targetServerName" } + } catch (e: CancellationException) { + // Shutdown cancelling the delivery queue is not a delivery failure, and reporting it as + // SEVERE while carrying on past the cancellation would be wrong twice over. + throw e } catch (e: Exception) { logger.log(Level.SEVERE, "Failed to send cross-server direct message", e) } diff --git a/platform-paper/src/test/kotlin/dev/m1sk9/lunaticChat/paper/converter/RomanjiConverterTest.kt b/platform-paper/src/test/kotlin/dev/m1sk9/lunaticChat/paper/converter/RomanjiConverterTest.kt index a7e0a52..eba2c54 100644 --- a/platform-paper/src/test/kotlin/dev/m1sk9/lunaticChat/paper/converter/RomanjiConverterTest.kt +++ b/platform-paper/src/test/kotlin/dev/m1sk9/lunaticChat/paper/converter/RomanjiConverterTest.kt @@ -9,6 +9,7 @@ import io.mockk.mockk import io.mockk.verify import kotlinx.coroutines.delay import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.withTimeoutOrNull import java.util.concurrent.atomic.AtomicInteger import kotlin.test.Test import kotlin.test.assertEquals @@ -319,4 +320,21 @@ class RomanjiConverterTest { assertEquals(3, peakInFlight.get(), "each word should be in flight at the same time") } + + @Test + fun `a message-level timeout caches nothing`() = + runBlocking { + val (converter, cache, apiClient) = createConverter() + coEvery { apiClient.convert(any()) } coAnswers { + delay(1_000) + "変換" + } + + val result = withTimeoutOrNull(100) { converter.convert("konnichiwa ohayou arigatou") } + + assertNull(result) + // The words share one timeout, so caching the hiragana fallback here would pin the whole + // message - not just one word - to its unconverted form for the life of the cache. + verify(exactly = 0) { cache.put(any(), any()) } + } } |