refactor(cli): collapse provider-null check into Providers.require helper#169
Merged
Conversation
β¦lper Every CLI command that needed a typed provider repeated the same 5-line block: lookup β null check β print error.provider.none β return. Each copy returned with exit 0, so a missing provider silently looked like success to scripts piping argus output. Introduce a single Providers.require(provider, pid, messages) static helper in the command package. It prints the localized error and throws CommandExitException(1), which ArgusCli already converts to a non-zero exit. 32 call sites across 31 command files shrink from 5 lines to 1, and missing-provider runs now correctly exit 1. Sites that don't fit the shape are left alone: - ClassLeak/CompilerQueue/Events/GcRunCommand check JcmdExecutor availability directly, not a typed provider. - ProfileCommand's parallel fan-out callable wraps the message into a ProfileResult.error rather than printing. - PsCommand uses "ps" instead of a pid. Behaviour change: CLI commands that previously printed the missing- provider message and exited 0 now exit 1. Signed-off-by: rlaope <piyrw9754@gmail.com>
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.
Summary
Every CLI command that resolves a typed provider repeated the same 5-line block:
The block returned with exit 0, so missing-provider runs silently looked like success to scripts piping argus output.
This PR introduces a single
Providers.require(provider, pid, messages)static helper in the command package. It prints the localized error and throwsCommandExitException(1), whichArgusClialready converts to a non-zero exit. 32 call sites across 31 command files shrink from 5 lines to 1, and missing-provider runs now correctly exit 1.argus-cli/src/main/java/io/argus/cli/command/Providers.java(~30 LOC, single static helper)Providers.require(...)ProvidersTestcovers both branches (non-null pass-through, null β throwsCommandExitException(1)+ writes to stderr)Out of scope
Sites that don't fit the helper shape are left alone:
ClassLeak/CompilerQueue/Events/GcRunCommandβ checkJcmdExecutor.isJcmdAvailable()directly, not a typed provider.ProfileCommand's parallel fan-out callable wraps the message into aProfileResult.errorrather than printing/exiting.PsCommandpasses"ps"instead of a pid.Behaviour change
CLI commands that previously printed
error.provider.noneto stderr and exited 0 now exit 1. This is the correct behaviour for shell pipelines and CI gates β silent exit-0 on a missing provider was a latent bug.Test plan
:argus-cli:testpasses (incl. newProvidersTest)./gradlew build(excludingintegrationTest) passesargus gc 999999exits 1 with stderr message