Repository navigation
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves observability for recurring Cargo sparse-index transport failures when downloading from the public index.crates.io sparse index by attaching a small, sanitized set of structured fields to HelperSubprocessFailed error context (without adding structured context for arbitrary/private registries).
Changes:
- Add parsing logic in the Cargo lockfile updater to detect
index.crates.iosparse-index download failures and extract safe structured fields (host, index path, download path, package name, curl code/message, protocol when known). - Sanitize curl error messages before attaching them to structured context (dropping messages that look like they might contain URLs or userinfo).
- Add specs covering (1) the crates.io sparse-index transport failure case and (2) an arbitrary registry failure case to ensure no credential-like registry details are added to
error_context.
Show a summary per file
| File | Description |
|---|---|
cargo/lib/dependabot/cargo/file_updater/lockfile_updater.rb |
Adds crates.io sparse-index error-context extraction and sanitized curl error message handling, merged into existing helper subprocess failure context. |
cargo/spec/dependabot/cargo/file_updater/lockfile_updater_spec.rb |
Adds regression coverage ensuring crates.io failures get structured context and arbitrary registry failures do not. |
Copilot's findings
- Files reviewed: 2/2 changed files
- Comments generated: 0
Contributor
Author
|
Fixed through dependabot/proxy#138 |
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.
What are you trying to accomplish?
Improve observability for recurring Cargo sparse-index download failures from crates.io, especially transport errors such as
Weird server reply (Invalid status line). The goal is to add enough sanitized structured context to distinguish whether these failures are coming from Cargo/libcurl, crates.io/CDN, Dependabot proxying, or surrounding infrastructure, without adding sensitive registry data to Sentry context.Related public issues: #14743, #14540
Anything you want to highlight for special attention from reviewers?
The added context is intentionally narrow: it is only populated for recognized public
index.crates.iosparse-index download failures. Arbitrary registry URLs, credentials, Cargo config contents, environment values, and raw Cargo output are not copied into the new structured context.This keeps the existing Cargo subprocess failure flow intact while adding safe fields that should make the recurring failure mode easier to group and investigate.
How will you know you've accomplished your goal?
The Cargo lockfile updater spec covers both the positive crates.io sparse-index case and a private/arbitrary registry failure case to ensure sensitive registry details are not added to the structured context.
Validation run:
bundle exec rspec spec/dependabot/cargo/file_updater/lockfile_updater_spec.rbbundle exec rubocop lib/dependabot/cargo/file_updater/lockfile_updater.rb spec/dependabot/cargo/file_updater/lockfile_updater_spec.rbChecklist