sentry: Log the Nix command / subcommand#436
Conversation
📝 WalkthroughWalkthroughSentry initialization now sets a Changes
Sequence Diagram(s)(No sequence diagram generated — change is a localized reorder and tagging in a single source file.) Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
7b7b077 to
2e25546
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/nix/main.cc`:
- Line 417: The sentry_set_tag call is tagging nix_command with argv[0] which
can be a full path and leak user-specific data; update the code in main (where
sentry_set_tag is called) to normalize argv[0] to the executable basename before
tagging (e.g., use std::filesystem::path(argc>0?argv[0]:"").filename().string()
or an equivalent basename/libgen approach) and pass that basename string into
sentry_set_tag instead of the raw argv[0].
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Note: this only include the command/subcommand (e.g. `nix flake update`), but no other command-line arguments.
2e25546 to
6196dc2
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/nix/main.cc (1)
573-581: Minor: Redundant null check forcommand.The condition
command &&on line 576 is redundant since thewhile (command)loop condition on line 575 already guaranteescommandis non-null when entering the loop body.Proposed simplification
std::vector<std::string> subcommand; MultiCommand * command = &args; while (command) { - if (command && command->command) { + if (command->command) { subcommand.push_back(command->command->first); command = dynamic_cast<MultiCommand *>(&*command->command->second); } else break; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/nix/main.cc` around lines 573 - 581, The if condition inside the while loop redundantly checks `command` even though the loop `while (command)` already ensures it's non-null; update the loop body in function using `MultiCommand * command = &args` to remove the redundant `command &&` and just test `if (command->command)` (or equivalently `if (command->command) { ... } else break;`) when pushing into `subcommand` and performing the `dynamic_cast` to the next `MultiCommand`.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/nix/main.cc`:
- Around line 573-581: The if condition inside the while loop redundantly checks
`command` even though the loop `while (command)` already ensures it's non-null;
update the loop body in function using `MultiCommand * command = &args` to
remove the redundant `command &&` and just test `if (command->command)` (or
equivalently `if (command->command) { ... } else break;`) when pushing into
`subcommand` and performing the `dynamic_cast` to the next `MultiCommand`.
Motivation
Improve Sentry crash reports.
Note: this only include the command/subcommand (e.g.
nix flake update), but no other command-line arguments.Context
Summary by CodeRabbit