Skip to content
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

feat(agent): Handle OpenAI API key exceptions gracefully #6992

Conversation

kcze
Copy link
Contributor

@kcze kcze commented Mar 8, 2024

OPEN-337, OPEN-305

  • Better handle no API keys or invalid ones
  • Handle exception and exit when invalid key is provided
  • Handle any APIError exception when trying to get OpenAI models and exit

Copy link

netlify bot commented Mar 8, 2024

Deploy Preview for auto-gpt-docs canceled.

Name Link
🔨 Latest commit 357e3cd
🔍 Latest deploy log https://app.netlify.com/sites/auto-gpt-docs/deploys/65fd5c8bad067c000822f57b

@kcze kcze changed the title OPEN-337: Handle openai API key exceptions feat(autogpt): Handle OpenAI API key exceptions gracefully Mar 9, 2024
autogpts/autogpt/autogpt/config/config.py Outdated Show resolved Hide resolved
Comment on lines 330 to 337
logger.info(
"OpenAI API key successfully set!",
extra={"color": Fore.GREEN},
)
logger.info(
"NOTE: The API key you've set is only temporary. "
"For longer sessions, please set it in .env file",
extra={"color": Fore.YELLOW},
Copy link
Member

Choose a reason for hiding this comment

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

Why print -> logger.info?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you mean only those two infos or the other logger.* in the same file as well? I'm not sure when to use print or logging, why would I use print if we have logging?

Copy link
Member

Choose a reason for hiding this comment

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

Logging is the application reporting what's going on. If a user wants less info, they can raise the log level and e.g. only get WARNING and higher urgency messages.

print() is direct terminal output to interface with the user. This is part of the UI, functionally different from the logging output.

Copy link
Contributor Author

@kcze kcze Mar 18, 2024

Choose a reason for hiding this comment

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

Thanks, I replaced this just below as well - I think that's part of UI (prints after user inputted wrong key).

else:
    print(f"{Fore.RED}Invalid OpenAI API key{Fore.RESET}")

@kcze kcze requested a review from Pwuts March 18, 2024 15:26
@Pwuts
Copy link
Member

Pwuts commented Mar 21, 2024

This PR needs updating because clean_input is no longer async (since 632686c). I'll do that myself if you don't get to it.

@github-actions github-actions bot added the conflicts Automatically applied to PRs with merge conflicts label Mar 21, 2024
Copy link
Contributor

This pull request has conflicts with the base branch, please resolve those so we can evaluate the pull request.

@github-actions github-actions bot removed the conflicts Automatically applied to PRs with merge conflicts label Mar 21, 2024
Copy link
Contributor

Conflicts have been resolved! 🎉 A maintainer will review the pull request shortly.

@kcze
Copy link
Contributor Author

kcze commented Mar 21, 2024

This PR needs updating because clean_input is no longer async (since 632686c). I'll do that myself if you don't get to it.

Done

Copy link

codecov bot commented Mar 22, 2024

Codecov Report

Attention: Patch coverage is 25.92593% with 20 lines in your changes are missing coverage. Please review.

Project coverage is 45.40%. Comparing base (dde0c70) to head (357e3cd).
Report is 1 commits behind head on master.

Files Patch % Lines
autogpts/autogpt/autogpt/config/config.py 16.66% 15 Missing ⚠️
autogpts/autogpt/autogpt/llm/api_manager.py 57.14% 3 Missing ⚠️
autogpts/autogpt/autogpt/app/main.py 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6992      +/-   ##
==========================================
- Coverage   45.54%   45.40%   -0.15%     
==========================================
  Files         139      139              
  Lines        6514     6530      +16     
  Branches      915      917       +2     
==========================================
- Hits         2967     2965       -2     
- Misses       3397     3413      +16     
- Partials      150      152       +2     
Flag Coverage Δ
Linux 45.40% <25.92%> (-0.04%) ⬇️
Windows ?
autogpt-agent 45.40% <25.92%> (-0.12%) ⬇️
macOS ?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@kcze kcze merged commit 028d2c3 into Significant-Gravitas:master Mar 22, 2024
12 of 18 checks passed
@kcze kcze changed the title feat(autogpt): Handle OpenAI API key exceptions gracefully feat(agent): Handle OpenAI API key exceptions gracefully Mar 22, 2024
@kcze kcze deleted the kpczerwinski/open-337-authenticationerror-incorrect-api-key branch March 22, 2024 12:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants