feat(library): add F5 Guardrails integration#2105
Conversation
github-actions
Bot
added
size: L
status: needs triage
Signed-off-by: Ciaran Courtney <c.courtney@f5.com>
ciarancourtney
force-pushed
the
feat/f5-ai-guardrails
branch
from
e63e72d to
d3b3870
Compare
Pouyanpi
added
status: triaged
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| async with session.post(endpoint, headers=headers, json=payload) as response: | ||
| if response.status != 200: | ||
| error_detail = await response.text() | ||
| log.error(f"F5 Guardrails API call failed: {response.status} - {error_detail}") | ||
|
|
||
| if fail_open: | ||
| log.warning( | ||
| "F5 Guardrails API call failed, but F5_GUARDRAILS_FAIL_OPEN is enabled, allowing content.") | ||
| return {"result": {"outcome": "cleared"}} | ||
|
|
||
| raise RuntimeError(f"F5 Guardrails API error: {response.status}") | ||
|
|
||
| result = await response.json() | ||
| return result | ||
| except Exception as e: | ||
| log.error(f"Error connecting to F5 Guardrails API: {str(e)}") | ||
|
|
||
| if fail_open: | ||
| log.warning("F5 Guardrails API call failed, but F5_GUARDRAILS_FAIL_OPEN is enabled, allowing content.") | ||
| return {"result": {"outcome": "cleared"}} | ||
|
|
||
| if isinstance(e, aiohttp.ClientError): | ||
| raise RuntimeError(f"Connection error to F5 Guardrails API: {str(e)}") from e | ||
|
|
||
| raise e |
There was a problem hiding this comment.
RuntimeError re-caught causes double-logging and misleading messages
When response.status != 200 and fail_open=False, the RuntimeError raised on line 84 escapes the try block and is immediately caught by the outer except Exception as e on line 88. This causes two log entries: the first correct one ("F5 Guardrails API call failed: {status} - {detail}") and a second, misleading one saying "Error connecting to F5 Guardrails API: F5 Guardrails API error: {status}" — using "connecting" language for what is an HTTP status error. The fix is to use a narrower except aiohttp.ClientError instead of the broad except Exception, so that deliberate RuntimeError raises from the HTTP-error path propagate cleanly without being re-caught.
Prompt To Fix With AI
This is a comment left during a code review.
Path: nemoguardrails/library/f5/actions.py
Line: 74-98
Comment:
**`RuntimeError` re-caught causes double-logging and misleading messages**
When `response.status != 200` and `fail_open=False`, the `RuntimeError` raised on line 84 escapes the `try` block and is immediately caught by the outer `except Exception as e` on line 88. This causes two log entries: the first correct one (`"F5 Guardrails API call failed: {status} - {detail}"`) and a second, misleading one saying `"Error connecting to F5 Guardrails API: F5 Guardrails API error: {status}"` — using "connecting" language for what is an HTTP status error. The fix is to use a narrower `except aiohttp.ClientError` instead of the broad `except Exception`, so that deliberate `RuntimeError` raises from the HTTP-error path propagate cleanly without being re-caught.
How can I resolve this? If you propose a fix, please make it concise.| def test_f5_guardrails_fail_open(config, monkeypatch): | ||
| monkeypatch.setenv("F5_GUARDRAILS_API_KEY", "test-key") | ||
| monkeypatch.setenv("F5_GUARDRAILS_FAIL_OPEN", "true") | ||
| chat = TestChat( | ||
| config, | ||
| llm_completions=[ | ||
| " express greeting", | ||
| "Hello! How can I assist you today?", | ||
| ], | ||
| ) | ||
|
|
||
| with aioresponses() as m: | ||
| # Simulate an API error (e.g., 500) | ||
| m.post( | ||
| "https://us1.calypsoai.app/backend/v1/scans", | ||
| status=500, | ||
| ) | ||
|
|
||
| chat >> "Hello!" | ||
| chat << "express greeting" | ||
| chat << "Hello! How can I assist you today?" |
There was a problem hiding this comment.
fail_open test only mocks one of two required API calls
test_f5_guardrails_fail_open registers a single mock (status=500, no repeat=True), but the configured rails call the scan endpoint twice — once for the input rail and once for the output rail. The second call hits an unregistered URL in aioresponses, which raises an aiohttp.ClientConnectionError. This happens to be swallowed by the fail_open branch, so the test passes by accident rather than by design. Adding repeat=True (or registering a second mock) makes the test's intent explicit and avoids relying on this side-effect.
Prompt To Fix With AI
This is a comment left during a code review.
Path: tests/test_f5_guardrails.py
Line: 110-130
Comment:
**`fail_open` test only mocks one of two required API calls**
`test_f5_guardrails_fail_open` registers a single mock (status=500, no `repeat=True`), but the configured rails call the scan endpoint twice — once for the input rail and once for the output rail. The second call hits an unregistered URL in `aioresponses`, which raises an `aiohttp.ClientConnectionError`. This happens to be swallowed by the `fail_open` branch, so the test passes by accident rather than by design. Adding `repeat=True` (or registering a second mock) makes the test's intent explicit and avoids relying on this side-effect.
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! |
📝 WalkthroughWalkthroughAdds F5 AI Guardrails integration: a new ChangesF5 AI Guardrails Integration
Estimated code review effort: 2 (Simple) | ~15 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant Rails as NeMo Guardrails
participant Action as f5_guardrails_scan
participant F5API as F5 Guardrails API
participant LLM
User->>Rails: send message
Rails->>Action: scan input (user_message)
Action->>F5API: POST /backend/v1/scans
F5API-->>Action: outcome result
alt outcome not cleared
Action-->>Rails: blocked
Rails-->>User: refuse to respond
else outcome cleared
Rails->>LLM: generate response
LLM-->>Rails: bot_message
Rails->>Action: scan output (bot_message)
Action->>F5API: POST /backend/v1/scans
F5API-->>Action: outcome result
alt outcome not cleared
Action-->>Rails: blocked
Rails-->>User: refuse to respond
else outcome cleared
Rails-->>User: deliver response
end
end
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (4 passed)
✨ 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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
nemoguardrails/library/f5/flows.v1.co (1)
1-17: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winFix trailing newline (CI lint failure).
The
end-of-file-fixerpre-commit hook reports this file was modified; the file needs a proper trailing newline fixed before merge.🤖 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/library/f5/flows.v1.co` around lines 1 - 17, The file needs a proper trailing newline to satisfy the end-of-file-fixer lint check. Update the `nemoguardrails/library/f5/flows.v1.co` content so it ends with a single newline character after the last `stop` in the `f5 guardrails scan output` subflow, keeping the `define subflow` blocks unchanged.Source: Pipeline failures
♻️ Duplicate comments (1)
nemoguardrails/library/f5/flows.v1.co (1)
1-15: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winSame unhandled-exception risk as the Colang v2 flows.
Duplicates the same issue flagged in
nemoguardrails/library/f5/flows.co:f5_guardrails_scan(viaexecute) can raiseRuntimeErrorwhen fail-open is disabled and the API call fails, and this subflow has no handling for that case, so it would propagate as an unhandled error rather than triggeringbot refuse to respond/stop.🤖 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/library/f5/flows.v1.co` around lines 1 - 15, The F5 scan subflows in f5 guardrails scan input and f5 guardrails scan output call execute f5_guardrails_scan without handling RuntimeError when fail-open is disabled and the API call fails. Wrap the execute calls in these subflows with the same error handling used in the Colang v2 flow, so failures fall back to bot refuse to respond and stop instead of propagating as unhandled exceptions. Use the existing subflow names and the f5_guardrails_scan invocation points to place the fix.
🤖 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 `@docs/configure-rails/guardrail-catalog/community/f5-ai-guardrails.md`:
- Around line 20-24: The F5 Guardrails configuration docs are missing the
fail-open environment variable, so update the config variables list in the F5
Guardrails section to include F5_GUARDRAILS_FAIL_OPEN alongside
F5_GUARDRAILS_API_KEY and F5_GUARDRAILS_API_URL. Make sure the new entry clearly
states that it enables allow-through behavior when the API errors, using the
existing integration wording so readers can discover and set it correctly.
In `@nemoguardrails/library/f5/actions.py`:
- Around line 75-77: The F5 API failure logging in actions.py is writing the
full response body from response.text() into logs, which can expose sensitive
user or request data. Update the error handling in the F5 Guardrails API call
path to avoid logging raw error bodies: in the block around response.status and
error_detail, truncate, redact, or otherwise sanitize the logged detail before
passing it to log.error. Keep the log message useful for debugging, but ensure
any response content is safely limited and non-sensitive.
- Around line 73-98: The `send_to_f5_guardrails` flow is catching its own
deliberate `RuntimeError` from the non-200 response path and logging it as a
connection failure. Narrow the `except` in `actions.py` so it only handles
actual request/network errors from `session.post` (for example, the aiohttp
client exception path), and let the HTTP-status `RuntimeError` raised after the
`response.status != 200` check propagate without being re-caught; keep the
existing non-200 log in place and update the exception logging to use `e!s`
instead of `str(e)`.
In `@tests/test_f5_guardrails.py`:
- Around line 121-126: The mocked POST for the scans API is only consumed once,
so the second call in this test can fall through and hit an unmatched-route
error instead of the intended 500. Update the aioresponses stub in the test
around the scans POST so it repeats for both input and output scan calls by
enabling repeat behavior on that mock.
---
Outside diff comments:
In `@nemoguardrails/library/f5/flows.v1.co`:
- Around line 1-17: The file needs a proper trailing newline to satisfy the
end-of-file-fixer lint check. Update the `nemoguardrails/library/f5/flows.v1.co`
content so it ends with a single newline character after the last `stop` in the
`f5 guardrails scan output` subflow, keeping the `define subflow` blocks
unchanged.
---
Duplicate comments:
In `@nemoguardrails/library/f5/flows.v1.co`:
- Around line 1-15: The F5 scan subflows in f5 guardrails scan input and f5
guardrails scan output call execute f5_guardrails_scan without handling
RuntimeError when fail-open is disabled and the API call fails. Wrap the execute
calls in these subflows with the same error handling used in the Colang v2 flow,
so failures fall back to bot refuse to respond and stop instead of propagating
as unhandled exceptions. Use the existing subflow names and the
f5_guardrails_scan invocation points to place the fix.
🪄 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: 0ba8018f-b2f3-4b96-8d43-8f967f4d6465
📒 Files selected for processing (12)
docs/configure-rails/guardrail-catalog/community/f5-ai-guardrails.mdexamples/configs/f5_guardrails/README.mdexamples/configs/f5_guardrails/config.ymlexamples/configs/f5_guardrails_v2/README.mdexamples/configs/f5_guardrails_v2/config.ymlexamples/configs/f5_guardrails_v2/main.coexamples/configs/f5_guardrails_v2/rails.conemoguardrails/library/f5/__init__.pynemoguardrails/library/f5/actions.pynemoguardrails/library/f5/flows.conemoguardrails/library/f5/flows.v1.cotests/test_f5_guardrails.py
| The following environment variables can be used to configure the integration: | ||
|
|
||
| - `F5_GUARDRAILS_API_KEY`: The API key for the F5 Guardrails API. | ||
| - `F5_GUARDRAILS_API_URL`: The base URL for the F5 Guardrails API (defaults to https://us1.calypsoai.app). | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Document the fail-open toggle.
F5_GUARDRAILS_FAIL_OPEN is used by the integration but isn't documented here, so readers won't know how to enable the allow-through behavior on API errors.
Update the config variables list
- `F5_GUARDRAILS_API_URL`: The base URL for the F5 Guardrails API (defaults to https://us1.calypsoai.app).
+ `F5_GUARDRAILS_API_URL`: The base URL for the F5 Guardrails API (defaults to `https://us1.calypsoai.app`).
+ - `F5_GUARDRAILS_FAIL_OPEN`: When `true`, API failures are treated as cleared and the content is allowed through.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| The following environment variables can be used to configure the integration: | |
| - `F5_GUARDRAILS_API_KEY`: The API key for the F5 Guardrails API. | |
| - `F5_GUARDRAILS_API_URL`: The base URL for the F5 Guardrails API (defaults to https://us1.calypsoai.app). | |
| The following environment variables can be used to configure the integration: | |
| - `F5_GUARDRAILS_API_KEY`: The API key for the F5 Guardrails API. | |
| - `F5_GUARDRAILS_API_URL`: The base URL for the F5 Guardrails API (defaults to `https://us1.calypsoai.app`). | |
| - `F5_GUARDRAILS_FAIL_OPEN`: When `true`, API failures are treated as cleared and the content is allowed through. |
🤖 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 `@docs/configure-rails/guardrail-catalog/community/f5-ai-guardrails.md` around
lines 20 - 24, The F5 Guardrails configuration docs are missing the fail-open
environment variable, so update the config variables list in the F5 Guardrails
section to include F5_GUARDRAILS_FAIL_OPEN alongside F5_GUARDRAILS_API_KEY and
F5_GUARDRAILS_API_URL. Make sure the new entry clearly states that it enables
allow-through behavior when the API errors, using the existing integration
wording so readers can discover and set it correctly.
| try: | ||
| async with session.post(endpoint, headers=headers, json=payload) as response: | ||
| if response.status != 200: | ||
| error_detail = await response.text() | ||
| log.error(f"F5 Guardrails API call failed: {response.status} - {error_detail}") | ||
|
|
||
| if fail_open: | ||
| log.warning( | ||
| "F5 Guardrails API call failed, but F5_GUARDRAILS_FAIL_OPEN is enabled, allowing content.") | ||
| return {"result": {"outcome": "cleared"}} | ||
|
|
||
| raise RuntimeError(f"F5 Guardrails API error: {response.status}") | ||
|
|
||
| result = await response.json() | ||
| return result | ||
| except Exception as e: | ||
| log.error(f"Error connecting to F5 Guardrails API: {str(e)}") | ||
|
|
||
| if fail_open: | ||
| log.warning("F5 Guardrails API call failed, but F5_GUARDRAILS_FAIL_OPEN is enabled, allowing content.") | ||
| return {"result": {"outcome": "cleared"}} | ||
|
|
||
| if isinstance(e, aiohttp.ClientError): | ||
| raise RuntimeError(f"Connection error to F5 Guardrails API: {str(e)}") from e | ||
|
|
||
| raise e |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Deliberate RuntimeError (non-200 status) gets re-caught and mis-logged as a connection error.
The raise RuntimeError(...) at line 84 is raised inside the same try block guarded by except Exception as e (line 88). When fail_open is False and the API returns a non-200 status, this RuntimeError propagates into the except handler, which logs it as "Error connecting to F5 Guardrails API: ..." (line 89) — duplicating the earlier, more accurate log at line 77 and mischaracterizing an HTTP error response as a connection failure. Final behavior is unchanged (the error is re-raised via raise e), but the log output is confusing/misleading and complicates debugging. Also addressing Ruff RUF010: use {e!s} instead of {str(e)}.
🐛 Proposed fix: scope the except to actual network errors
timeout = aiohttp.ClientTimeout(total=30)
- async with aiohttp.ClientSession(timeout=timeout) as session:
- try:
+ try:
+ async with aiohttp.ClientSession(timeout=timeout) as session:
async with session.post(endpoint, headers=headers, json=payload) as response:
if response.status != 200:
error_detail = await response.text()
log.error(f"F5 Guardrails API call failed: {response.status} - {error_detail}")
if fail_open:
log.warning(
"F5 Guardrails API call failed, but F5_GUARDRAILS_FAIL_OPEN is enabled, allowing content.")
return {"result": {"outcome": "cleared"}}
raise RuntimeError(f"F5 Guardrails API error: {response.status}")
result = await response.json()
return result
- except Exception as e:
- log.error(f"Error connecting to F5 Guardrails API: {str(e)}")
-
- if fail_open:
- log.warning("F5 Guardrails API call failed, but F5_GUARDRAILS_FAIL_OPEN is enabled, allowing content.")
- return {"result": {"outcome": "cleared"}}
-
- if isinstance(e, aiohttp.ClientError):
- raise RuntimeError(f"Connection error to F5 Guardrails API: {str(e)}") from e
-
- raise e
+ except (aiohttp.ClientError, asyncio.TimeoutError) as e:
+ log.error(f"Error connecting to F5 Guardrails API: {e!s}")
+
+ if fail_open:
+ log.warning("F5 Guardrails API call failed, but F5_GUARDRAILS_FAIL_OPEN is enabled, allowing content.")
+ return {"result": {"outcome": "cleared"}}
+
+ raise RuntimeError(f"Connection error to F5 Guardrails API: {e!s}") from eRequires import asyncio at the top of the file.
🧰 Tools
🪛 Ruff (0.15.20)
[warning] 89-89: Use explicit conversion flag
Replace with conversion flag
(RUF010)
[warning] 96-96: Use explicit conversion flag
Replace with conversion flag
(RUF010)
🤖 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/library/f5/actions.py` around lines 73 - 98, The
`send_to_f5_guardrails` flow is catching its own deliberate `RuntimeError` from
the non-200 response path and logging it as a connection failure. Narrow the
`except` in `actions.py` so it only handles actual request/network errors from
`session.post` (for example, the aiohttp client exception path), and let the
HTTP-status `RuntimeError` raised after the `response.status != 200` check
propagate without being re-caught; keep the existing non-200 log in place and
update the exception logging to use `e!s` instead of `str(e)`.
Source: Linters/SAST tools
| if response.status != 200: | ||
| error_detail = await response.text() | ||
| log.error(f"F5 Guardrails API call failed: {response.status} - {error_detail}") |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Logging raw API error body may leak sensitive content.
error_detail = await response.text() is logged verbatim. If the F5 API echoes the submitted text (or account/request metadata) in its error response, this writes potentially sensitive user content into application logs.
🛡️ Proposed fix: truncate/redact logged error detail
if response.status != 200:
error_detail = await response.text()
- log.error(f"F5 Guardrails API call failed: {response.status} - {error_detail}")
+ log.error(f"F5 Guardrails API call failed with status {response.status}")
+ log.debug(f"F5 Guardrails API error detail: {error_detail[:200]}")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if response.status != 200: | |
| error_detail = await response.text() | |
| log.error(f"F5 Guardrails API call failed: {response.status} - {error_detail}") | |
| if response.status != 200: | |
| error_detail = await response.text() | |
| log.error(f"F5 Guardrails API call failed with status {response.status}") | |
| log.debug(f"F5 Guardrails API error detail: {error_detail[:200]}") |
🤖 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/library/f5/actions.py` around lines 75 - 77, The F5 API
failure logging in actions.py is writing the full response body from
response.text() into logs, which can expose sensitive user or request data.
Update the error handling in the F5 Guardrails API call path to avoid logging
raw error bodies: in the block around response.status and error_detail,
truncate, redact, or otherwise sanitize the logged detail before passing it to
log.error. Keep the log message useful for debugging, but ensure any response
content is safely limited and non-sensitive.
| with aioresponses() as m: | ||
| # Simulate an API error (e.g., 500) | ||
| m.post( | ||
| "https://us1.calypsoai.app/backend/v1/scans", | ||
| status=500, | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the test file and nearby related tests
git ls-files tests/test_f5_guardrails.py
# Show the relevant section with line numbers
sed -n '1,220p' tests/test_f5_guardrails.py | cat -n
# Search for related scan mock patterns in the same file
rg -n "repeat=True|backend/v1/scans|aioresponses|fail-open|500" tests/test_f5_guardrails.pyRepository: NVIDIA-NeMo/Guardrails
Length of output: 5400
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the F5 guardrails implementation and the test helper used here.
rg -n "f5 guardrails scan input|f5 guardrails scan output|backend/v1/scans|fail_open|F5_GUARDRAILS_FAIL_OPEN" -S nemoguardrails tests
# Map the relevant test helper and implementation files before reading them.
fd -a "test_chat.py|f5|guardrails" nemoguardrails tests
# If aioresponses is installed, inspect its route-matching semantics directly.
python3 - <<'PY'
try:
import aioresponses
import inspect
from aioresponses import core
print("aioresponses module:", aioresponses.__file__)
print("core module:", core.__file__)
print("aioresponses.Aioresponses source excerpt:")
src = inspect.getsource(aioresponses.aioresponses)
for i, line in enumerate(src.splitlines(), 1):
if "repeat" in line or "match" in line or "assert" in line:
print(f"{i}: {line}")
except Exception as e:
print("IMPORT_ERROR:", repr(e))
PYRepository: NVIDIA-NeMo/Guardrails
Length of output: 7399
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the F5 action implementation with line numbers.
sed -n '1,220p' nemoguardrails/library/f5/actions.py | cat -n
# Inspect the related flow definitions to confirm both input and output scan hooks.
sed -n '1,120p' nemoguardrails/library/f5/flows.co | cat -n
sed -n '1,120p' nemoguardrails/library/f5/flows.v1.co | cat -n
# Check whether the repo pins aioresponses and, if so, the version.
rg -n "aioresponses" pyproject.toml poetry.lock requirements*.txt setup.cfg setup.pyRepository: NVIDIA-NeMo/Guardrails
Length of output: 6211
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the TestChat helper.
fd -a "utils.py" tests
rg -n "class TestChat|def __rshift__|def __lshift__|send_message|llm_completions" tests/utils.py tests -S
# Read the helper file if present.
sed -n '1,260p' tests/utils.py | cat -nRepository: NVIDIA-NeMo/Guardrails
Length of output: 47526
🌐 Web query:
aioresponses default behavior single mocked request consumed once repeat=False unmatched route raises connection error documentation
💡 Result:
In aioresponses, the default behavior for a mocked request is to be consumed exactly once [1][2]. This is controlled by the repeat parameter, which defaults to False (equivalent to repeat=1) [1][3]. If you need to mock a request that should be handled multiple times, you must explicitly set the repeat argument [1][4]: * repeat=n: Repeats the response n times [1][3]. * repeat=True: Repeats the response indefinitely [1][3]. Regarding unmatched routes, aioresponses defaults to raising an aiohttp.client_exceptions.ClientConnectionError when a request is made that does not match any configured mock [5]. To change this behavior and allow unmatched requests to proceed, you can initialize aioresponses with the passthrough_unmatched=True flag [6]: @aioresponses(passthrough_unmatched=True) def test_passthrough(m): # Requests not matching defined mocks will now be allowed to pass through... Alternatively, you can provide a list of URLs to the passthrough argument if you only want to allow specific requests to bypass the mocking system [6].
Citations:
- 1: https://github.com/pnuckowski/aioresponses/blob/master/README.rst
- 2: https://github.com/pnuckowski/aioresponses
- 3: https://pypi.org/project/aioresponses/
- 4: Send a mocked request more than once then got ClientConnectionError pnuckowski/aioresponses#164
- 5: https://github.com/pnuckowski/aioresponses/blob/master/tests/test_aioresponses.py
- 6: https://github.com/pnuckowski/aioresponses/
Make the 500 mock repeatable here
This test hits both input and output scans, but aioresponses consumes the mocked POST only once by default. The second scan can fall through to an unmatched-route connection error, so the fail-open path may pass without actually exercising a second 500. Add repeat=True to keep both calls on the intended response.
🤖 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 `@tests/test_f5_guardrails.py` around lines 121 - 126, The mocked POST for the
scans API is only consumed once, so the second call in this test can fall
through and hit an unmatched-route error instead of the intended 500. Update the
aioresponses stub in the test around the scans POST so it repeats for both input
and output scan calls by enabling repeat behavior on that mock.
| timeout = aiohttp.ClientTimeout(total=30) | ||
| async with aiohttp.ClientSession(timeout=timeout) as session: | ||
| try: | ||
| async with session.post(endpoint, headers=headers, json=payload) as response: | ||
| if response.status != 200: | ||
| error_detail = await response.text() | ||
| log.error(f"F5 Guardrails API call failed: {response.status} - {error_detail[:200]}") | ||
|
|
||
| if fail_open: | ||
| log.warning( | ||
| "F5 Guardrails API call failed, but F5_GUARDRAILS_FAIL_OPEN is enabled, allowing content.") | ||
| return {"result": {"outcome": "cleared"}} | ||
|
|
||
| raise RuntimeError(f"F5 Guardrails API error: {response.status}") | ||
|
|
||
| result = await response.json() | ||
| return result | ||
| except aiohttp.ClientError as e: | ||
| log.error(f"Error connecting to F5 Guardrails API: {str(e)}") | ||
|
|
||
| if fail_open: | ||
| log.warning("F5 Guardrails API call failed, but F5_GUARDRAILS_FAIL_OPEN is enabled, allowing content.") | ||
| return {"result": {"outcome": "cleared"}} | ||
|
|
||
| raise RuntimeError(f"Connection error to F5 Guardrails API: {str(e)}") from e |
There was a problem hiding this comment.
fail_open silently fails on request timeout
aiohttp raises asyncio.TimeoutError (not a subclass of aiohttp.ClientError) when the ClientTimeout.total of 30 seconds is exceeded. The except aiohttp.ClientError block therefore does not catch it, so a slow API will cause the exception to propagate uncaught regardless of whether fail_open=True was set. Another integration in this codebase — nemoguardrails/library/jailbreak_detection/request.py line 146 — catches asyncio.TimeoutError explicitly for exactly this reason. Add a separate except asyncio.TimeoutError handler with the same fail-open / raise logic applied to aiohttp.ClientError.
Prompt To Fix With AI
This is a comment left during a code review.
Path: nemoguardrails/library/f5/actions.py
Line: 71-95
Comment:
**Timeout not caught — `fail_open` silently fails on request timeout**
`aiohttp` raises `asyncio.TimeoutError` (not a subclass of `aiohttp.ClientError`) when the `ClientTimeout.total` of 30 seconds is exceeded. The `except aiohttp.ClientError` block therefore does not catch it, so a slow API will cause the exception to propagate uncaught regardless of whether `fail_open=True` was set. Another integration in this codebase — `nemoguardrails/library/jailbreak_detection/request.py` line 146 — catches `asyncio.TimeoutError` explicitly for exactly this reason. Add a separate `except asyncio.TimeoutError` handler with the same fail-open / raise logic applied to `aiohttp.ClientError`.
How can I resolve this? If you propose a fix, please make it concise.NVIDIA-NeMo#2105 (comment) ``` <a href="#"><img alt="P1" src="https://greptile-static-assets.s3.amazonaws.com/badges/p1.svg?v=9" align="top"></a> **Timeout not caught — `fail_open` silently fails on request timeout** `aiohttp` raises `asyncio.TimeoutError` (not a subclass of `aiohttp.ClientError`) when the `ClientTimeout.total` of 30 seconds is exceeded. The `except aiohttp.ClientError` block therefore does not catch it, so a slow API will cause the exception to propagate uncaught regardless of whether `fail_open=True` was set. Another integration in this codebase — `nemoguardrails/library/jailbreak_detection/request.py` line 146 — catches `asyncio.TimeoutError` explicitly for exactly this reason. Add a separate `except asyncio.TimeoutError` handler with the same fail-open / raise logic applied to `aiohttp.ClientError`. <details><summary>Prompt To Fix With AI</summary> `````markdown This is a comment left during a code review. Path: nemoguardrails/library/f5/actions.py Line: 71-95 Comment: **Timeout not caught — `fail_open` silently fails on request timeout** `aiohttp` raises `asyncio.TimeoutError` (not a subclass of `aiohttp.ClientError`) when the `ClientTimeout.total` of 30 seconds is exceeded. The `except aiohttp.ClientError` block therefore does not catch it, so a slow API will cause the exception to propagate uncaught regardless of whether `fail_open=True` was set. Another integration in this codebase — `nemoguardrails/library/jailbreak_detection/request.py` line 146 — catches `asyncio.TimeoutError` explicitly for exactly this reason. Add a separate `except asyncio.TimeoutError` handler with the same fail-open / raise logic applied to `aiohttp.ClientError`. How can I resolve this? If you propose a fix, please make it concise. ````` </details> ```
2 participants
Description
Adds a new community integration for F5 AI Guardrails
Checklist
Summary by CodeRabbit
New Features
Documentation
Tests