Skip to content

feat(guardrails): log the provider error-response body on a config-error 4xx#773

Merged
jarvis9443 merged 2 commits into
mainfrom
fix/guardrail-log-error-response-body
Jul 15, 2026
Merged

feat(guardrails): log the provider error-response body on a config-error 4xx#773
jarvis9443 merged 2 commits into
mainfrom
fix/guardrail-log-error-response-body

Conversation

@jarvis9443

@jarvis9443 jarvis9443 commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

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_error on every request with just

ERROR aisix_guardrails::aliyun: aliyun TextModerationPlus returned 4xx — check region/access keys configuration row=aliyun http_status=404

http_status=404 alone 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).

  • 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 (e.g. an access key lacking llm_response_moderation permission).

Behavior / compatibility

  • Logging/observability only — no guardrail decision behavior changes. Failure classification (ConfigError/ServerError/Throttled) and fail-open/closed handling are unchanged; the existing per-guardrail wiremock tests stay green.
  • The body is read on the error paths only; the success path is untouched (aliyun reads the body once and parses from it, equivalent to the previous resp.json()).

Tests

  • truncate_error_body_for_log unit 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 carries response_body and 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-level Code:403 with a Message; asserts the log echoes it.
  • The other five guardrails get the identical mechanical change through the shared helper (covered representatively by the aliyun capture test) — not duplicated per kind.

No user-facing config/behavior surface, so no api7/docs change.

Summary by CodeRabbit

  • Bug Fixes

    • Improved diagnostic logging for guardrail service errors by including a safely truncated response message.
    • Preserved existing error handling and response behavior for failed requests.
    • Ensured logged error details remain within a safe size limit and do not split multibyte characters.
  • Tests

    • Added coverage for short, oversized, and UTF-8 error messages.

…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.
@coderabbitai

coderabbitai Bot commented Jul 15, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 41 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 04a73d08-4128-40a8-9862-6d04df5ecff9

📥 Commits

Reviewing files that changed from the base of the PR and between 63568d0 and a487bf0.

📒 Files selected for processing (7)
  • crates/aisix-guardrails/src/aliyun.rs
  • crates/aisix-guardrails/src/lakera.rs
  • crates/aisix-guardrails/src/lib.rs
  • crates/aisix-guardrails/src/openai_moderation.rs
  • crates/aisix-guardrails/src/presidio.rs
  • crates/aisix-guardrails/src/prompt_shield.rs
  • crates/aisix-guardrails/src/text_moderation.rs
📝 Walkthrough

Walkthrough

Guardrail 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.

Changes

Guardrail error logging

Layer / File(s) Summary
Error-body truncation contract
crates/aisix-guardrails/src/lib.rs, crates/aisix-guardrails/Cargo.toml
Adds a 2048-byte UTF-8-safe truncation helper, unit tests, and the workspace tracing subscriber for development builds.
Provider 4xx logging integration
crates/aisix-guardrails/src/{lakera,openai_moderation,presidio,prompt_shield,text_moderation}.rs
Reads response bodies in non-429 4xx paths and adds truncated response_body fields to error logs while preserving existing error returns.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

  • api7/aisix#730: Touches the same guardrail implementations and their 4xx request error handling.

Suggested reviewers: moonming


Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (1 error, 1 warning)

Check name Status Explanation Resolution
Security Check ❌ Error 4xx handlers now log raw upstream response bodies (response_body) in multiple guardrails, which can expose PII/secrets in logs. Remove raw body logging or redacted parse only safe fields; keep status/code and correlation IDs, not full provider error bodies.
E2e Test Quality Review ⚠️ Warning Only module-level wiremock tests were added; there’s no API→service E2E coverage, and the new 4xx path still uses unbounded resp.text() reads. Add a true end-to-end test through the public request path that hits a mocked 4xx provider, and bound body reads before logging them.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: logging provider error-response bodies for config-error 4xx cases.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/guardrail-log-error-response-body

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between 30829d6 and 63568d0.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (7)
  • crates/aisix-guardrails/Cargo.toml
  • crates/aisix-guardrails/src/lakera.rs
  • crates/aisix-guardrails/src/lib.rs
  • crates/aisix-guardrails/src/openai_moderation.rs
  • crates/aisix-guardrails/src/presidio.rs
  • crates/aisix-guardrails/src/prompt_shield.rs
  • crates/aisix-guardrails/src/text_moderation.rs

Comment thread crates/aisix-guardrails/src/lakera.rs Outdated
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).
@jarvis9443 jarvis9443 merged commit 812fc72 into main Jul 15, 2026
12 checks passed
@jarvis9443 jarvis9443 deleted the fix/guardrail-log-error-response-body branch July 15, 2026 08:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant