Group Excon EOF errors by call site - #16167
Conversation
There was a problem hiding this comment.
馃煛 Changes recommended
All unknown-error reporting paths must use the shared EOF fingerprint logic and receive coverage.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds call-site-aware grouping for EOF-backed Excon socket errors while preserving existing fingerprints.
Changes:
- Introduces shared EOF fingerprint generation.
- Integrates fingerprints into service and updater reporting.
- Adds focused regression coverage.
A moderate issue remains: several command-specific error-reporting paths bypass the shared fingerprint extraction, allowing duplicate ungrouped Sentry events.
File summaries
| File | Description |
|---|---|
updater/spec/dependabot/updater/error_handler_spec.rb |
Tests updater fingerprint integration and preservation. |
updater/spec/dependabot/service_spec.rb |
Tests service fingerprint handling. |
updater/spec/dependabot/sentry/error_fingerprint_spec.rb |
Tests EOF fingerprint generation. |
updater/lib/dependabot/updater/error_handler.rb |
Applies fingerprints to updater error details. |
updater/lib/dependabot/service.rb |
Applies fallback fingerprints, but some command-specific reporting paths remain uncovered. |
updater/lib/dependabot/sentry/error_fingerprint.rb |
Generates package-manager and call-site fingerprints. |
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 1
- Review effort level: Balanced
馃挕 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
馃數 Needs a closer look
Approval assessments are split, and the unresolved Sorbet strictness nit warrants human review.
Review details
Suppressed comments (1)
updater/spec/dependabot/sentry/error_fingerprint_spec.rb:1
- New files are required to use at least
# typed: strict;typed: falseleaves this new spec unchecked by Sorbet. Please raise the sigil and address any resulting type errors.
# typed: false
- Files reviewed: 11/11 changed files
- Comments generated: 0 new
- Review effort level: Balanced
What are you trying to accomplish?
Group
Excon::Error::Socketfailures backed byEOFErrorusing the package manager and first non-registry-client Dependabot call site. This prevents unrelated registry EOF failures from being collapsed into one Sentry issue while preserving existing custom fingerprints and grouping behavior for every other error.Anything you want to highlight for special attention from reviewers?
The new fingerprint is deliberately a fallback only for EOF-backed Excon socket errors. Existing
sentry_contextfingerprints retain their previous handling across service and command-specific error-reporting paths.How will you know you have accomplished your goal?
Focused updater coverage verifies the new grouping, existing fingerprint preservation, unchanged behavior for unrelated and non-EOF socket errors, and consistent fingerprints in command-specific unknown-error reports.
Validation:
bin/test --workdir updater bundler rspec spec/dependabot/sentry/error_fingerprint_spec.rb spec/dependabot/service_spec.rb spec/dependabot/updater/error_handler_spec.rb spec/dependabot/update_files_command_spec.rb spec/dependabot/update_graph_command_spec.rb(141 examples, 0 failures)Checklist