Skip to content

fix(golang-rewrite): print error message when app.Run fails#2228

Closed
teal-bauer wants to merge 3 commits into
asdf-vm:masterfrom
teal-bauer:fix/print-cli-error
Closed

fix(golang-rewrite): print error message when app.Run fails#2228
teal-bauer wants to merge 3 commits into
asdf-vm:masterfrom
teal-bauer:fix/print-cli-error

Conversation

@teal-bauer

Copy link
Copy Markdown

Summary

  • Print CLI error messages to stderr before exiting with error code

Previously, when app.Run() returned an error (e.g., asdf set -h foo 1.2.3), the CLI would exit with code 1 but print nothing, making it difficult to understand what went wrong.

Now the error message is printed (e.g., "No help topic for 'foo'").

Test plan

  • Verify asdf set -h foo 1.2.3 now prints error message
  • Verify asdf set -h still shows help with exit code 0
  • Verify normal asdf set <tool> <version> still works
  • Run existing CLI tests (go test ./internal/cli/...)

@teal-bauer teal-bauer requested a review from a team as a code owner January 5, 2026 13:27
@teal-bauer

Copy link
Copy Markdown
Author

I cannot count the number of times I have accidentally run asdf set -h foo 1.2.3 for home directory, where it would not show ANY error output, and then wonder why things didn't work, until at some point I remember to use -u instead. But I also always run asdf set --help, think "huh, but -h should've shown help but didn't, so maybe the -h did work...?"...

@teal-bauer teal-bauer changed the title fix(cli): print error message when app.Run fails fix(golang-rewrite): print error message when app.Run fails Jan 5, 2026
@teal-bauer

Copy link
Copy Markdown
Author

Ping @asdf-vm/core?

@Stratus3D Stratus3D left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hi @teal-bauer , thanks for the PR, unfortunately some of the existing tests are failing with these changes. It appears multiple errors are getting logged in some instances. I'll see if I can figure out what is going wrong.

@teal-bauer

Copy link
Copy Markdown
Author

Hi @Stratus3D, the local tests were green when I opened this PR in January, I think CI tests didn't run due to missing approval. Ping me if the forward merge doesn't resolve it - I see the workflows are still waiting for approval though.

@Stratus3D

Copy link
Copy Markdown
Member

Thanks for the PR @teal-bauer ! This was very confusing behavior and I'm glad you reported this. When investigating this I found that -h was being interpeted by urfave/cli as a global help flag, hence the odd output. I think disabling the global help flag is the right approach. I'm closing this PR in favor of that approach. #2294

@Stratus3D Stratus3D closed this Jul 8, 2026
@teal-bauer

Copy link
Copy Markdown
Author

Ah, interesting. Yeah, either way is fine with me, and having -h print the help is probably useful. I think semantically it would still make sense to print the err here if it's not nil, but I didn't encounter a specific issue other than not knowing that -h didn't take.

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.

2 participants