Skip to content

Revert Excon EOF error fingerprint updates - #16818

Open
v-abhishekbhaskar wants to merge 1 commit into
mainfrom
abhishekbhaskar/revert-excon-eof-fingerprint
Open

v-abhishekbhaskar wants to merge 1 commit into
mainfrom
abhishekbhaskar/revert-excon-eof-fingerprint

Conversation

@v-abhishekbhaskar

Copy link
Copy Markdown
Contributor

What are you trying to accomplish?

This removes the Excon EOF fingerprint added in #16167 (Dependabot::Sentry::ErrorFingerprint). Errors without sentry_context are now reported with no fingerprint again. This applies in Service#capture_exception, BaseCommand#handle_unknown_error, UpdateFilesCommand#handle_parser_error, UpdateGraphCommand and Updater::ErrorHandler.

Why: we're making Sentry grouping consistent. A fingerprint should only hold a CLI command template. The EOF fingerprint also included the package manager and the calling method, which doesn't fit that rule. Splitting errors by where they were raised will move to the dependabot-api rollup, which will use the top Dependabot backtrace frames.

Anything you want to highlight for special attention from reviewers?

  • This needs to ship together with, or just before, the dependabot-api rollup change. Otherwise Excon EOF errors get regrouped in Sentry twice.
  • The EOF specs now check that the fingerprint key is absent, because empty values are dropped from error_details.
  • I kept two test additions from Group Excon EOF errors by call site #16167 that still hold: "preserves an existing fingerprint" and the exact fingerprint assertion in error_handler_spec.

How will you know you've accomplished your goal?

  • Excon EOF errors are sent to record_update_job_unknown_error without a fingerprint (covered in service_spec, update_files_command_spec, update_graph_command_spec and error_handler_spec).
  • bundle exec rubocop on the changed files: passed, no offenses.
  • bundle exec srb tc: passed.
  • bundle exec rspec on the 4 updater specs (run on the host): 138 examples, 2 failures. The 2 failures (update_files_command_spec.rb:56, update_graph_command_spec.rb:59) also fail on the host without this change. I haven't run the specs in Docker yet.

Checklist

  • I have run the complete test suite to ensure all tests and linters pass.
  • I have thoroughly tested my code changes to ensure they work as expected, including adding additional tests for new functionality.
  • I have written clear and descriptive commit messages.
  • I have provided a detailed description of the changes in the pull request, including the problem it addresses, how it fixes the problem, and any relevant details about the implementation.
  • I have ensured that the code is well-documented and easy to understand.

@v-abhishekbhaskar v-abhishekbhaskar self-assigned this Nov 4, 2026
@v-abhishekbhaskar
v-abhishekbhaskar requested a review from a team as a code owner November 4, 2026 08:00
Copilot AI balanced review requested due to automatic review settings November 4, 2026 08:00

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The production Sentry grouping change requires human verification of the unavailable dependabot-api companion change and deployment order to avoid regrouping EOF errors twice.

No open issues 🎉

What changed in this PR

Reverts the Excon EOF fingerprinting introduced in #16167, preparing for call-site grouping to move to dependabot-api. Errors without sentry_context no longer receive a fallback fingerprint; existing custom fingerprints remain unchanged.

Changes:

  • Removes the EOF fingerprint helper and its dedicated specs.
  • Removes fallback fingerprint generation from five updater error-reporting paths.
  • Updates regression specs to assert that EOF error reports omit the fingerprint key.

Deployment should occur together with, or just before, the dependabot-api rollup change to avoid regrouping EOF errors twice.

File Description
updater/​spec/​dependabot/​updater/​error_handler_spec.rb Expects EOF error reports without a fingerprint.
updater/​spec/​dependabot/​update_graph_command_spec.rb Checks duplicate graph error reports omit the fingerprint.
updater/​spec/​dependabot/​update_files_command_spec.rb Checks duplicate file-update error reports omit the fingerprint.
updater/​spec/​dependabot/​service_spec.rb Verifies EOF socket errors receive no fingerprint.
updater/​spec/​dependabot/​sentry/​error_fingerprint_spec.rb Deletes specs for the removed helper.
updater/​lib/​dependabot/​updater/​error_handler.rb Returns nil when no existing fingerprint is available.
updater/​lib/​dependabot/​update_graph_command.rb Removes fallback fingerprint generation.
updater/​lib/​dependabot/​update_files_command.rb Uses only fingerprints supplied by sentry_context.
updater/​lib/​dependabot/​service.rb Removes the helper dependency and fallback fingerprint.
updater/​lib/​dependabot/​sentry/​error_fingerprint.rb Deletes the EOF call-site fingerprint helper.
updater/​lib/​dependabot/​base_command.rb Removes fallback fingerprinting for unknown errors.

Review effort: Balanced

Removes Dependabot::Sentry::ErrorFingerprint (added in #16167). Errors
without sentry_context are once again reported with no fingerprint.
Fingerprints are limited to CLI command templates; call-site grouping
will come from the dependabot-api rollup instead.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 3d452abb-cad5-4768-92e3-632ae273a163
@v-abhishekbhaskar
v-abhishekbhaskar force-pushed the abhishekbhaskar/revert-excon-eof-fingerprint branch from 699b82e to 6c9cff2 Compare November 6, 2026 19:32
Copilot AI balanced review requested due to automatic review settings November 6, 2026 19:32

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The companion dependabot-api change and rollout timing are unavailable, requiring human confirmation of deployment sequencing to avoid regrouping Excon EOF errors twice.

No open issues 🎉

Review effort: Balanced

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants