Skip to content

Conversation

@GaryZhous
Copy link
Contributor

@GaryZhous GaryZhous commented Jun 13, 2025

This pull request improves error handling in the build_session function of the goose-cli crate. Specifically, it replaces an unwrap call with a more robust error-handling mechanism to provide user-friendly error messages and guidance.

Error handling improvements:

  • Updated the build_session function in crates/goose-cli/src/session/builder.rs to replace the unwrap call with a match statement. This change ensures that errors during the creation of a provider are caught and handled gracefully. If an error occurs, a detailed error message is displayed, including troubleshooting steps and a link to relevant documentation, before the process exits with a non-zero status.

Details:

The error message displayed when no keyring is found is different from what the goose doc mentioned. It currently looked like this (Display thread 'main' panicked.... is kinda off to users I think, since this message is intended for developers and exposes internal file paths and implementation details that are confusing or irrelevant to end users.)
image
I made the system handle the error gracefully and display a user-friendly message with reference to the troubleshooting session to the goose doc.

output::render_error(&format!(
"Error {}.\n\
Please check your system keychain and run 'goose configure' again.\n\
If your system is unable to use the keyring, please try setting secret key(s) via environment variables.\n\
Copy link
Contributor

Choose a reason for hiding this comment

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

can we check the error here? the error may not be related to keyring

Copy link
Contributor Author

@GaryZhous GaryZhous Jun 13, 2025

Choose a reason for hiding this comment

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

If you go the goose doc link I attached in the PR description, you can see the error I got when I ran goose session without api key configured as an ent variable is the same as what they mentioned in the doc.
image
It is just that the actual error message is fuzzier than what it is supposed to be.

@yingjiehe-xyz yingjiehe-xyz merged commit 661446a into block:main Jun 13, 2025
6 checks passed
opdich added a commit to opdich/goose that referenced this pull request Jun 13, 2025
* upstream/main:
  Disable updater until we can debug more in release (block#2908)
  fix router trait error (block#2910)
  fix: Check for stderr error in receive() (block#2905)
  Damien/sagemaker tgi (block#2888)
  feat: (tool router) llm tool selector (block#2866)
  feat: (tool router) adds extension name in vector db & search tool (block#2855)
  Check for UPDATES_ENABLED flag before running update logic or in ui (block#2897)
  fix: handled the missing keyring error gracefully with a user-friendly message (block#2900)
  fix: handle JsonRPC error variants as responses (block#2903)
@GaryZhous GaryZhous deleted the GaryZ/Refine-Cli branch June 21, 2025 04:04
s-soroosh pushed a commit to s-soroosh/goose that referenced this pull request Jul 18, 2025
cbruyndoncx pushed a commit to cbruyndoncx/goose that referenced this pull request Jul 20, 2025
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