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" } }