fix(content_safety): clear error when Azure model init fails#1927
fix(content_safety): clear error when Azure model init fails#1927adityasingh2400 wants to merge 1 commit into
Conversation
…VIDIA-NeMo#1338) When a content_safety rail is configured against an Azure model that fails to initialize because of a missing or blank api_key (or deployment_name), the rail later tried to call .create on a None client and surfaced as LLMCallException: 'NoneType' object has no attribute 'create'. This change validates the model init at configuration time and raises a clear error pointing at the missing field, so users can fix the config without diffing tracebacks. Signed-off-by: Aditya Singh <adisin650@gmail.com>
📝 WalkthroughWalkthroughThe PR improves error detection and messaging when LLM model initialization or execution fails due to missing underlying SDK clients, typically caused by invalid or missing credentials. It adds a proactive initialization check in LangChain model setup and a reactive runtime error enrichment in the LLM call path, along with improved error context in content-safety actions. ChangesLLM Client Validation and Error Enrichment
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@nemoguardrails/integrations/langchain/langchain_initializer.py`:
- Around line 255-257: Update the docstring near the client/async_client
credential check in langchain_initializer.py to reflect the actual trigger: the
code raises when both client and async_client are None regardless of whether
credential kwargs were omitted or falsy; mention the check is not limited to
wrappers that expose a missing `client` attribute but instead ensures at least
one of `client` or `async_client` is provided. Reference the `client` and
`async_client` variables and the surrounding conditional that raises the error
so the docstring matches current behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 48e83aab-00e5-49e6-92dc-c3b8ac1d6907
📒 Files selected for processing (6)
nemoguardrails/actions/llm/utils.pynemoguardrails/integrations/langchain/langchain_initializer.pynemoguardrails/library/content_safety/actions.pytests/integrations/langchain/llm/models/test_langchain_initializer.pytests/integrations/langchain/test_actions_llm_utils.pytests/test_content_safety_actions.py
| This check is intentionally narrow: it only fires when ``client`` is explicitly ``None`` | ||
| while the corresponding kwarg was either omitted or set to a falsy value, so wrappers | ||
| that legitimately do not expose a ``client`` attribute (e.g. providers that lazily |
There was a problem hiding this comment.
Docstring does not match the actual trigger condition.
Line 255-257 says this check only fires when missing/falsy credential kwargs are identified, but the implementation raises whenever both client and async_client are None (even with no missing-kwargs evidence). Please align the docstring with current behavior.
✏️ Suggested docstring adjustment
- This check is intentionally narrow: it only fires when ``client`` is explicitly ``None``
- while the corresponding kwarg was either omitted or set to a falsy value, so wrappers
- that legitimately do not expose a ``client`` attribute (e.g. providers that lazily
- construct their transport) are not affected.
+ This check fires when a wrapper exposes ``client`` but both ``client`` and
+ ``async_client`` are ``None``. Wrappers that do not expose a ``client`` attribute
+ (e.g. providers that lazily construct transport and hide client internals) are not affected.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@nemoguardrails/integrations/langchain/langchain_initializer.py` around lines
255 - 257, Update the docstring near the client/async_client credential check in
langchain_initializer.py to reflect the actual trigger: the code raises when
both client and async_client are None regardless of whether credential kwargs
were omitted or falsy; mention the check is not limited to wrappers that expose
a missing `client` attribute but instead ensures at least one of `client` or
`async_client` is provided. Reference the `client` and `async_client` variables
and the surrounding conditional that raises the error so the docstring matches
current behavior.
Greptile SummaryThis PR improves the developer experience when an Azure (or other OpenAI-compatible) model fails to initialize because a required credential is missing or empty, replacing the cryptic
|
| Filename | Overview |
|---|---|
| nemoguardrails/integrations/langchain/langchain_initializer.py | Adds post-init client check _verify_model_has_usable_client; the exception it raises is caught by the broad except Exception loop in init_langchain_model, allowing a community Azure provider to return a broken model undetected |
| nemoguardrails/actions/llm/utils.py | Adds _is_none_client_attribute_error pattern match and enriches LLMCallException with a credential hint; logic is correct and provides a reliable fallback layer |
| nemoguardrails/library/content_safety/actions.py | Improves model-not-found error messages with available model list and credential hint; both input and output check paths updated symmetrically |
| tests/integrations/langchain/llm/models/test_langchain_initializer.py | Adds five unit tests for _verify_model_has_usable_client; covers happy paths and the regression case; no integration-level test of the swallowed-exception scenario |
| tests/integrations/langchain/test_actions_llm_utils.py | New regression test for NoneType attribute error hint in LLMCallException; assertions are correct and the test exercises the fallback detection path |
| tests/test_content_safety_actions.py | Two new tests verify that the updated model-not-found error message contains credential hints; assertions are correct for the new template strings |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[init_langchain_model] --> B[_init_chat_completion_model]
B --> C[init_chat_model returns model]
C --> D{_verify_model_has_usable_client}
D -->|client != None| E[return model ✓]
D -->|client == None| F[raise ModelInitializationError]
F --> G[except Exception in init_langchain_model
last_exception = error]
G --> H[_init_community_chat_models]
H --> I{provider in _chat_providers?}
I -->|No - returns None| J[All initializers exhausted
raise ModelInitializationError
with last_exception message]
I -->|Yes e.g. azure_openai| K[provider_cls with empty creds]
K --> L{client == None?}
L -->|community model also broken| M[return broken model ⚠️
ModelInitializationError silently discarded]
M --> N[llm_call → AttributeError]
N --> O{_is_none_client_attribute_error?}
O -->|Yes| P[LLMCallException with credential hint ✓]
O -->|No| Q[bare LLMCallException]
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
nemoguardrails/integrations/langchain/langchain_initializer.py:236
**`ModelInitializationError` swallowed by the outer retry loop**
`_verify_model_has_usable_client` raises `ModelInitializationError`, but `init_langchain_model` wraps every `try_initialization_method` call in `except Exception as e: last_exception = e`, so the exception is silently stored and the loop advances to `_init_community_chat_models`. That initializer looks up providers in `_chat_providers`, which is populated from `langchain_community.chat_models._module_lookup`. `AzureChatOpenAI` is registered under the key `azure_openai` (last segment of `langchain_community.chat_models.azure_openai`), so for `provider_name="azure_openai"`, the community initializer finds the class, instantiates it with the same empty credentials, and — because there is no equivalent `_verify_model_has_usable_client` call on that path — returns a broken model with `client = None`. `init_langchain_model` returns that model, the helpful `ModelInitializationError` is discarded, and the original `NoneType` attribute error resurfaces at first rail invocation.
### Issue 2 of 2
nemoguardrails/integrations/langchain/langchain_initializer.py:260-266
**`async_client` check inadvertently fires for models with no `async_client` attribute**
When `model.client` is `None` and the model has no `async_client` attribute at all (i.e. the provider only exposes a sync client path), the ternary `getattr(model, "async_client", None) if hasattr(model, "async_client") else None` returns `None`, and `client is not None or async_client is not None` evaluates to `False`, so `ModelInitializationError` is raised. Any LangChain integration that legitimately holds `client = None` during construction before finalising it in a `model_post_init` or validator hook, and that does not expose an `async_client` attribute, would be incorrectly rejected.
Reviews (1): Last reviewed commit: "fix(content_safety): clear error when Az..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| except ValueError: | ||
| raise | ||
|
|
||
| _verify_model_has_usable_client(model, model_name, provider_name, kwargs) |
There was a problem hiding this comment.
ModelInitializationError swallowed by the outer retry loop
_verify_model_has_usable_client raises ModelInitializationError, but init_langchain_model wraps every try_initialization_method call in except Exception as e: last_exception = e, so the exception is silently stored and the loop advances to _init_community_chat_models. That initializer looks up providers in _chat_providers, which is populated from langchain_community.chat_models._module_lookup. AzureChatOpenAI is registered under the key azure_openai (last segment of langchain_community.chat_models.azure_openai), so for provider_name="azure_openai", the community initializer finds the class, instantiates it with the same empty credentials, and — because there is no equivalent _verify_model_has_usable_client call on that path — returns a broken model with client = None. init_langchain_model returns that model, the helpful ModelInitializationError is discarded, and the original NoneType attribute error resurfaces at first rail invocation.
Prompt To Fix With AI
This is a comment left during a code review.
Path: nemoguardrails/integrations/langchain/langchain_initializer.py
Line: 236
Comment:
**`ModelInitializationError` swallowed by the outer retry loop**
`_verify_model_has_usable_client` raises `ModelInitializationError`, but `init_langchain_model` wraps every `try_initialization_method` call in `except Exception as e: last_exception = e`, so the exception is silently stored and the loop advances to `_init_community_chat_models`. That initializer looks up providers in `_chat_providers`, which is populated from `langchain_community.chat_models._module_lookup`. `AzureChatOpenAI` is registered under the key `azure_openai` (last segment of `langchain_community.chat_models.azure_openai`), so for `provider_name="azure_openai"`, the community initializer finds the class, instantiates it with the same empty credentials, and — because there is no equivalent `_verify_model_has_usable_client` call on that path — returns a broken model with `client = None`. `init_langchain_model` returns that model, the helpful `ModelInitializationError` is discarded, and the original `NoneType` attribute error resurfaces at first rail invocation.
How can I resolve this? If you propose a fix, please make it concise.| if not hasattr(model, "client"): | ||
| return | ||
|
|
||
| client = getattr(model, "client", None) | ||
| async_client = getattr(model, "async_client", None) if hasattr(model, "async_client") else None | ||
| if client is not None or async_client is not None: | ||
| return |
There was a problem hiding this comment.
async_client check inadvertently fires for models with no async_client attribute
When model.client is None and the model has no async_client attribute at all (i.e. the provider only exposes a sync client path), the ternary getattr(model, "async_client", None) if hasattr(model, "async_client") else None returns None, and client is not None or async_client is not None evaluates to False, so ModelInitializationError is raised. Any LangChain integration that legitimately holds client = None during construction before finalising it in a model_post_init or validator hook, and that does not expose an async_client attribute, would be incorrectly rejected.
Prompt To Fix With AI
This is a comment left during a code review.
Path: nemoguardrails/integrations/langchain/langchain_initializer.py
Line: 260-266
Comment:
**`async_client` check inadvertently fires for models with no `async_client` attribute**
When `model.client` is `None` and the model has no `async_client` attribute at all (i.e. the provider only exposes a sync client path), the ternary `getattr(model, "async_client", None) if hasattr(model, "async_client") else None` returns `None`, and `client is not None or async_client is not None` evaluates to `False`, so `ModelInitializationError` is raised. Any LangChain integration that legitimately holds `client = None` during construction before finalising it in a `model_post_init` or validator hook, and that does not expose an `async_client` attribute, would be incorrectly rejected.
How can I resolve this? If you propose a fix, please make it concise.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
1 participant
Closes #1338.
When a content_safety rail is configured against an Azure model that fails to initialize because of a missing or blank api_key (or deployment_name), the rail later tried to call
.createon a None client and surfaced asLLMCallException: 'NoneType' object has no attribute 'create'. This change validates the model init at configuration time and raises a clear error pointing at the missing field, so users can fix the config without diffing tracebacks.The fix has three pieces. The LangChain initializer now verifies that the wrapper returned by
init_chat_modelhas a usable client; when an older provider integration swallows credential errors and returns a half-built wrapper whoseclientattribute is None, we surface that as aModelInitializationErrornaming the model, provider, and the credential-shaped kwargs that were missing or empty. The content_safety actions report which models actually loaded and call out the most common configuration causes when the requested model is absent. Finally,llm_callrecognizes theNoneTypeattribute error pattern and prepends an init-time hint to theLLMCallException, so any remaining regression of this class still produces a configuration-shaped message instead of a bare attribute error.Added regression tests covering the half-built Azure wrapper, the content_safety check input and output paths when the model is missing, and the
llm_callexception enrichment when a None client AttributeError is raised. The happy path is unchanged.Summary by CodeRabbit
Bug Fixes
Tests