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

Filter errors from environment information output #857

Merged
merged 1 commit into from
Nov 13, 2024

Conversation

pelikhan
Copy link
Member

@pelikhan pelikhan commented Nov 13, 2024

Introduce an option to display errors when showing environment information, enhancing the command-line interface for better error visibility.


  • 🚀 Added a new option -e, --error to the CLI command for showing .env information, allowing users to display errors encountered during the operation.
  • 🛠 Updated the envInfo function to handle the new error option, capturing and storing any errors related to provider information.
  • 🔄 Refactored some logic in the parseTokenFromEnv function by removing redundant code for improved readability.
  • 🗂️ Reorganized the MODEL_PROVIDERS constant list to include “Anthropic models” and “Hugging Face models” in a more consistent order.
  • 📜 Improved code formatting, including breaking long lines and ensuring options can be passed to show errors along with tokens, enhancing debug and configuration capabilities.

generated by pr-describe

}
if (!token && !/^http:\/\//i.test(base))
// localhost typically requires no key
throw new Error("OPENAI_API_KEY missing")

Choose a reason for hiding this comment

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

The code throws an error if the OPENAI_API_KEY is missing, but it doesn't check if the base is a valid URL before this. This could lead to an error being thrown for the wrong reason. Consider checking the validity of base before throwing an error for a missing API key. 🔑

generated by pr-review-commit missing_key_check

@@ -237,16 +248,6 @@ export const MODEL_PROVIDERS = Object.freeze([
detail: "LiteLLM proxy",
url: DOCS_CONFIGURATION_LITELLM_URL,
},

Choose a reason for hiding this comment

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

The MODEL_PROVIDERS array contains duplicate entries for MODEL_PROVIDER_ANTHROPIC and MODEL_PROVIDER_HUGGINGFACE. This could lead to confusion or unexpected behavior when iterating over providers. Please ensure each provider is listed only once. 🔄

generated by pr-review-commit duplicate_model_providers

res.providers.push({
provider: modelProvider.id,
error: errorMessage(e),
})

Choose a reason for hiding this comment

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

The error handling logic inside the envInfo function only pushes errors to the result if the error option is set. This might lead to silent failures if the option is not set. Consider logging or handling errors in a way that ensures they are not completely ignored. 🚨

generated by pr-review-commit error_handling

Copy link

The changes in the GIT_DIFF include:

  1. CLI Enhancements: A new command-line option --error has been added to show errors in the .env information display feature.

  2. Refactoring in info.ts: The envInfo function has been updated to accommodate the new error option, allowing it to capture and store errors when encountered.

  3. Code Cleanup in connection.ts: Redundant if-statements were removed to streamline the parseTokenFromEnv function, making the code cleaner and more concise.

  4. Constants Update: New entries for Anthropic and Hugging Face models have been added to the MODEL_PROVIDERS array, rearranging the structure for better organization.

These changes enhance the functionality by providing more control over CLI options, improve code readability, and ensure the system supports additional model providers.

LGTM 🚀

generated by pr-review

@pelikhan pelikhan merged commit 2c225ed into main Nov 13, 2024
11 checks passed
@pelikhan pelikhan deleted the openai-connection-error branch November 13, 2024 19:44
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