feat(guardrails): log the provider error-response body on a config-error 4xx#773
Conversation
…ror 4xx When a cloud-safety guardrail provider answered a non-2xx, we logged only the bare HTTP status and dropped the response body. A customer hit `aliyun_config_error` on every request with just `http_status=404` in the log — not enough to tell a wrong access key from a wrong region/endpoint, since the actual reason lives in the provider's response body. Echo the (truncated) response body into the error log across the whole cloud-safety guardrail family — aliyun, lakera, openai_moderation, presidio, and both Azure Content Safety kinds (prompt_shield, text_moderation) — which all shared the identical blind spot. A new shared helper `truncate_error_body_for_log` caps the snippet at 2048 bytes (nginx's `NGX_MAX_ERROR_STR`) on a UTF-8 char boundary so a verbose HTML error page can't blow up the log. Aliyun also signals auth/permission failures as an app-level `Code` on an HTTP 200; that branch now logs the Aliyun `Message` too (previously only the numeric code), so the operator sees the human reason. Logging/observability only — no guardrail decision behavior changes.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 41 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughGuardrail providers now capture truncated HTTP response bodies in non-429 4xx error logs. A shared UTF-8-safe 2048-byte truncation helper and unit tests support this behavior. ChangesGuardrail error logging
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
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 `@crates/aisix-guardrails/src/lakera.rs`:
- Around line 170-174: Replace the unbounded resp.text() error-body reads with a
bounded, deadline-aware read in the shared error logging paths for Lakera,
OpenAI moderation, Presidio, Prompt Shield, and text moderation. Apply the same
guardrail pattern at crates/aisix-guardrails/src/lakera.rs:170-174,
crates/aisix-guardrails/src/openai_moderation.rs:164-168,
crates/aisix-guardrails/src/presidio.rs:219-224,
crates/aisix-guardrails/src/prompt_shield.rs:177-181, and
crates/aisix-guardrails/src/text_moderation.rs:176-180, while preserving
truncation for the resulting logged body.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3b38efad-de02-4e4a-84a8-d29726519905
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
crates/aisix-guardrails/Cargo.tomlcrates/aisix-guardrails/src/lakera.rscrates/aisix-guardrails/src/lib.rscrates/aisix-guardrails/src/openai_moderation.rscrates/aisix-guardrails/src/presidio.rscrates/aisix-guardrails/src/prompt_shield.rscrates/aisix-guardrails/src/text_moderation.rs
Two fixes on top of the initial commit: 1. Aliyun was missing from the first commit — restore its share of the feature: the transport-4xx log now echoes the (capped) response body, and Aliyun's app-level error branch (HTTP 200 with an error `Code`) logs the `Message` too. This is the guardrail the customer actually hit. 2. Bound the error-body read (CodeRabbit): instead of reading the whole body with `resp.text()` and truncating, a shared `read_error_body_capped` helper stops after MAX_ERROR_BODY_LOG_BYTES so a broken provider returning a huge 4xx body can't make us buffer it all. Applied across aliyun, lakera, openai_moderation, presidio, prompt_shield, and text_moderation. Tests: helper unit tests plus a consolidated aliyun capture test covering the transport-4xx reason, the size cap, and the app-level Message (kept in one test so the thread-local capture subscriber isn't raced by a parallel sibling).
Problem
When a cloud-safety guardrail provider returned a non-2xx, we logged only the bare HTTP status and dropped the response body. Follow-up to AISIX-Cloud#1030: a customer's Aliyun guardrail hit
aliyun_config_erroron every request with justhttp_status=404alone can't distinguish a wrong access key from a wrong region/endpoint — the actual reason lives in the provider's response body, which we discarded.Change
Echo the (truncated) response body into the error log across the whole cloud-safety guardrail family that shared this blind spot:
aliyun,lakera,openai_moderation,presidio, and both Azure Content Safety kinds (prompt_shield,text_moderation).truncate_error_body_for_logcaps the snippet at 2048 bytes (nginx'sNGX_MAX_ERROR_STR) on a UTF-8 char boundary, so a verbose HTML error page can't blow up the log.Codeon an HTTP 200; that branch now logs the AliyunMessagetoo (previously only the numeric code), so the operator sees the human reason (e.g. an access key lackingllm_response_moderationpermission).Behavior / compatibility
ConfigError/ServerError/Throttled) and fail-open/closed handling are unchanged; the existing per-guardrail wiremock tests stay green.resp.json()).Tests
truncate_error_body_for_logunit tests: short/at-cap passthrough, byte cap, and never splitting a multi-byte char.aliyun::tests::transport_4xx_logs_response_body_for_diagnosis: a mock 404 with a JSON error body; asserts the captured log carriesresponse_bodyand the provider's reason. Verified to fail before the change and pass after.aliyun::tests::app_level_error_logs_aliyun_message: an HTTP-200 app-levelCode:403with aMessage; asserts the log echoes it.No user-facing config/behavior surface, so no
api7/docschange.Summary by CodeRabbit
Bug Fixes
Tests