Repository navigation
fix(bugsnag): group server errors by class and reason - #1930
Merged
Merged
Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bugsnag groups handled errors on the top stack frame. For a
CodeServerErrorthat frame is the line that built it, e.g.SubmitIntentError$Companion.typed, so when that line moves the next release opens a new group for the same server result. That's howSubmitIntentError$StaleState"gift card balance has already been claimed" ended up in two groups keyed on lines 213 and 215, andSendMessageError$Deniedin four, one per release. #1921 fixed this forValidationExceptiononly.This implements the Android
CodeServerErrorpart of Decision 4 indocs/specs/bugsnag-cleanup.md.BugsnagErrorReporter.groupingHashFornow returns, in order:ValidationException(error or cause): its existingfield:rulehash, unchanged.CodeServerError, with the reported error checked before its cause:<class binary name>:<normalized message>.null, so crashes keep Bugsnag's stack grouping.The message is part of the hash because one class can carry several reasons.
StaleStatehas six ("race detected", "gift card balance has already been claimed", "intent already exists", ...), and only some are notifiable, so a class-only hash would put aWARNINGinside the gift cardINFOgroup. None of the error classes expose a structured reason code:SubmitIntentError,SwapErrorand the rest keep the proto's reason strings in private lists and join them intomessage. So every class hashes on the normalized message.
has insufficient..." in one group whatever the amount or account. A base58 run must contain a letter, so a long amount still becomesnormalizeMessageremoves[N]indexes and replaces base58 runs of 32+ characters with<address>and numbers (including decimals) with<n>. That keeps messages likeSwapError$Denied"fee amount must be 184467440737095440 quarks" andSubmitIntentError$InvalidIntent"actions[0]:<n>and the amount's length can't change the hash.The class name is stable across builds because
proguard-rules.prokeeps the names of publicThrowablesubclasses (-keepnames public class * extends java.lang.Throwable). A scan of the compiled classes found 722CodeServerErrorsubclasses, all withACC_PUBLICset, so the rule covers each of them.BugsnagErrorReporterTestadds cases for: the same error built at two call sites sharing a hash, the sixStaleStatereasons getting six hashes, messages that differ only in an amount or address sharing a hash, the same message on two classes staying apart, the reported error winning over a server-error cause, and the hash dropping theCodeServerError:prefix. TheValidationExceptionhash and thenullcase for other throwables are pinned too.