Skip to content

Throw UserError instead of Traceback error, when api-key is empty/incorrect #209

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.

Sign up for GitHub

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

Open
moctarjallo wants to merge 1 commit into
base: 18.0
Choose a base branch
from
Open

Throw UserError instead of Traceback error, when api-key is empty/incorrect #209

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
105 changes: 67 additions & 38 deletions llm/models/llm_provider.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,10 @@ def list_models(self, model_id=None):
def action_fetch_models(self):
"""Fetch models from provider and open import wizard"""
self.ensure_one()

# Check if API key is configured
if not self.api_key:
Copy link
Contributor

@adar2378 adar2378 Dec 8, 2025

Choose a reason for hiding this comment

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

Not all provider requires api key, Ollama for example if you want to run locally may not need API key.

raise UserError(_("API key is required to fetch models. Please configure the API key for this provider."))

# Create wizard first so it has an ID
wizard = self.env["llm.fetch.models.wizard"].create(
Expand All @@ -192,50 +196,75 @@ def action_fetch_models(self):
for model in self.env["llm.model"].search([("provider_id", "=", self.id)])
}

# Fetch models from provider
# Fetch models from provider with error handling for incorrect API keys
model_to_fetch = self._context.get("default_model_to_fetch")
if model_to_fetch:
models_data = self.list_models(model_id=model_to_fetch)
else:
models_data = self.list_models()

# Track models to prevent duplicates
wizard_models = set()
lines_to_create = []

for model_data in models_data:
details = model_data.get("details", {})
name = model_data.get("name") or details.get("id")

if not name:
continue

# Skip duplicates
if name in wizard_models:
continue
wizard_models.add(name)

# Determine model use and capabilities
capabilities = details.get("capabilities", ["chat"])
model_use = self._determine_model_use(name, capabilities)

# Check against existing models
existing = existing_models.get(name)
status = "new"
if existing:
status = "modified" if existing.details != details else "existing"

lines_to_create.append(
{
"wizard_id": wizard.id,
"name": name,
"model_use": model_use,
"status": status,
"details": details,
"existing_model_id": existing.id if existing else False,
"selected": status in ["new", "modified"],
}
)
try:
if model_to_fetch:
models_data = self.list_models(model_id=model_to_fetch)
else:
models_data = self.list_models()

# Iterate over the generator (this is where authentication errors occur)
for model_data in models_data:
details = model_data.get("details", {})
name = model_data.get("name") or details.get("id")

if not name:
continue

# Skip duplicates
if name in wizard_models:
continue
wizard_models.add(name)

# Determine model use and capabilities
capabilities = details.get("capabilities", ["chat"])
model_use = self._determine_model_use(name, capabilities)

# Check against existing models
existing = existing_models.get(name)
status = "new"
if existing:
status = "modified" if existing.details != details else "existing"

lines_to_create.append(
{
"wizard_id": wizard.id,
"name": name,
"model_use": model_use,
"status": status,
"details": details,
"existing_model_id": existing.id if existing else False,
"selected": status in ["new", "modified"],
}
)

except Exception as e:
# Check for authentication errors (incorrect API key)
error_msg = str(e)
if any(keyword in error_msg.lower() for keyword in [
'incorrect api key',
'invalid_api_key',
'authentication',
'401',
'unauthorized',
'invalid api key'
]):
raise UserError(_(
"Authentication failed. The provided API key appears to be incorrect. "
"Please verify your API key and try again.\n\n"
"Error details: %s"
) % error_msg.split('\n')[0]) # Show only the first line for brevity
# Re-raise other exceptions with a user-friendly message
raise UserError(_(
Copy link
Contributor

@adar2378 adar2378 Dec 8, 2025

Choose a reason for hiding this comment

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

We should not probably mask all exceptions under one UserError.. it would be better if we let them propagate and handle provider specific errors on provider level rather than here.

Copy link
Author

@moctarjallo moctarjallo Dec 8, 2025

Choose a reason for hiding this comment

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

@adar2378 if llm_ollama is the only module that breaks, then maybe we could detect if this provider is ollama or not ?

the reason why i wanted to throw a UserError is because not everyone has the reflex to read entirely a traceback so not really knowing what's the issue (some may think that the module just does not work because a traceback is a too generic error..)

Copy link
Contributor

@adar2378 adar2378 Dec 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

@moctarjallo I think you are right about user experience. But still, it should be handled on the provider-specific part, like in llm_openai or llm_ollama etc, modules and not here. Because it can hide bugs under the carpet of UserError.

For example, we could do something like this in llm_openai:

# In llm_openai provider
  from openai import AuthenticationError, APIConnectionError, RateLimitError

  def openai_models(self, model_id=None):
      try:
          # ... fetch models
      except AuthenticationError as e:
          raise UserError(_("Invalid API key. Please check your OpenAI API key 
  configuration.\n\nDetails: %s") % e)
      except APIConnectionError as e:
          raise UserError(_("Could not connect to OpenAI API. Please check your network 
  connection.\n\nDetails: %s") % e)
      except RateLimitError as e:
          raise UserError(_("OpenAI rate limit exceeded. Please try again 
  later.\n\nDetails: %s") % e)

"Failed to fetch models from the provider. Error: %s\n\n"
"Please check your API configuration and network connection."
) % error_msg)

# Create all lines
if lines_to_create:
Expand Down