Repository navigation
ref(cli): Parse arity-dependent arguments in the scanner - #1714
Open
sentry-junior[bot] wants to merge 2 commits into
Open
sentry-junior[bot] wants to merge 2 commits into
sentry-junior[bot] wants to merge 2 commits into
Conversation
Follow-up to #1655 for #1564. Positionals whose meaning depends on how many arguments were given (`[<org>/<project>] <replay-id>`, `log list`'s trailing trace ID) kept parsing in `func`, because a per-element Stricli `parse` can't see the count. Our `@stricli/core` patch now lets an array positional declare `combine`: it gets every parsed element and returns the one value the command receives. A throw becomes an `ArgumentParseError`, so these commands fail in the scanner with exit 252 like other arguments. `boundArguments` (`src/lib/positional-parse.ts`) builds such a positional: a pure `parse` over all the arguments, bound to a `resolve` that runs after the auth guard, like `boundPositional`. `log list`, `replay view` and `replay download` move onto it. Parsing a pasted Sentry URL no longer applies its server; the parse returns `baseUrl` (and trace-target warnings), and `resolve` applies them. `parseTraceTargetInput` and `parseDualModeInput` are the side-effect-free trace parsers; the existing ones wrap them, so span and trace commands are unchanged and can migrate onto the same primitive next. A parse error next to a `help` argument is deferred to `func`, so `sentry replay view help` still shows the command's help. Plural alias interception drops the first argument and parses the rest again. Co-Authored-By: BYK <burak.kaya@sentry.io>
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 07f92a3. Configure here.
Addresses Bugbot on #1714. `buildListCommand` dropped a subcommand name (`sentry log list view …`) in `func` and parsed the other arguments again, after the scanner had already parsed all of them: - `log list view acme/web <trace-id>` failed in the scanner (exit 252), because "acme/web" was read as the trace ID, so the intercept never ran - a parse error deferred next to `help` in the second parse was never thrown, so `func` read an undefined parse `skipLeadingArgument` now wraps a `boundArguments` positional so the name is left out in the scanner, before it changes what the other arguments mean. `func` only prints the hint.
This branch was previously deployed
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.
Follow-up to #1655 for #1564. Some positionals mean different things depending on how many arguments were given, like
[<org>/<project>] <replay-id>orlog list's optional trailing trace ID. Those kept parsing infuncbecause a per-element Stricliparseonly sees one argument and can't count the rest.The new parser type. Our
@stricli/corepatch now lets an array positional declarecombine. It receives every parsed element and returns the one value the command gets. If it throws, the error becomes anArgumentParseError. Arity-dependent targets therefore fail in the scanner with exit 252, like every other argument. Whencombineis unset the patch does nothing, andcheck:patcheschecks that it is still present.On top of the patch,
boundArguments(src/lib/positional-parse.ts) builds the positional. It pairs a pureparseover all the arguments with aresolvethat runs after the auth guard, the same contract asboundPositional.functakes oneBoundArguments<Parsed, Resolved>, reads.parsedand awaits.resolved. Each element still completes as its completion kind.Migrated:
log list,replay viewandreplay download.baseUrland any trace-target warnings, andresolveapplies them.parseTraceTargetInputandparseDualModeInput(trace-target.ts) are the side-effect-free trace parsers.parseTraceTargetandparseDualModeArgsnow wrap them, so span and trace commands don't change in this PR.replay downloadgets org/project completion, whichreplay viewalready had.Kept behavior:
help, the parse error is deferred tofunc, sosentry replay view helpstill prints the command's help.buildListCommand's subcommand-name interception (sentry log list view …) drops the first argument and parses the rest again.Next: issue and dashboard targets, plus
trace view/logs,span list/viewandlog view, fit the same[<org>/[<project>/]]<id>shape. They can each move ontoboundArgumentsin follow-ups. The list-target positional for dispatch-based lists is in #1711, kept separate from this PR.Verification (Node 24):
tsc,lintandcheck:patchespass.delta-upgradestable patch) and fail onmaintoo.replay view help,replay view acme/web 0123(exit 252),replay download a b c(exit 252), andlog list view <trace-id>(shows the interception tip, then parses the trace ID alone).via BYK.
--
View Junior Session in Sentry