8000 Improve user experience when API token fails to auth by technicalpickles · Pull Request #250 · Shopify/roast · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Improve user experience when API token fails to auth #250

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

technicalpickles
Copy link
Contributor

I gave an overview of this idea on #249

My thought is that Roast::CLI should rescue any errors that it can give more detailed explanation of what it means and/or how to fix. For example, when running Roast, and the API fails to authenticate, we shouldn't be showing a backtrace, but instead instructions on how to fix.

TODO:

  • is raising Thor::Error the best way to show this? Or should we just output to the terminal? should we exit explicitly, or handle as an exception?
  • should there be an option to always show the stacktrace? useful for development or running on head. I'm thinking like how rspec has a --backtrace to show everything
  • test/roast/workflow/workflow_initializer_test.rb is failing for the openrouter. seems like it has to do with needing to stub the right things

Previously, the error message is shown as a stack trace, which is a lot
extra information to process. In addition, it refers to an API
needing a token, but doesn't mention which one is expected and that it's
pulled from the environment.

This does two things:

- Update the CLI to rescue the authentication error, and re-raise as a
  Thor::Error. These are shown as text, and cause the CLI to exit with a
  non-zero exit code.

- Add more details about which API provider is expected and which
  environment variable is expected.
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.

1 participant
0