Repository navigation
fix(bugsnag): group validation failures by field and rule - #1921
Merged
Merged
Conversation
Keep the failed rule on Violation, report failures on client-filled fields (addresses, address, user_id) at WARNING, and set the Bugsnag grouping hash for ValidationException to the field path without list indexes plus the rule.
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 group 6aba977 ("Request validation failed") holds 1,057 events from eight different fields. Every
ValidationExceptionis built from the sameCodeServerError.<init>frame, so Bugsnag groups them together, andThrowable.ktdropped the failed rule, leaving nothing but the message to tell them apart. Hashing on the message would split the groups again whenever a contract limit changes (the username{2,15}to{2,30}bump is the example in the triage brief).This implements Decisions 3 and 4 and the Android item for
Throwable.kt:12indocs/specs/bugsnag-cleanup.md, using the per-field table in the triage brief ondocs/bugsnag-validation-triage.Violationnow carriesrulenext tofield;toValidationOrElsecopies it fromFieldViolation.ValidationExceptionisConditionallyNotifiable. It is notifiable (soWARNING) when a violation is onaddresses,addressoruser_id, which the request builders fill themselves.group.title,display_name, currencynameandusernameare unchanged and stay atINFO; those need UI guards or the contract bump, not a warning.ValidationException.groupingHashis the sorted, de-duplicated set of<field path with [N] removed>:<rule>.BugsnagErrorReportersets it as the event's grouping hash, whether the exception is the error or its cause. The message is not part of it.Tests:
ThrowableExtensionsTestcovers rule and field surviving into the exception,messages[3].contentandmessages[7].contentsharing a hash, a changed limit message not changing the hash, different rules or fields on one request giving different hashes, and the notifiable split between client-filled and user-input fields.BugsnagErrorReporterTestcovers the reporter picking the hash from the error or its cause and leaving other errors on default grouping.Not in this PR: the empty
NoId, the short token address caller, the UI input checks and the 0.24.0 contract bump. AWARNINGon client-filled fields will also report the next pin drift on those fields, which the triage brief expects.