From 62022f1e192e76b44076a3da0279ccd6bb10c4ef Mon Sep 17 00:00:00 2001 From: Sho Sakuma Date: Wed, 5 Aug 2026 00:42:19 +0900 Subject: fix: stop treating cancellation as a conversion failure The catch-all around the Google IME call also caught the CancellationException from the caller's timeout, and then cached the unconverted hiragana. Since the words of a message are now converted concurrently, one timeout pinned every word of that message to its hiragana form for the life of the cache instead of just the word that timed out. The same swallowing let a cancelled delivery carry on past the plugin scope being cancelled, and reported the shutdown of an in-flight cross-server message as a SEVERE delivery failure. Co-Authored-By: Claude --- .../paper/converter/RomajiConversionHelper.kt | 12 ++++++++++-- .../lunaticChat/paper/converter/RomanjiConverter.kt | 6 ++++++ .../paper/velocity/CrossServerDirectMessageManager.kt | 12 +++++++++--- .../paper/converter/RomanjiConverterTest.kt | 18 ++++++++++++++++++ 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()) } + } } -- cgit v1.2.1