diff --git a/src/praisonai-agents/praisonaiagents/agent/agent.py b/src/praisonai-agents/praisonaiagents/agent/agent.py index 122dad4c61..74f4fd8405 100644 --- a/src/praisonai-agents/praisonaiagents/agent/agent.py +++ b/src/praisonai-agents/praisonaiagents/agent/agent.py @@ -2449,7 +2449,7 @@ def _any_output_mode_enabled(): self.tools = tools # Otherwise, fall back to OpenAI environment/name (cached for performance) else: - model_name = llm or Agent._get_default_model() + model_name = Agent._get_default_model() if llm is None else llm # Pointing at a non-OpenAI endpoint without naming a model leaves the # OpenAI default in place, so the endpoint is asked for a model it # has never heard of -- Ollama answers 404 "model 'gpt-4o-mini' not diff --git a/src/praisonai-agents/praisonaiagents/agent/chat_mixin.py b/src/praisonai-agents/praisonaiagents/agent/chat_mixin.py index b378ba2fc6..0a7f6a61d8 100644 --- a/src/praisonai-agents/praisonaiagents/agent/chat_mixin.py +++ b/src/praisonai-agents/praisonaiagents/agent/chat_mixin.py @@ -13,7 +13,13 @@ import threading import logging from praisonaiagents._logging import get_logger -from ..errors import BudgetExceededError, ToolExecutionError +from ..errors import ( + AgentErrorKind, + BudgetExceededError, + LLMError, + ToolExecutionError, + sanitize_llm_error_message, +) from .tool_execution import _active_turn_cancel # Shared lazy display helpers (cached, thread-safe; avoid circular imports) @@ -44,6 +50,58 @@ def _exit_turn(agent, ctx_token): _active_turn_cancel.set(None) +_FATAL_LLM_ERROR_KINDS = ("model_not_found", "format_error", "auth", "auth_permanent") + + +def _is_fatal_llm_configuration_error(exc: LLMError) -> bool: + """Return True when ``exc`` represents an invalid model, malformed request, or auth failure + that must propagate to callers rather than being converted into ``None`` (#5956).""" + return not exc.is_retryable and exc.error_category in _FATAL_LLM_ERROR_KINDS + + +def _resolve_agent_error_kind( + exc: BaseException, + classification_category: str, +) -> AgentErrorKind: + """Map an ``LLMErrorClassification.error_category`` + raw exception to ``AgentErrorKind``.""" + status_code = getattr(exc, "status_code", None) + if status_code is None: + resp = getattr(exc, "response", None) + status_code = getattr(resp, "status_code", None) if resp is not None else None + exc_type = type(exc).__name__.lower() + exc_text = str(exc).lower() + + if classification_category == "model_error": + if ( + status_code == 404 + or "notfound" in exc_type + or any( + tok in exc_text + for tok in ( + "model_not_found", + "not found", + "does not exist", + "unknown model", + "unsupported model", + "invalid llm", + ) + ) + ): + return "model_not_found" + return "format_error" + if classification_category == "auth": + if status_code == 403 or "permissiondenied" in exc_type or "forbidden" in exc_text: + return "auth_permanent" + return "auth" + if classification_category == "rate_limit": + return "rate_limit" + if classification_category == "overloaded": + return "overloaded" + if classification_category == "context_overflow": + return "context_overflow" + return "unknown" + + def _reset_loop_guard_for_turn(agent): """Reset per-turn loop-guard limits unless another turn on this agent is still running (the guard is shared; resetting it would restart that turn's limits). @@ -2190,7 +2248,6 @@ def _chat_completion(self, messages, temperature=None, tools=None, stream=None, # a second time by the non-streaming fallback below. raise except Exception as e: - from ..errors import LLMError # Don't retry if it's an LLMError that has exhausted retries if isinstance(e, LLMError): raise # Re-raise LLMErrors immediately to avoid double retry @@ -2323,21 +2380,25 @@ def _chat_completion(self, messages, temperature=None, tools=None, stream=None, except ToolExecutionError: raise except Exception as e: - from ..errors import LLMError - # Emit LLM response trace event on error _duration_ms = (time.time() - start_time) * 1000 _trace_emitter.llm_response( self.name, duration_ms=_duration_ms, finish_reason="error", - response_content=str(e), # Include error for context replay + response_content=sanitize_llm_error_message( + str(e), + extra_secrets=(getattr(self, "api_key", None),), + preserve_tokens=(self.llm if isinstance(self.llm, str) else None,), + ), ) + if isinstance(e, LLMError): + raise # Use structured error classification for all error types (replaces legacy heuristic checks) from ..llm.error_classifier import classify_llm_error - model_name = self.llm if isinstance(self.llm, str) else "unknown" + model_name = self.llm if isinstance(self.llm, str) else repr(self.llm) session_id = getattr(self, '_session_id', 'unknown') # Determine provider from model name or use default @@ -2474,12 +2535,17 @@ def _chat_completion(self, messages, temperature=None, tools=None, stream=None, str(e), model_name=model_name, agent_id=self.name, + error_category=_resolve_agent_error_kind(e, classification.error_category), is_retryable=classification.is_retryable, context={ "session_id": session_id, "error_category": classification.error_category, "user_message": user_message, }, + extra_secrets=( + getattr(self, "api_key", None), + getattr(getattr(self, "_openai_client", None), "api_key", None), + ), ) # Call error hook if available for error interception @@ -2514,9 +2580,11 @@ async def _handle_async_llm_error( """ import asyncio from ..llm.error_classifier import classify_llm_error - from ..errors import LLMError + + if isinstance(exc, LLMError): + raise exc - model_name = self.llm if isinstance(self.llm, str) else "unknown" + model_name = self.llm if isinstance(self.llm, str) else repr(self.llm) session_id = getattr(self, '_session_id', 'unknown') # Determine provider from model name or use default @@ -2659,12 +2727,17 @@ async def _handle_async_llm_error( str(exc), model_name=model_name, agent_id=self.name, + error_category=_resolve_agent_error_kind(exc, classification.error_category), is_retryable=classification.is_retryable, context={ "session_id": session_id, "error_category": classification.error_category, "user_message": user_message, }, + extra_secrets=( + getattr(self, "api_key", None), + getattr(getattr(self, "_openai_client", None), "api_key", None), + ), ) # Call error hook if available for error interception @@ -2677,7 +2750,74 @@ async def _handle_async_llm_error( logging.debug(f"Error in on_error hook: {hook_error}") # Raise the enriched error - raise error + raise error from exc + + def _validate_default_llm_model(self) -> None: + """Reject non-string or blank ``self.llm`` before default-provider dispatch.""" + if not self._using_custom_llm and ( + not isinstance(self.llm, str) or not self.llm.strip() + ): + error = LLMError( + f"Invalid llm parameter {self.llm!r}: expected a non-empty model name string or compatible LLM backend.", + model_name=repr(self.llm), + agent_id=getattr(self, "name", None) or "unknown", + error_category="model_not_found", + is_retryable=False, + context={ + "session_id": getattr(self, "_session_id", "unknown"), + "error_category": "model_error", + "user_message": "Invalid model name or LLM backend configuration.", + }, + extra_secrets=(getattr(self, "api_key", None),), + ) + if hasattr(self, "on_error") and self.on_error: + try: + self.on_error(error) + except Exception as hook_error: + logging.debug(f"Error in on_error hook: {hook_error}") + raise error + + def _wrap_custom_llm_error(self, exc: Exception) -> LLMError: + """Classify and wrap an unhandled exception from ``self.llm_instance`` into ``LLMError``.""" + if isinstance(exc, LLMError): + return exc + from ..llm.error_classifier import classify_llm_error + + raw_model = getattr(getattr(self, "llm_instance", None), "model", None) + if not raw_model: + raw_model = self.llm.get("model") if isinstance(self.llm, dict) else self.llm + model_name = str(raw_model) if raw_model else "unknown" + provider = "openai" + if "/" in model_name: + provider = model_name.split("/", 1)[0].lower() or "openai" + elif "claude" in model_name.lower() or "anthropic" in model_name.lower(): + provider = "anthropic" + elif "azure" in model_name.lower(): + provider = "azure" + + classification = classify_llm_error(exc, provider=provider, model=model_name) + error = LLMError( + str(exc), + model_name=model_name, + agent_id=getattr(self, "name", None) or "unknown", + error_category=_resolve_agent_error_kind(exc, classification.error_category), + is_retryable=classification.is_retryable, + context={ + "session_id": getattr(self, "_session_id", "unknown"), + "error_category": classification.error_category, + "user_message": classification.user_message, + }, + extra_secrets=( + getattr(self, "api_key", None), + getattr(getattr(self, "llm_instance", None), "api_key", None), + ), + ) + if hasattr(self, "on_error") and self.on_error: + try: + self.on_error(error) + except Exception as hook_error: + logging.debug(f"Error in on_error hook: {hook_error}") + return error def _execute_unified_chat_completion( self, @@ -2701,6 +2841,8 @@ def _execute_unified_chat_completion( composition for safety and maintainability. """ from ..llm import create_llm_dispatcher + + self._validate_default_llm_model() # Get or create unified dispatcher if not hasattr(self, '_unified_dispatcher') or self._unified_dispatcher is None: @@ -2855,6 +2997,7 @@ async def _execute_unified_achat_completion( # concurrent turns. dispatcher = _dispatcher if dispatcher is None: + self._validate_default_llm_model() # Get or create the shared cached dispatcher if not hasattr(self, '_unified_dispatcher') or self._unified_dispatcher is None: # Provider selection based on existing agent configuration @@ -3022,7 +3165,6 @@ async def _execute_unified_achat_completion( except BudgetExceededError: raise except Exception as e: - from ..errors import LLMError # Apply the same structured error classification for async path return await self._handle_async_llm_error( e, messages, temperature, tools, stream, reasoning_steps, @@ -3470,7 +3612,7 @@ def chat(self, prompt: str, temperature: Optional[float] = None, tools: Optional durable_context.finalize(outcome) self.execution.resume_run_id = None return result - except ToolExecutionError as exc: + except (ToolExecutionError, LLMError) as exc: if durable_context is not None and not exc.is_retryable: durable_context.finalize("failed") self.execution.resume_run_id = None @@ -3856,19 +3998,31 @@ def _chat_impl(self, prompt, temperature, tools, output_json, output_pydantic, r # Rollback chat history on guardrail failure self._rollback_chat_history_to(chat_history_length) return self._guardrail_blocked_message(e) - except ToolExecutionError: + except (ToolExecutionError, InterruptedError, BudgetExceededError): raise - except Exception as e: + except BaseException as e: + if not isinstance(e, Exception): + raise self._last_stop_reason = "error" # Rollback chat history if LLM call fails self._rollback_chat_history_to(chat_history_length) - _get_display_functions()['display_error'](f"Error in LLM chat: {e}") + llm_error = self._wrap_custom_llm_error(e) + _get_display_functions()['display_error'](f"Error in LLM chat: {llm_error}", console=self.console) + if _is_fatal_llm_configuration_error(llm_error): + if llm_error is e: + raise + raise llm_error from e return None - except ToolExecutionError: + except (ToolExecutionError, InterruptedError, BudgetExceededError, LLMError): raise except Exception as e: self._last_stop_reason = "error" - _get_display_functions()['display_error'](f"Error in LLM chat: {e}") + llm_error = self._wrap_custom_llm_error(e) + _get_display_functions()['display_error'](f"Error in LLM chat: {llm_error}", console=self.console) + if _is_fatal_llm_configuration_error(llm_error): + if llm_error is e: + raise + raise llm_error from e return None finally: self._end_turn_tracking(_turn_token) @@ -4142,6 +4296,8 @@ def _chat_impl(self, prompt, temperature, tools, output_json, output_pydantic, r reflection_count += 1 continue # Continue the loop for more reflections + except (LLMError, BudgetExceededError, ToolExecutionError, InterruptedError): + raise except Exception as e: _get_display_functions()['display_error'](f"Error in parsing self-reflection json {e}. Retrying", console=self.console) logging.error("Reflection parsing failed.", exc_info=True) @@ -4167,12 +4323,24 @@ def _chat_impl(self, prompt, temperature, tools, output_json, output_pydantic, r except Exception: # Catch any exception from the inner try block and re-raise to outer handler raise - except ToolExecutionError: + except (ToolExecutionError, InterruptedError, BudgetExceededError): raise - except Exception as e: + except LLMError as e: self._last_stop_reason = "error" - # Catch any exceptions that escape the while loop _get_display_functions()['display_error'](f"Unexpected error in chat: {e}", console=self.console) + self._rollback_chat_history_to(chat_history_length) + if _is_fatal_llm_configuration_error(e): + raise + return None + except Exception as e: + self._last_stop_reason = "error" + # Catch any non-LLM internal exceptions that escape the while loop + _safe_err = sanitize_llm_error_message( + str(e), + extra_secrets=(getattr(self, "api_key", None),), + preserve_tokens=(self.llm if isinstance(self.llm, str) else None,), + ) + _get_display_functions()['display_error'](f"Unexpected error in chat: {_safe_err}", console=self.console) # Rollback chat history self._rollback_chat_history_to(chat_history_length) return None @@ -4311,7 +4479,7 @@ async def achat(self, prompt: str, temperature: Optional[float] = None, tools: O await durable_context.afinalize(outcome) self.execution.resume_run_id = None return result - except ToolExecutionError as exc: + except (ToolExecutionError, LLMError) as exc: if durable_context is not None and not exc.is_retryable: await durable_context.afinalize("failed") self.execution.resume_run_id = None @@ -4422,6 +4590,7 @@ async def _achat_impl(self, prompt, temperature, tools, output_json, output_pyda start_time = time.time() reasoning_steps = reasoning_steps or self.reasoning_steps + chat_history_length = len(self.chat_history) try: # Async adapters are awaited (sync adapters are offloaded by # AsyncMemoryMixin), so turn-start retrieval never blocks the loop. @@ -4580,16 +4749,21 @@ async def _achat_impl(self, prompt, temperature, tools, output_json, output_pyda # Rollback chat history on guardrail failure self._rollback_chat_history_to(chat_history_length) return self._guardrail_blocked_message(e) - except ToolExecutionError: + except (ToolExecutionError, InterruptedError, asyncio.CancelledError, BudgetExceededError): raise except Exception as e: self._last_stop_reason = "error" # Rollback chat history if LLM call fails self._rollback_chat_history_to(chat_history_length) - _get_display_functions()['display_error'](f"Error in LLM chat: {e}") + llm_error = self._wrap_custom_llm_error(e) + _get_display_functions()['display_error'](f"Error in LLM chat: {llm_error}", console=self.console) if get_logger().getEffectiveLevel() == logging.DEBUG: total_time = time.time() - start_time - logging.debug(f"Agent.achat failed in {total_time:.2f} seconds: {str(e)}") + logging.debug(f"Agent.achat failed in {total_time:.2f} seconds: {llm_error}") + if _is_fatal_llm_configuration_error(llm_error): + if llm_error is e: + raise + raise llm_error from e return None # For OpenAI client @@ -5078,27 +5252,47 @@ async def _achat_impl(self, prompt, temperature, tools, output_json, output_pyda # Rollback chat history on guardrail failure self._rollback_chat_history_to(chat_history_length) return self._guardrail_blocked_message(e) - except ToolExecutionError: + except (ToolExecutionError, InterruptedError, asyncio.CancelledError, LLMError, BudgetExceededError): raise except Exception as e: self._last_stop_reason = "error" - _get_display_functions()['display_error'](f"Error in chat completion: {e}") + _safe_err = sanitize_llm_error_message( + str(e), + extra_secrets=(getattr(self, "api_key", None),), + preserve_tokens=(self.llm if isinstance(self.llm, str) else None,), + ) + _get_display_functions()['display_error'](f"Error in chat completion: {_safe_err}") if get_logger().getEffectiveLevel() == logging.DEBUG: total_time = time.time() - start_time - logging.debug(f"Agent.achat failed in {total_time:.2f} seconds: {str(e)}") + logging.debug(f"Agent.achat failed in {total_time:.2f} seconds: {_safe_err}") # Rollback chat history so a failed async turn does not leave # a dangling, unanswered user message (mirrors the sync path # and achat()'s own guardrail handler). self._rollback_chat_history_to(chat_history_length) return None - except ToolExecutionError: + except (ToolExecutionError, InterruptedError, asyncio.CancelledError, BudgetExceededError): raise + except LLMError as e: + self._last_stop_reason = "error" + _get_display_functions()['display_error'](f"Error in achat: {e}", console=self.console) + if get_logger().getEffectiveLevel() == logging.DEBUG: + total_time = time.time() - start_time + logging.debug(f"Agent.achat failed in {total_time:.2f} seconds: {e}") + self._rollback_chat_history_to(chat_history_length) + if _is_fatal_llm_configuration_error(e): + raise + return None except Exception as e: self._last_stop_reason = "error" - _get_display_functions()['display_error'](f"Error in achat: {e}") + _safe_err = sanitize_llm_error_message( + str(e), + extra_secrets=(getattr(self, "api_key", None),), + preserve_tokens=(self.llm if isinstance(self.llm, str) else None,), + ) + _get_display_functions()['display_error'](f"Error in achat: {_safe_err}") if get_logger().getEffectiveLevel() == logging.DEBUG: total_time = time.time() - start_time - logging.debug(f"Agent.achat failed in {total_time:.2f} seconds: {str(e)}") + logging.debug(f"Agent.achat failed in {total_time:.2f} seconds: {_safe_err}") # Rollback chat history so a failed async turn does not leave a # dangling, unanswered user message (mirrors the sync path). self._rollback_chat_history_to(chat_history_length) @@ -5346,7 +5540,7 @@ def _start_stream(self, prompt: str, **kwargs) -> Generator[str, None, None]: if durable_context is not None: durable_context.finalize("succeeded") self.execution.resume_run_id = None - except ToolExecutionError as exc: + except (ToolExecutionError, LLMError) as exc: if durable_context is not None and not exc.is_retryable: durable_context.finalize("failed") self.execution.resume_run_id = None @@ -6570,8 +6764,6 @@ def _chat_completion_with_retry_core(self, messages, temperature=None, tools=Non cancel_token=cancel_token) except Exception as e: - from ..errors import LLMError - # Only retry LLMErrors that are marked as retryable if not isinstance(e, LLMError) or not e.is_retryable: raise # Re-raise non-retryable errors immediately @@ -6725,8 +6917,6 @@ async def _achat_completion_with_retry_core(self, messages, temperature=None, to ) except Exception as e: - from ..errors import LLMError - # Only retry LLMErrors that are marked as retryable if not isinstance(e, LLMError) or not e.is_retryable: raise # Re-raise non-retryable errors immediately diff --git a/src/praisonai-agents/praisonaiagents/agent/execution_mixin.py b/src/praisonai-agents/praisonaiagents/agent/execution_mixin.py index 9e799007f5..1b6ffbe33e 100644 --- a/src/praisonai-agents/praisonaiagents/agent/execution_mixin.py +++ b/src/praisonai-agents/praisonaiagents/agent/execution_mixin.py @@ -328,6 +328,11 @@ def run(self, prompt: str, **kwargs: Any) -> Optional[str]: Returns: The agent's response as a string, or a ``RunOutcome`` when ``return_outcome=True``. + + Raises: + LLMError: When the configured ``llm``/model is invalid or the LLM + provider returns an unrecoverable error (unless + ``return_outcome=True``). Example: ```python @@ -337,10 +342,11 @@ def run(self, prompt: str, **kwargs: Any) -> Optional[str]: ``` Note: + Use ``run()`` in automation; ``start()`` is interactive. Unlike .start() which enables verbose output in TTY for interactive use, .run() is always silent. This makes it suitable for: - Production pipelines - - Automated scripts + - Automated scripts and CI - Background processing - API endpoints """ @@ -814,6 +820,11 @@ def start(self, prompt: Optional[str] = None, **kwargs: Any) -> Union[str, Gener Returns: - If streaming: Generator yielding response chunks - If not streaming: The complete response as a string + + Raises: + LLMError: When the configured ``llm``/model is invalid or the LLM + provider returns an unrecoverable error. Rich UI displays a + sanitized error before propagating the exception to callers. Example: ```python @@ -827,6 +838,7 @@ def start(self, prompt: Optional[str] = None, **kwargs: Any) -> Union[str, Gener ``` Note: + Use ``run()`` in automation; ``start()`` is interactive. Unlike .run() which is always silent (production use), .start() enables verbose output by default when in a TTY for beginner-friendly interactive use. Use .run() for programmatic/scripted usage. @@ -1006,20 +1018,18 @@ def run_chat(): current_status[0] = "Sending to LLM..." # Check for steering messages before starting chat + steering_msg = None if hasattr(self, '_check_steering_messages'): try: steering_msg = self._check_steering_messages() - if steering_msg: - # Inject steering message into prompt with clear separator - prompt_with_steering = f"{prompt}\n\n{steering_msg}" - current_status[0] = "Processing steering guidance..." - result_holder[0] = self.chat(prompt_with_steering, **kwargs) - else: - result_holder[0] = self.chat(prompt, **kwargs) except Exception as e: logger.warning(f"Steering check failed, continuing without steering: {e}") current_status[0] = "Steering failed, proceeding normally..." - result_holder[0] = self.chat(prompt, **kwargs) + + if steering_msg: + prompt_with_steering = f"{prompt}\n\n{steering_msg}" + current_status[0] = "Processing steering guidance..." + result_holder[0] = self.chat(prompt_with_steering, **kwargs) else: result_holder[0] = self.chat(prompt, **kwargs) @@ -1104,12 +1114,14 @@ def run_chat(): if current_status[0] != last_status: last_status = current_status[0] status.update(f"[bold yellow]Working...[/] {current_status[0]}") + + chat_thread.join() # Calculate elapsed time elapsed = time_module.time() - start_time # Re-raise any error from chat - if error_holder[0]: + if error_holder[0] is not None: raise error_holder[0] result = result_holder[0] diff --git a/src/praisonai-agents/praisonaiagents/errors.py b/src/praisonai-agents/praisonaiagents/errors.py index 8ccdb4e740..559d51f6d5 100644 --- a/src/praisonai-agents/praisonaiagents/errors.py +++ b/src/praisonai-agents/praisonaiagents/errors.py @@ -9,11 +9,62 @@ - External integrations """ -from typing import Literal, Protocol, runtime_checkable, Optional, Dict, Any, get_args +from typing import Literal, Protocol, runtime_checkable, Optional, Dict, Any, Sequence, get_args from dataclasses import dataclass, field +import os +import re import uuid import warnings +_API_KEY_TOKEN_PATTERN = re.compile(r"\bsk-[A-Za-z0-9_\-]{12,}\b") +_SECRET_ENV_VARS = ( + "OPENAI_API_KEY", + "ANTHROPIC_API_KEY", + "GOOGLE_API_KEY", + "GEMINI_API_KEY", + "GROQ_API_KEY", + "MISTRAL_API_KEY", + "DEEPSEEK_API_KEY", + "OPENROUTER_API_KEY", + "XAI_API_KEY", + "COHERE_API_KEY", + "AZURE_OPENAI_API_KEY", +) + + +def sanitize_llm_error_message( + message: str, + *, + extra_secrets: Optional[Sequence[Optional[str]]] = None, + preserve_tokens: Optional[Sequence[Optional[str]]] = None, +) -> str: + """Redact API keys and credential literals from an LLM error message.""" + if not message: + return message + text = str(message) + try: + from .trace.redact import redact_string + text = redact_string(text, enabled=True) + except Exception: + pass + text = _API_KEY_TOKEN_PATTERN.sub("[REDACTED]", text) + + preserved = {t for t in (preserve_tokens or ()) if t} + candidates = [] + if extra_secrets: + candidates.extend(extra_secrets) + for env_name in _SECRET_ENV_VARS: + candidates.append(os.environ.get(env_name)) + for secret in candidates: + if ( + isinstance(secret, str) + and len(secret) >= 6 + and secret not in preserved + and secret in text + ): + text = text.replace(secret, "[REDACTED]") + return text + # Closed error taxonomy for typed failure classification AgentErrorKind = Literal[ @@ -167,19 +218,34 @@ def __init__( run_id: Optional[str] = None, error_category: AgentErrorKind = "unknown", is_retryable: bool = False, # Default to non-retryable unless specified - context: Optional[Dict[str, Any]] = None + context: Optional[Dict[str, Any]] = None, + extra_secrets: Optional[Sequence[Optional[str]]] = None, ): + model_str = str(model_name) if model_name else "unknown" + sanitized_message = sanitize_llm_error_message( + str(message), + extra_secrets=extra_secrets, + preserve_tokens=(model_str,), + ) context = context or {} - context["model_name"] = model_name + context["model_name"] = model_str super().__init__( - message, - agent_id=agent_id, - run_id=run_id, + sanitized_message, + agent_id=agent_id or "unknown", + run_id=run_id, error_category=error_category, is_retryable=is_retryable, - context=context + context=context, ) - self.model_name = model_name + self.model_name = model_str + + def __str__(self) -> str: + if self.model_name and self.model_name != "unknown": + return ( + f"[{self.error_category}] {self.message} " + f"(model: {self.model_name}, agent: {self.agent_id}, run: {self.run_id})" + ) + return super().__str__() class BudgetExceededError(PraisonAIError): @@ -471,5 +537,6 @@ def __init__(self, message: str, validation_errors: Optional[list] = None, **kwa "HandoffDepthError", "HandoffTimeoutError", "HandoffValidationError", - "PraisonAIConfigError" + "PraisonAIConfigError", + "sanitize_llm_error_message", ] \ No newline at end of file diff --git a/src/praisonai-agents/praisonaiagents/llm/llm.py b/src/praisonai-agents/praisonaiagents/llm/llm.py index 2f70f8aa7e..9a3a181024 100644 --- a/src/praisonai-agents/praisonaiagents/llm/llm.py +++ b/src/praisonai-agents/praisonaiagents/llm/llm.py @@ -4651,7 +4651,13 @@ def _prepare_return_value(text: str) -> Union[str, tuple]: return _prepare_return_value(response_text) except Exception as error: - _get_display_functions()['display_error'](f"Error in get_response: {str(error)}") + from ..errors import sanitize_llm_error_message + _safe_err = sanitize_llm_error_message( + str(error), + extra_secrets=(getattr(self, "api_key", None),), + preserve_tokens=(getattr(self, "model", None),), + ) + _get_display_functions()['display_error'](f"Error in get_response: {_safe_err}") raise # Log completion time if in debug mode @@ -6528,7 +6534,13 @@ def _prepare_return_value(text: str) -> Union[str, tuple]: except Exception as error: if LLMContextLengthExceededException(str(error))._is_context_limit_error(str(error)): raise LLMContextLengthExceededException(str(error)) - _get_display_functions()['display_error'](f"Error in get_response_async: {str(error)}") + from ..errors import sanitize_llm_error_message + _safe_err = sanitize_llm_error_message( + str(error), + extra_secrets=(getattr(self, "api_key", None),), + preserve_tokens=(getattr(self, "model", None),), + ) + _get_display_functions()['display_error'](f"Error in get_response_async: {_safe_err}") raise # Log completion time if in debug mode diff --git a/src/praisonai-agents/praisonaiagents/llm/openai_client.py b/src/praisonai-agents/praisonaiagents/llm/openai_client.py index 7e93673d19..5d51d5f23b 100644 --- a/src/praisonai-agents/praisonaiagents/llm/openai_client.py +++ b/src/praisonai-agents/praisonaiagents/llm/openai_client.py @@ -1465,8 +1465,12 @@ def process_stream_response( status = raw.get("status") if isinstance(raw, dict) else getattr(raw, "status", None) if status == "failed": raise - self.logger.error(f"Error in stream processing: {e}") - return None + from ..errors import sanitize_llm_error_message + self.logger.error( + f"Error in stream processing: " + f"{sanitize_llm_error_message(str(e), extra_secrets=(self.api_key,), preserve_tokens=(model,))}" + ) + raise async def process_stream_response_async( self, @@ -1738,8 +1742,12 @@ async def emit_event(event: 'StreamEvent'): status = raw.get("status") if isinstance(raw, dict) else getattr(raw, "status", None) if status == "failed": raise - self.logger.error(f"Error in async stream processing: {e}") - return None + from ..errors import sanitize_llm_error_message + self.logger.error( + f"Error in async stream processing: " + f"{sanitize_llm_error_message(str(e), extra_secrets=(self.api_key,), preserve_tokens=(model,))}" + ) + raise def _track_token_usage(self, response: Any, model: str, agent_name: Optional[str] = None) -> None: """Record token usage from a response into the global collector. diff --git a/src/praisonai-agents/tests/unit/agent/test_invalid_llm_propagation.py b/src/praisonai-agents/tests/unit/agent/test_invalid_llm_propagation.py new file mode 100644 index 0000000000..5f2a6fe701 --- /dev/null +++ b/src/praisonai-agents/tests/unit/agent/test_invalid_llm_propagation.py @@ -0,0 +1,399 @@ +"""Unit and regression tests for invalid ``llm`` handling & exception propagation (#5956). + +Verifies that ``Agent.start()``, ``Agent.run()``, ``Agent.chat()``, and their +async counterparts (``astart()``, ``arun()``, ``achat()``) raise ``LLMError`` +to callers when configured with an invalid ``llm`` parameter or when the +provider rejects the model, even after the Rich UI renderer displays the error, +and never leak API keys in ``str(exc)`` or Rich UI output. +""" + +from __future__ import annotations + +import os +import subprocess +import sys +from pathlib import Path +from unittest.mock import AsyncMock, MagicMock + +import httpx +import openai +import pytest +from praisonaiagents import Agent +from praisonaiagents.errors import LLMError, PraisonAIError, sanitize_llm_error_message + +INVALID_MODEL = "not-a-real-model-xyz" +SECRET_API_KEY = "sk-proj-supersecretkey1234567890abcdef" + + +def _make_not_found_error( + model: str = INVALID_MODEL, secret: str = "" +) -> openai.NotFoundError: + """Construct a realistic OpenAI 404 NotFoundError without network I/O.""" + req = httpx.Request("POST", "https://api.openai.com/v1/chat/completions") + detail = f"The model `{model}` does not exist or you do not have access to it." + if secret: + detail += f" (api_key={secret} Bearer {secret})" + resp = httpx.Response( + 404, + request=req, + json={ + "error": { + "message": detail, + "type": "invalid_request_error", + "param": None, + "code": "model_not_found", + } + }, + ) + return openai.NotFoundError( + f"Error code: 404 - {{'error': {{'message': {detail!r}, 'code': 'model_not_found'}}}}", + response=resp, + body={"error": {"message": detail, "code": "model_not_found"}}, + ) + + +def _wire_failing_openai_client(agent: Agent, error: Exception) -> MagicMock: + """Attach a mock OpenAIClient whose sync and async transports raise ``error``.""" + client = MagicMock() + client.api_key = SECRET_API_KEY + client.build_messages.side_effect = lambda prompt, **kw: ( + [{"role": "user", "content": str(prompt)}], + prompt, + ) + client.sync_client.chat.completions.create.side_effect = error + client.async_client.chat.completions.create = AsyncMock(side_effect=error) + client.chat_completion_with_tools.side_effect = error + client.achat_completion_with_tools = AsyncMock(side_effect=error) + agent._Agent__openai_client = client + agent._unified_dispatcher = None + return client + + +class TestSyncInvalidModelPropagation: + """Sync start(), run(), and chat() must raise LLMError on invalid models.""" + + def test_start_raises_on_invalid_model_non_tty(self, monkeypatch): + monkeypatch.setenv("OPENAI_API_KEY", SECRET_API_KEY) + monkeypatch.setattr(sys.stdout, "isatty", lambda: False) + + agent = Agent(instructions="Reply with OK", llm=INVALID_MODEL) + _wire_failing_openai_client(agent, _make_not_found_error()) + + with pytest.raises(LLMError) as exc_info: + agent.start("Say hello") + + exc = exc_info.value + assert isinstance(exc, PraisonAIError) + assert exc.model_name == INVALID_MODEL + assert INVALID_MODEL in str(exc) + assert exc.error_category == "model_not_found" + assert exc.is_retryable is False + assert agent._last_stop_reason == "error" + assert agent.chat_history == [] + + def test_start_raises_on_invalid_model_tty_after_rich_display(self, monkeypatch): + """Rich UI display_error may render the error, but start() MUST still raise.""" + monkeypatch.setenv("OPENAI_API_KEY", SECRET_API_KEY) + monkeypatch.setattr(sys.stdout, "isatty", lambda: True) + + displayed_errors: list[str] = [] + from praisonaiagents.agent import chat_mixin + + real_get_display = chat_mixin._get_display_functions + + def _capturing_display_functions(): + funcs = dict(real_get_display()) + orig_display_error = funcs["display_error"] + + def _record_error(msg, *args, **kwargs): + displayed_errors.append(str(msg)) + return orig_display_error(msg, *args, **kwargs) + + funcs["display_error"] = _record_error + return funcs + + monkeypatch.setattr( + chat_mixin, "_get_display_functions", _capturing_display_functions + ) + + agent = Agent(instructions="Reply with OK", llm=INVALID_MODEL, output="verbose") + client = _wire_failing_openai_client( + agent, _make_not_found_error(secret=SECRET_API_KEY) + ) + + with pytest.raises(LLMError) as exc_info: + agent.start("Say hello") + + assert exc_info.value.model_name == INVALID_MODEL + assert exc_info.value.error_category == "model_not_found" + # Rich UI rendered the error AND exception still propagated + assert len(displayed_errors) >= 1 + for rendered in displayed_errors: + assert SECRET_API_KEY not in rendered + # Provider must only be called once (steering wrapper must not re-run chat on LLMError) + total_calls = ( + client.chat_completion_with_tools.call_count + + client.achat_completion_with_tools.await_count + ) + assert total_calls == 1 + + def test_start_stream_raises_on_invalid_model(self, monkeypatch): + monkeypatch.setenv("OPENAI_API_KEY", SECRET_API_KEY) + + agent = Agent(instructions="Reply with OK", llm=INVALID_MODEL) + _wire_failing_openai_client(agent, _make_not_found_error()) + + with pytest.raises(LLMError) as exc_info: + list(agent.start("Say hello", stream=True)) + + assert exc_info.value.model_name == INVALID_MODEL + assert exc_info.value.__cause__ is not None + + def test_run_raises_on_invalid_model(self, monkeypatch): + monkeypatch.setenv("OPENAI_API_KEY", SECRET_API_KEY) + + agent = Agent(instructions="Reply with OK", llm=INVALID_MODEL) + _wire_failing_openai_client(agent, _make_not_found_error()) + + with pytest.raises(LLMError) as exc_info: + agent.run("Say hello") + + assert exc_info.value.model_name == INVALID_MODEL + assert exc_info.value.error_category == "model_not_found" + assert agent.chat_history == [] + + def test_run_return_outcome_captures_llm_error_as_failed(self, monkeypatch): + monkeypatch.setenv("OPENAI_API_KEY", SECRET_API_KEY) + + agent = Agent(instructions="Reply with OK", llm=INVALID_MODEL) + _wire_failing_openai_client(agent, _make_not_found_error(secret=SECRET_API_KEY)) + + outcome = agent.run("Say hello", return_outcome=True) + assert outcome.reason == "failed" + assert outcome.succeeded is False + assert outcome.error is not None + assert INVALID_MODEL in outcome.error + assert SECRET_API_KEY not in outcome.error + + def test_chat_raises_on_invalid_model(self, monkeypatch): + monkeypatch.setenv("OPENAI_API_KEY", SECRET_API_KEY) + + agent = Agent(instructions="Reply with OK", llm=INVALID_MODEL) + _wire_failing_openai_client(agent, _make_not_found_error()) + + with pytest.raises(LLMError) as exc_info: + agent.chat("Say hello") + + assert exc_info.value.model_name == INVALID_MODEL + assert exc_info.value.error_category == "model_not_found" + assert agent.chat_history == [] + + +class TestAsyncInvalidModelPropagation: + """Async astart(), arun(), and achat() must raise LLMError (including TTY stream).""" + + @pytest.mark.asyncio + async def test_astart_non_tty_raises_llm_error(self, monkeypatch): + monkeypatch.setenv("OPENAI_API_KEY", SECRET_API_KEY) + monkeypatch.setattr(sys.stdout, "isatty", lambda: False) + + agent = Agent(instructions="Reply with OK", llm=INVALID_MODEL) + _wire_failing_openai_client(agent, _make_not_found_error()) + + with pytest.raises(LLMError) as exc_info: + await agent.astart("Say hello") + + assert exc_info.value.model_name == INVALID_MODEL + assert exc_info.value.error_category == "model_not_found" + assert agent.chat_history == [] + + @pytest.mark.asyncio + async def test_astart_tty_streaming_raises_llm_error_via_real_openai_client( + self, monkeypatch + ): + """Regression: OpenAIClient.process_stream_response_async previously returned None.""" + from praisonaiagents.llm.openai_client import OpenAIClient + + monkeypatch.setenv("OPENAI_API_KEY", SECRET_API_KEY) + monkeypatch.setattr(sys.stdout, "isatty", lambda: True) + + agent = Agent(instructions="Reply with OK", llm=INVALID_MODEL) + real_client = OpenAIClient(api_key=SECRET_API_KEY) + fake_async = MagicMock() + fake_async.chat.completions.create = AsyncMock( + side_effect=_make_not_found_error() + ) + real_client._async_client = fake_async + agent._Agent__openai_client = real_client + agent._unified_dispatcher = None + + with pytest.raises(LLMError) as exc_info: + await agent.astart("Say hello") + + assert exc_info.value.model_name == INVALID_MODEL + assert exc_info.value.error_category == "model_not_found" + assert agent.chat_history == [] + + @pytest.mark.asyncio + async def test_arun_and_achat_raise_llm_error(self, monkeypatch): + monkeypatch.setenv("OPENAI_API_KEY", SECRET_API_KEY) + + agent = Agent(instructions="Reply with OK", llm=INVALID_MODEL) + _wire_failing_openai_client(agent, _make_not_found_error()) + + with pytest.raises(LLMError): + await agent.arun("Say hello") + assert agent.chat_history == [] + + with pytest.raises(LLMError): + await agent.achat("Say hello") + assert agent.chat_history == [] + + +class TestCustomLLMAndInvalidParameterInitialization: + """Custom provider strings, dict configs, blank strings, and invalid objects.""" + + @pytest.mark.parametrize( + "llm_param,expected_model", + [ + (f"openai/{INVALID_MODEL}", f"openai/{INVALID_MODEL}"), + ({"model": f"openai/{INVALID_MODEL}"}, f"openai/{INVALID_MODEL}"), + ], + ) + def test_custom_llm_sync_methods_raise_llm_error( + self, monkeypatch, llm_param, expected_model + ): + monkeypatch.setenv("OPENAI_API_KEY", SECRET_API_KEY) + monkeypatch.setattr(sys.stdout, "isatty", lambda: False) + + agent = Agent(instructions="Reply with OK", llm=llm_param) + assert agent._using_custom_llm is True + agent.llm_instance.get_response = MagicMock( + side_effect=_make_not_found_error( + model=expected_model, secret=SECRET_API_KEY + ) + ) + + for method_name in ("start", "run", "chat"): + with pytest.raises(LLMError) as exc_info: + getattr(agent, method_name)("Say hello") + exc = exc_info.value + assert exc.model_name == expected_model + assert exc.error_category == "model_not_found" + assert SECRET_API_KEY not in str(exc) + assert agent.chat_history == [] + + @pytest.mark.asyncio + async def test_custom_llm_async_methods_raise_llm_error(self, monkeypatch): + expected_model = f"openai/{INVALID_MODEL}" + monkeypatch.setenv("OPENAI_API_KEY", SECRET_API_KEY) + monkeypatch.setattr(sys.stdout, "isatty", lambda: False) + + agent = Agent(instructions="Reply with OK", llm=expected_model) + agent.llm_instance.get_response_async = AsyncMock( + side_effect=_make_not_found_error( + model=expected_model, secret=SECRET_API_KEY + ) + ) + + for method_name in ("astart", "arun", "achat"): + with pytest.raises(LLMError) as exc_info: + await getattr(agent, method_name)("Say hello") + exc = exc_info.value + assert exc.model_name == expected_model + assert exc.error_category == "model_not_found" + assert SECRET_API_KEY not in str(exc) + assert agent.chat_history == [] + + @pytest.mark.parametrize("bad_llm", ["", " ", 12345, object()]) + def test_invalid_llm_value_raises_llm_error_across_sync_entrypoints( + self, monkeypatch, bad_llm + ): + monkeypatch.setenv("OPENAI_API_KEY", SECRET_API_KEY) + monkeypatch.setattr(sys.stdout, "isatty", lambda: False) + + agent = Agent(instructions="Reply with OK", llm=bad_llm) + assert agent._using_custom_llm is False + + for method_name in ("start", "run", "chat"): + with pytest.raises(LLMError) as exc_info: + getattr(agent, method_name)("Say hello") + assert exc_info.value.error_category == "model_not_found" + assert exc_info.value.is_retryable is False + assert agent.chat_history == [] + + @pytest.mark.asyncio + @pytest.mark.parametrize("bad_llm", ["", " ", 12345, object()]) + async def test_invalid_llm_value_raises_llm_error_across_async_entrypoints( + self, monkeypatch, bad_llm + ): + monkeypatch.setenv("OPENAI_API_KEY", SECRET_API_KEY) + monkeypatch.setattr(sys.stdout, "isatty", lambda: False) + + agent = Agent(instructions="Reply with OK", llm=bad_llm) + + for method_name in ("astart", "arun", "achat"): + with pytest.raises(LLMError) as exc_info: + await getattr(agent, method_name)("Say hello") + assert exc_info.value.error_category == "model_not_found" + assert exc_info.value.is_retryable is False + assert agent.chat_history == [] + + +class TestSecretRedactionAndProcessExit: + """API keys must never appear in str(exc), and unhandled scripts must exit non-zero.""" + + def test_no_api_key_in_str_exc(self, monkeypatch): + env_secret = "custom-provider-secret-987654321" + monkeypatch.setenv("OPENAI_API_KEY", env_secret) + + raw_msg = ( + f"Authentication/model failure for {INVALID_MODEL}: " + f"Bearer {SECRET_API_KEY}, api_key={env_secret}, raw={env_secret}" + ) + err = LLMError( + raw_msg, + model_name=INVALID_MODEL, + agent_id="test-agent", + error_category="model_not_found", + ) + + rendered = str(err) + assert INVALID_MODEL in rendered + assert SECRET_API_KEY not in rendered + assert env_secret not in rendered + assert SECRET_API_KEY not in err.message + assert env_secret not in err.message + assert "[REDACTED]" in rendered + + def test_sanitize_llm_error_message_preserves_model_name(self): + cleaned = sanitize_llm_error_message( + f"Model {INVALID_MODEL} rejected token {SECRET_API_KEY}", + extra_secrets=("my-custom-key-abcdef",), + preserve_tokens=(INVALID_MODEL,), + ) + assert INVALID_MODEL in cleaned + assert SECRET_API_KEY not in cleaned + + def test_unhandled_start_exits_nonzero_in_subprocess(self): + """Issue #5956: unhandled invalid llm in a script must exit non-zero, not 0.""" + code = ( + "from praisonaiagents import Agent; " + "Agent(instructions='Reply OK', llm=' ').start('Hello')" + ) + env = dict(os.environ) + pkg_root = str(Path(__file__).resolve().parents[3]) + existing_pp = env.get("PYTHONPATH", "") + env["PYTHONPATH"] = ( + f"{pkg_root}{os.pathsep}{existing_pp}" if existing_pp else pkg_root + ) + proc = subprocess.run( + [sys.executable, "-c", code], + check=False, + capture_output=True, + text=True, + timeout=30, + env=env, + ) + assert proc.returncode != 0 + assert "LLMError" in proc.stderr