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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/src/content/docs/reference/cli/commands.md
Original file line number Diff line number Diff line change
Expand Up @@ -492,5 +492,6 @@ Show .env information

Options:
-t, --token show token
-e, --error show errors
-h, --help display help for command
```
1 change: 1 addition & 0 deletions packages/cli/src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,7 @@ export async function cli() {
.description("Show .env information")
.arguments("[provider]")
.option("-t, --token", "show token")
.option("-e, --error", "show errors")
.action(envInfo) // Action to show environment information
program.parse() // Parse command-line arguments
}
18 changes: 11 additions & 7 deletions packages/cli/src/info.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,11 @@
* @param provider - The specific provider to filter by (optional).
* @param options - Configuration options, including whether to show tokens.
*/
export async function envInfo(provider: string, options?: { token?: boolean }) {
const { token } = options || {}
export async function envInfo(
provider: string,
options?: { token?: boolean; error?: boolean }
) {
const { token, error } = options || {}
const res: any = {}
res[".env"] = host.dotEnvPath ?? ""
res.providers = []
Expand All @@ -52,11 +55,12 @@
res.providers.push(conn)
}
} catch (e) {
// Capture and store any errors encountered
res.providers.push({
provider: modelProvider.id,
error: errorMessage(e),
})
if (error)
// Capture and store any errors encountered
res.providers.push({
provider: modelProvider.id,
error: errorMessage(e),
})

Check failure on line 63 in packages/cli/src/info.ts

View workflow job for this annotation

GitHub Actions / build

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. 🚨

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

}
}
console.log(YAMLStringify(res))
Expand Down
84 changes: 38 additions & 46 deletions packages/core/src/connection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,54 +87,46 @@
const BASE_SUFFIX = ["_API_BASE", "_API_ENDPOINT", "_BASE", "_ENDPOINT"]

if (provider === MODEL_PROVIDER_OPENAI) {
if (env.OPENAI_API_KEY || env.OPENAI_API_BASE || env.OPENAI_API_TYPE) {
const token = env.OPENAI_API_KEY ?? ""
let base = env.OPENAI_API_BASE
let type = (env.OPENAI_API_TYPE as OpenAIAPIType) || "openai"
const version = env.OPENAI_API_VERSION
if (
type !== "azure" &&
type !== "openai" &&
type !== "localai" &&
type !== "azure_serverless" &&
type !== "azure_serverless_models"
const token = env.OPENAI_API_KEY ?? ""
let base = env.OPENAI_API_BASE
let type = (env.OPENAI_API_TYPE as OpenAIAPIType) || "openai"
const version = env.OPENAI_API_VERSION
if (
type !== "azure" &&
type !== "openai" &&
type !== "localai" &&
type !== "azure_serverless" &&
type !== "azure_serverless_models"
)
throw new Error(
"OPENAI_API_TYPE must be 'azure', 'azure_serverless', 'azure_serverless_models' or 'openai' or 'localai'"
)
throw new Error(
"OPENAI_API_TYPE must be 'azure', 'azure_serverless', 'azure_serverless_models' or 'openai' or 'localai'"
)
if (type === "openai" && !base) base = OPENAI_API_BASE
if (type === "localai" && !base) base = LOCALAI_API_BASE
if ((type === "azure" || type === "azure_serverless") && !base)
throw new Error(
"OPENAI_API_BASE must be set when type is 'azure'"
)
if (type === "azure") base = cleanAzureBase(base)
if (
type === "azure" &&
version &&
version !== AZURE_OPENAI_API_VERSION
if (type === "openai" && !base) base = OPENAI_API_BASE
if (type === "localai" && !base) base = LOCALAI_API_BASE
if ((type === "azure" || type === "azure_serverless") && !base)
throw new Error("OPENAI_API_BASE must be set when type is 'azure'")
if (type === "azure") base = cleanAzureBase(base)
if (type === "azure" && version && version !== AZURE_OPENAI_API_VERSION)
throw new Error(
`OPENAI_API_VERSION must be '${AZURE_OPENAI_API_VERSION}'`
)
throw new Error(
`OPENAI_API_VERSION must be '${AZURE_OPENAI_API_VERSION}'`
)
if (!token && !/^http:\/\//i.test(base))
// localhost typically requires no key
throw new Error("OPENAI_API_KEY missing")
if (token === PLACEHOLDER_API_KEY)
throw new Error("OPENAI_API_KEY not configured")
if (base === PLACEHOLDER_API_BASE)
throw new Error("OPENAI_API_BASE not configured")
if (base && !URL.canParse(base))
throw new Error("OPENAI_API_BASE must be a valid URL")
return {
provider,
model,
base,
type,
token,
source: "env: OPENAI_API_...",
version,
}
if (!token && !/^http:\/\//i.test(base))
// localhost typically requires no key
throw new Error("OPENAI_API_KEY missing")

Check failure on line 115 in packages/core/src/connection.ts

View workflow job for this annotation

GitHub Actions / build

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. 🔑

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

if (token === PLACEHOLDER_API_KEY)
throw new Error("OPENAI_API_KEY not configured")
if (base === PLACEHOLDER_API_BASE)
throw new Error("OPENAI_API_BASE not configured")
if (base && !URL.canParse(base))
throw new Error("OPENAI_API_BASE must be a valid URL")
return {
provider,
model,
base,
type,
token,
source: "env: OPENAI_API_...",
version,
}
}

Expand Down
26 changes: 14 additions & 12 deletions packages/core/src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,8 @@
"https://microsoft.github.io/genaiscript/getting-started/configuration/#huggingface"
export const DOCS_CONFIGURATION_CONTENT_SAFETY_URL =
"https://microsoft.github.io/genaiscript/reference/scripts/content-safety"
export const DOCS_DEF_FILES_IS_EMPTY_URL = "https://microsoft.github.io/genaiscript/reference/scripts/context/#empty-files"
export const DOCS_DEF_FILES_IS_EMPTY_URL =
"https://microsoft.github.io/genaiscript/reference/scripts/context/#empty-files"

export const MODEL_PROVIDERS = Object.freeze([
{
Expand Down Expand Up @@ -222,31 +223,31 @@
detail: "Azure AI Models (serverless deployments, not OpenAI)",
url: DOCS_CONFIGURATION_AZURE_MODELS_SERVERLESS_URL,
},
{
id: MODEL_PROVIDER_ANTHROPIC,
detail: "Anthropic models",
url: DOCS_CONFIGURATION_ANTHROPIC_URL,
},
{
id: MODEL_PROVIDER_HUGGINGFACE,
detail: "Hugging Face models",
url: DOCS_CONFIGURATION_HUGGINGFACE_URL,
},
{
id: MODEL_PROVIDER_OLLAMA,
detail: "Ollama local model",
url: DOCS_CONFIGURATION_OLLAMA_URL,
},
{
id: MODEL_PROVIDER_LLAMAFILE,
detail: "llamafile.ai local model",
url: DOCS_CONFIGURATION_LLAMAFILE_URL,
},
{
id: MODEL_PROVIDER_LITELLM,
detail: "LiteLLM proxy",
url: DOCS_CONFIGURATION_LITELLM_URL,
},

Check failure on line 250 in packages/core/src/constants.ts

View workflow job for this annotation

GitHub Actions / build

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. 🔄

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

{
id: MODEL_PROVIDER_ANTHROPIC,
detail: "Anthropic models",
url: DOCS_CONFIGURATION_ANTHROPIC_URL,
},
{
id: MODEL_PROVIDER_HUGGINGFACE,
detail: "Hugging Face models",
url: DOCS_CONFIGURATION_HUGGINGFACE_URL,
},
])

export const NEW_SCRIPT_TEMPLATE = `$\`Write a short poem in code.\`
Expand Down Expand Up @@ -335,4 +336,5 @@

export const CHOICE_LOGIT_BIAS = 5

export const SANITIZED_PROMPT_INJECTION = "...prompt injection detected, content removed..."
export const SANITIZED_PROMPT_INJECTION =
"...prompt injection detected, content removed..."