From 4f542befb027d4fe54bc29ee8482775b6cfcbeb1 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Sun, 1 Nov 2026 09:08:47 -0500 Subject: [PATCH] fix(bugsnag): group server errors by class and reason Bugsnag groups handled errors on the top frame, and for a CodeServerError that is the line that built it, such as SubmitIntentError.typed. When the line moves, the next release opens a new group for the same server result. groupingHashFor now hashes every CodeServerError, as the error or its cause, on its class name plus its message with list indexes, base58 addresses and numbers replaced by placeholders. ValidationException keeps its field:rule hash, and other throwables still return null. --- .../internal/startup/BugsnagErrorReporter.kt | 49 +++++++- .../startup/BugsnagErrorReporterTest.kt | 115 ++++++++++++++++++ 2 files changed, 160 insertions(+), 4 deletions(-) diff --git a/apps/flipcash/app/src/main/kotlin/com/flipcash/app/internal/startup/BugsnagErrorReporter.kt b/apps/flipcash/app/src/main/kotlin/com/flipcash/app/internal/startup/BugsnagErrorReporter.kt index 6025c1fbb3..cd40600784 100644 --- a/apps/flipcash/app/src/main/kotlin/com/flipcash/app/internal/startup/BugsnagErrorReporter.kt +++ b/apps/flipcash/app/src/main/kotlin/com/flipcash/app/internal/startup/BugsnagErrorReporter.kt @@ -3,6 +3,7 @@ package com.flipcash.app.internal.startup import com.bugsnag.android.Bugsnag import com.bugsnag.android.Severity import com.getcode.opencode.model.core.errors.ValidationException +import com.getcode.utils.CodeServerError import com.getcode.utils.ErrorReporter class BugsnagErrorReporter : ErrorReporter { @@ -19,10 +20,50 @@ class BugsnagErrorReporter : ErrorReporter { internal companion object { /** - * Validation failures all share one `CodeServerError.` frame, so Bugsnag would fold - * every field into one group. Group by field path and rule instead. + * Bugsnag groups on the top stack frame, which for a [CodeServerError] is the line that + * built it (`SubmitIntentError.typed` and friends). That line moves between releases, so + * each release would open a new group for the same server result. Group on the cause + * instead: + * + * - [ValidationException]: field path and rule (see [ValidationException.groupingHash]). + * - Any other [CodeServerError]: class name plus its [normalized][normalizeMessage] + * message. The message carries the server's reason, and one class can carry several + * (`SubmitIntentError.StaleState` has six, some notifiable and some not), so the class + * name alone would merge them. The class name is safe to hash because + * `proguard-rules.pro` keeps the names of public `Throwable` subclasses. + * + * The reported [error] is checked before [cause], matching the exception Bugsnag shows. + * Anything else returns null and keeps Bugsnag's stack grouping, where the stack is the + * signal. */ - fun groupingHashFor(error: Throwable, cause: Throwable): String? = - ((error as? ValidationException) ?: (cause as? ValidationException))?.groupingHash + fun groupingHashFor(error: Throwable, cause: Throwable): String? { + ((error as? ValidationException) ?: (cause as? ValidationException)) + ?.let { return it.groupingHash } + + val serverError = (error as? CodeServerError) ?: (cause as? CodeServerError) + ?: return null + // `message` is CodeServerError's own property, without the "CodeServerError: " + // prefix it passes to Throwable. + return "${serverError.javaClass.name}:${normalizeMessage(serverError.message)}" + } + + /** + * Replaces the values servers embed in reason strings (amounts, account addresses, list + * indexes) with placeholders, so two failures with the same reason share a group. + */ + fun normalizeMessage(message: String?): String = + message.orEmpty() + .replace(ListIndex, "") + .replace(Base58Token, "
") + .replace(Number, "") + + private val ListIndex = Regex("""\[\d+]""") + + // Solana addresses and signatures: 32+ base58 characters. Requiring a letter keeps a + // long amount in the Number bucket, so an amount's length can't change the hash. + private val Base58Token = + Regex("""\b(?=[1-9A-HJ-NP-Za-km-z]*[A-HJ-NP-Za-km-z])[1-9A-HJ-NP-Za-km-z]{32,}\b""") + + private val Number = Regex("""\d+(?:\.\d+)?""") } } diff --git a/apps/flipcash/app/src/test/kotlin/com/flipcash/app/internal/startup/BugsnagErrorReporterTest.kt b/apps/flipcash/app/src/test/kotlin/com/flipcash/app/internal/startup/BugsnagErrorReporterTest.kt index 4502da26f0..8e713c9f3b 100644 --- a/apps/flipcash/app/src/test/kotlin/com/flipcash/app/internal/startup/BugsnagErrorReporterTest.kt +++ b/apps/flipcash/app/src/test/kotlin/com/flipcash/app/internal/startup/BugsnagErrorReporterTest.kt @@ -1,9 +1,13 @@ package com.flipcash.app.internal.startup +import com.getcode.opencode.model.core.errors.SubmitIntentError +import com.getcode.opencode.model.core.errors.SwapError import com.getcode.opencode.model.core.errors.ValidationException import com.getcode.opencode.model.core.errors.Violation import org.junit.Test import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertNotEquals import kotlin.test.assertNull class BugsnagErrorReporterTest { @@ -11,6 +15,9 @@ class BugsnagErrorReporterTest { private fun validation(field: String, rule: String) = ValidationException(listOf(Violation(field = field, rule = rule, message = "m"))) + private fun hash(error: Throwable, cause: Throwable = error) = + BugsnagErrorReporter.groupingHashFor(error, cause) + @Test fun `grouping hash comes from the validation error or its cause`() { val error = validation("messages[3].content", "string.max_len") @@ -20,6 +27,14 @@ class BugsnagErrorReporterTest { assertEquals(error.groupingHash, BugsnagErrorReporter.groupingHashFor(wrapped, error)) } + @Test + fun `validation hash stays field path and rule`() { + assertEquals( + "messages.content:string.max_len", + hash(validation("messages[3].content", "string.max_len")), + ) + } + @Test fun `messages indexes with the same rule share a hash and other rules do not`() { val a = BugsnagErrorReporter.groupingHashFor( @@ -39,5 +54,105 @@ class BugsnagErrorReporterTest { @Test fun `other errors keep Bugsnag default grouping`() { assertNull(BugsnagErrorReporter.groupingHashFor(RuntimeException(), RuntimeException())) + assertNull(hash(IllegalStateException("gift card balance has already been claimed"))) + } + + private fun claimedFromSubmit() = + SubmitIntentError.StaleState(listOf("gift card balance has already been claimed")) + + private fun claimedFromRetry() = + SubmitIntentError.StaleState(listOf("gift card balance has already been claimed")) + + @Test + fun `same server error built at different call sites shares a hash`() { + val a = claimedFromSubmit() + val b = claimedFromRetry() + + assertNotEquals(a.stackTrace.first(), b.stackTrace.first()) + assertEquals(hash(a), hash(b)) + assertEquals( + "com.getcode.opencode.model.core.errors.SubmitIntentError\$StaleState:" + + "gift card balance has already been claimed", + hash(a), + ) + } + + @Test + fun `hash leaves out the CodeServerError message prefix`() { + assertFalse(hash(claimedFromSubmit())!!.contains("CodeServerError:")) + } + + @Test + fun `each stale state reason gets its own hash`() { + val reasons = listOf( + "race detected: $AddressA", + "gift card balance has already been claimed", + "gift card is expired", + "pool balance has already been distributed", + "intent already exists", + "account is already opened", + ) + + val hashes = reasons.map { hash(SubmitIntentError.StaleState(listOf(it))) }.toSet() + + assertEquals(reasons.size, hashes.size) + } + + @Test + fun `messages differing only in amount or address share a hash`() { + assertEquals( + hash(SwapError.Denied(listOf("fee amount must be 184467440737095440 quarks"))), + hash(SwapError.Denied(listOf("fee amount must be 5 quarks"))), + ) + assertEquals( + hash(SubmitIntentError.InvalidIntent(listOf("actions[0]: $AddressA has insufficient balance"))), + hash(SubmitIntentError.InvalidIntent(listOf("actions[3]: $AddressB has insufficient balance"))), + ) + assertEquals( + hash(SubmitIntentError.Denied(listOf("payment amount is below the fee of 3 USDC"))), + hash(SubmitIntentError.Denied(listOf("payment amount is below the fee of 0.25 USDC"))), + ) + } + + @Test + fun `same message on different classes stays apart`() { + assertNotEquals( + hash(SubmitIntentError.Denied(listOf("denied"))), + hash(SwapError.Denied(listOf("denied"))), + ) + } + + @Test + fun `a server error is hashed from the cause when the reported error is not one`() { + val cause = claimedFromSubmit() + + assertEquals(hash(cause), hash(RuntimeException("wrapped", cause), cause)) + } + + @Test + fun `the reported server error wins over a server error cause`() { + val cause = claimedFromSubmit() + val error = SubmitIntentError.Other(cause = cause) + + assertEquals( + "com.getcode.opencode.model.core.errors.SubmitIntentError\$Other:" + + "gift card balance has already been claimed", + hash(error, cause), + ) + } + + @Test + fun `normalizer replaces indexes, addresses and numbers`() { + assertEquals( + "actions:
has of quarks", + BugsnagErrorReporter.normalizeMessage("actions[12]: $AddressA has 1.5 of 184467440737095440 quarks"), + ) + assertEquals("", BugsnagErrorReporter.normalizeMessage(null)) + } + + private companion object { + // Synthetic base58 strings, the length of a Solana address. + const val AddressA = "3xKq7VbYwZ9mNpRtUfGhJkLcDeA2sB4nP6oQ8rStUvWx" + const val AddressB = "9ZyXwVuTsRqPoNmLkJhGfEdCbA87654321zyxwvutsrq" } }