fix(aiguard): handle empty AI Guard response on error#16556
fix(aiguard): handle empty AI Guard response on error#16556
Conversation
Codeowners resolved as |
Performance SLOsComparing candidate dd/fix/ai-guard-empty-response (147af46) with baseline main (b5b77a2) 📈 Performance Regressions (1 suite)📈 iastaspectsospath - 24/24✅ ospathbasename_aspectTime: ✅ 513.729µs (SLO: <700.000µs 📉 -26.6%) vs baseline: 📈 +20.5% Memory: ✅ 42.762MB (SLO: <46.000MB -7.0%) vs baseline: +5.2% ✅ ospathbasename_noaspectTime: ✅ 433.286µs (SLO: <700.000µs 📉 -38.1%) vs baseline: +0.8% Memory: ✅ 42.585MB (SLO: <46.000MB -7.4%) vs baseline: +5.0% ✅ ospathjoin_aspectTime: ✅ 628.105µs (SLO: <700.000µs 📉 -10.3%) vs baseline: ~same Memory: ✅ 42.507MB (SLO: <46.000MB -7.6%) vs baseline: +4.8% ✅ ospathjoin_noaspectTime: ✅ 634.615µs (SLO: <700.000µs -9.3%) vs baseline: +0.5% Memory: ✅ 42.625MB (SLO: <46.000MB -7.3%) vs baseline: +5.0% ✅ ospathnormcase_aspectTime: ✅ 348.763µs (SLO: <700.000µs 📉 -50.2%) vs baseline: -0.4% Memory: ✅ 42.605MB (SLO: <46.000MB -7.4%) vs baseline: +4.8% ✅ ospathnormcase_noaspectTime: ✅ 357.245µs (SLO: <700.000µs 📉 -49.0%) vs baseline: -1.0% Memory: ✅ 42.546MB (SLO: <46.000MB -7.5%) vs baseline: +4.5% ✅ ospathsplit_aspectTime: ✅ 492.273µs (SLO: <700.000µs 📉 -29.7%) vs baseline: -0.7% Memory: ✅ 42.703MB (SLO: <46.000MB -7.2%) vs baseline: +5.1% ✅ ospathsplit_noaspectTime: ✅ 500.661µs (SLO: <700.000µs 📉 -28.5%) vs baseline: +0.6% Memory: ✅ 42.526MB (SLO: <46.000MB -7.6%) vs baseline: +4.7% ✅ ospathsplitdrive_aspectTime: ✅ 378.406µs (SLO: <700.000µs 📉 -45.9%) vs baseline: +0.9% Memory: ✅ 42.585MB (SLO: <46.000MB -7.4%) vs baseline: +4.9% ✅ ospathsplitdrive_noaspectTime: ✅ 73.032µs (SLO: <700.000µs 📉 -89.6%) vs baseline: ~same Memory: ✅ 42.664MB (SLO: <46.000MB -7.3%) vs baseline: +5.3% ✅ ospathsplitext_aspectTime: ✅ 455.548µs (SLO: <700.000µs 📉 -34.9%) vs baseline: ~same Memory: ✅ 42.664MB (SLO: <46.000MB -7.3%) vs baseline: +5.1% ✅ ospathsplitext_noaspectTime: ✅ 466.414µs (SLO: <700.000µs 📉 -33.4%) vs baseline: ~same Memory: ✅ 42.743MB (SLO: <46.000MB -7.1%) vs baseline: +5.1% ✅ All Tests Passing (2 suites)✅ iastaspectssplit - 12/12✅ rsplit_aspectTime: ✅ 155.769µs (SLO: <250.000µs 📉 -37.7%) vs baseline: +3.4% Memory: ✅ 42.644MB (SLO: <46.000MB -7.3%) vs baseline: +5.1% ✅ rsplit_noaspectTime: ✅ 157.560µs (SLO: <250.000µs 📉 -37.0%) vs baseline: ~same Memory: ✅ 42.546MB (SLO: <46.000MB -7.5%) vs baseline: +4.6% ✅ split_aspectTime: ✅ 148.557µs (SLO: <250.000µs 📉 -40.6%) vs baseline: -0.7% Memory: ✅ 42.625MB (SLO: <46.000MB -7.3%) vs baseline: +5.1% ✅ split_noaspectTime: ✅ 154.135µs (SLO: <250.000µs 📉 -38.3%) vs baseline: +0.6% Memory: ✅ 42.526MB (SLO: <46.000MB -7.6%) vs baseline: +4.4% ✅ splitlines_aspectTime: ✅ 148.415µs (SLO: <250.000µs 📉 -40.6%) vs baseline: +1.9% Memory: ✅ 42.507MB (SLO: <46.000MB -7.6%) vs baseline: +4.5% ✅ splitlines_noaspectTime: ✅ 150.372µs (SLO: <250.000µs 📉 -39.9%) vs baseline: -0.6% Memory: ✅ 42.566MB (SLO: <46.000MB -7.5%) vs baseline: +4.9% ✅ iastpropagation - 8/8✅ no-propagationTime: ✅ 48.128µs (SLO: <60.000µs 📉 -19.8%) vs baseline: -1.0% Memory: ✅ 39.007MB (SLO: <42.000MB -7.1%) vs baseline: +5.0% ✅ propagation_enabledTime: ✅ 136.307µs (SLO: <190.000µs 📉 -28.3%) vs baseline: +0.7% Memory: ✅ 38.889MB (SLO: <42.000MB -7.4%) vs baseline: +5.0% ✅ propagation_enabled_100Time: ✅ 1.564ms (SLO: <2.300ms 📉 -32.0%) vs baseline: -0.2% Memory: ✅ 39.184MB (SLO: <42.000MB -6.7%) vs baseline: +5.7% ✅ propagation_enabled_1000Time: ✅ 29.206ms (SLO: <34.550ms 📉 -15.5%) vs baseline: +0.4% Memory: ✅ 39.027MB (SLO: <42.000MB -7.1%) vs baseline: +4.9% ℹ️ Scenarios Missing SLO Configuration (20 scenarios)The following scenarios exist in candidate data but have no SLO thresholds configured:
|
24f2b44 to
7245eb5
Compare
This comment has been minimized.
This comment has been minimized.
7245eb5 to
147af46
Compare
|
@cursor review |
|
@codex review |
|
Codex Review: Didn't find any major issues. Bravo. ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
There was a problem hiding this comment.
Pull request overview
This PR fixes a TypeError that occurs when AI Guard service responses return empty or unparsable JSON bodies. The fix defaults the result to an empty dictionary when get_json() returns None, allowing existing error handling logic to properly surface client errors instead of crashing on NoneType operations.
Changes:
- Modified
_api_client.pyto useresponse.get_json() or {}pattern to safely handle None responses - Added test case to verify HTTP error handling with empty JSON body
- Added release note documenting the TypeError fix
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| ddtrace/appsec/ai_guard/_api_client.py | Changed response parsing to default to empty dict when get_json() returns None |
| tests/appsec/ai_guard/api/test_api_client.py | Added test for HTTP error (status 500) with empty JSON body |
| releasenotes/notes/fix-ai-guard-empty-result-9ceb9de92d746a86.yaml | Added release note documenting the TypeError fix |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def test_evaluate_http_error_empty_json_body(mock_execute_request, telemetry_mock, ai_guard_client): | ||
| """Test HTTP error handling when the response body is empty.""" | ||
| mock_response = Mock() | ||
| mock_response.status = 500 | ||
| mock_response.get_json.return_value = None | ||
| mock_execute_request.return_value = mock_response | ||
|
|
||
| with pytest.raises(AIGuardClientError) as exc_info: | ||
| ai_guard_client.evaluate(TOOL_CALL) | ||
|
|
||
| assert str(exc_info.value) == "AI Guard service call failed, status: 500" | ||
| assert exc_info.value.status == 500 | ||
| assert exc_info.value.errors == [] | ||
| assert_telemetry(telemetry_mock, "ai_guard.requests", (("error", "true"),)) |
There was a problem hiding this comment.
The test only covers the case when status is 500 with an empty JSON body. Consider adding a test case for status 200 with an empty JSON body to ensure that the malformed response error handler works correctly in this scenario. While the existing try-except block at line 250 in the source code should handle this case properly, having an explicit test would provide better coverage and documentation of this edge case.
Description
Handle AI Guard evaluate responses that return empty or unparsable JSON by defaulting to an empty result and letting existing error handling surface a clear client error instead of raising a TypeError. This prevents NoneType checks from crashing and keeps error reporting consistent when the backend returns no body.
Fixes APPSEC-61354, a bug observed in the wild, possibly on a network error.
Testing
None
Risks
Low. Change only affects error handling when the response body is empty or unparsable.
Additional Notes
None
PR by Bits
View session in Datadog
Comment @DataDog to request changes