ENG-3255: Include response body in ClientUnsuccessfulException message

[dsill-ethyca]

Ticket ENG-3255

Description Of Changes

When a SaaS connector request fails with a non-2xx HTTP response, ClientUnsuccessfulException only includes the status code in its message (e.g., "Client call failed with status code '400'"). The actual response body — which contains actionable diagnostic details like FUNCTIONALITY_NOT_ENABLED, INVALID_TYPE, or MALFORMED_QUERY — is stored on the exception object but discarded when the exception is converted to a string via str(e).

This makes discovery monitor scan failures and connection test failures very difficult to diagnose. For example, a Salesforce monitor scan failure only shows "Client call failed with status code '400'" in logs, when the response body contains [{"message":"sObject type 'CustomObject' is not supported.","errorCode":"INVALID_TYPE"}].

This PR modifies ClientUnsuccessfulException.__init__ to append a truncated response body (max 500 chars) to the exception message, so str(e) carries diagnostic detail through all log layers.

Code Changes

• Modified ClientUnsuccessfulException.__init__ in common_exceptions.py to extract and append response.text[:500] to the message, with graceful fallback for None responses or missing .text attributes
• Updated 4 test assertions in test_http_connector.py and test_http_oauth_connector.py from exact string match to startswith checks
• Added 5 new unit tests for: response body inclusion, truncation at 500 chars, no-response regression, empty text, and missing text attribute

Steps to Confirm

1. Trigger a SaaS connector failure that returns a non-2xx response with a body (e.g., Salesforce Tooling API with an invalid query)
2. Verify the error log now includes the response body text after the status code
3. Verify that exceptions without a response (e.g., connection errors) still show only the status code message

Validated locally by triggering a Salesforce monitor scan with an invalid Tooling API query — confirmed the response body (INVALID_TYPE error) now appears in all log layers.

Pre-Merge Checklist

• Issue requirements met
• All CI pipelines succeeded
• CHANGELOG.md updated
  • Updates unreleased work already in Changelog, no new entry necessary
• UX feedback:
  • No UX review needed
• Followup issues:
  • No followup issues
• Database migrations:
  • No migrations
• Documentation:
  • No documentation updates required

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments

Project	Deployment	Actions	Updated (UTC)
fides-plus-nightly	Ignored	Preview	Apr 9, 2026 6:14pm
fides-privacy-center	Ignored		Apr 9, 2026 6:14pm

[codecov]

Codecov Report

❌ Patch coverage is 77.77778% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.07%. Comparing base (7192e68) to head (4d4f590).
⚠️ Report is 15 commits behind head on main.

Files with missing lines	Patch %	Lines
src/fides/api/common_exceptions.py	77.77%	2 Missing ⚠️

❌ Your patch status has failed because the patch coverage (77.77%) is below the target coverage (100.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7875      +/-   ##
==========================================
- Coverage   85.07%   85.07%   -0.01%     
==========================================
  Files         627      627              
  Lines       40780    40788       +8     
  Branches     4742     4743       +1     
==========================================
+ Hits        34694    34700       +6     
- Misses       5017     5019       +2     
  Partials     1069     1069              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[claude]

Code Review: Include response body in ClientUnsuccessfulException message

The core approach is sound — appending a truncated response body to the exception message improves diagnostic visibility for connector failures. The new unit tests are well-structured and cover the important cases. Two issues should be addressed before merge:

Issues to fix

1. http_connector.py line 101 — improvement is never triggered for that connector path

ClientUnsuccessfulException is raised at line 101 of http_connector.py without passing response=response, even though the response object is in scope. This means the new body-inclusion logic only applies to SaaS/AuthenticatedClient paths where response=response is already correctly forwarded — the HTTP connector path (the one that already logs Pii(response.text) at line 90) gets no benefit. See inline comment.

2. Response body ends up in DB-persisted error messages without PII protection

response.text is embedded raw into the exception message string. Call sites like request_runner_service.py line 1271–1283 build an error_message from exc.args[0] and persist it to the DB via add_error_execution_log. The Pii(str(exc.args[0])) wrapper on the log call no longer functions as a guard because the sensitive body is already baked into the string before Pii sees it. This is inconsistent with how the existing http_connector.py path handles the same body (wrapping it in Pii() before logging). An explicit decision is needed about whether storing truncated external response bodies in execution_log.message is acceptable. See inline comment.

Minor suggestions

• Bare except Exception: pass — narrowing to except (AttributeError, TypeError, UnicodeDecodeError) communicates the actual failure modes being guarded and lets unexpected errors surface. See inline comment.
• Test parametrize structure — the no_response case uses None as a sentinel and branches inside the test body; parametrizing the response argument directly would be slightly cleaner. Minor readability nit. See inline comment.
• startswith assertions in updated tests — the loosening is correct, but the tests in test_http_oauth_connector.py could additionally assert that the mocked response body appears in the message, giving regression coverage for the actual body-inclusion path end-to-end.

[claude]

Response bodies from external connectors are treated as potentially PII-sensitive elsewhere in the codebase — for example, http_connector.py wraps response.text in Pii(...) before logging (line 90 of that file).

By embedding response.text directly into the exception message string here, that text will propagate unmasked through all call sites that use str(exc) or exc.args[0]. In particular, request_runner_service.py line 1271 builds error_message = f"... {exc.args[0]}" and then persists that string to the DB via privacy_request.add_error_execution_log() (line 1278). The Pii(str(exc.args[0])) wrapper on the log call (line 1276) no longer provides meaningful protection because the sensitive body is already baked into the string upstream.

This is worth an explicit decision: is storing truncated external response bodies in execution_log.message (DB-persisted) acceptable? If not, callers that write to add_error_execution_log would need to strip or mask the body portion before passing it.

[vcruces]

What’s the probability of PII being present here?

[dsill-ethyca]

Discussing with @adamsachs , it probably shouldn't be here but this is conveniently touched by every saas connection so we're going to be a little more cautious

[claude]

The no_response case uses response_text=None as a sentinel and branches on it inside the test body. Since None and a Mock() with .text = None are meaningfully different things, this works — but it slightly obscures intent. A small improvement would be to parametrize response directly (passing None vs a Mock) rather than re-creating the object inside the test. Minor readability nit.

[vcruces]

Do you think it’s worth adding a log here?

[vcruces]

What’s the probability of PII being present here?