Repository navigation
fix(api): preserve error response body - #65
Conversation
The API returns diagnostic XML bodies for non-2xx responses, but RestClient discarded the body before constructing CommunicationException. That left client logs with an empty Response field and made valid endpoint/entity failures hard to diagnose.
There was a problem hiding this comment.
Lenses run: correctness/invariants · security · API-contract/backward-compat, with test-coverage as a standing pass. → 0 findings; nothing to anchor inline.
Reviewed at 4ab7611 (refs/pull/65/head), read from fetched objects rather than a working tree.
Three things worth being sure of on a two-line fix, all clean:
1. rawData is actually populated on this path. requestResultString is assigned unconditionally at RestClient.cs:178, and every route into the non-2xx branch at :182 passes through it with no intervening return. ReadAsStringAsync yields "" rather than null on an empty body, so the new argument is never null and carries the diagnostic whenever the API sends one.
2. No new leak surface. The access token travels as a request header (RestClient.cs:35), never in the URL or the body, so an error response cannot echo it back. And the same string is already written unconditionally to the log one line earlier (RestClient.cs:180) — this routes an already-logged value into the exception, it does not introduce a new class of data into logs.
3. Nothing downstream changes shape. Failure()'s rawData already defaulted to "" (RequestResult.cs:41), so no caller could have been branching on null. RawData is consumed only at RestClient.cs:79 and :128, both feeding CommunicationException.Response and its ToString() (CommunicationException.cs:42) — which is exactly the field this fix exists to fill. No test asserts on either. The call site uses named arguments, so parameter order is not in play.
Trailing newline is a harmless normalization. CI green.
Manual review by Tomáš: LGTM.
Co-Authored-By: Claude Opus 5
Summary
When an API request returns a non-2xx status, the resulting log line carries no
information about why it failed:
Response=is always empty, so a wrong endpoint and a genuinely missing entityare indistinguishable in logs.
The API does return a diagnostic body on every error:
RestClientreads that body intorequestResultStringand then discards it: thenon-2xx branch calls
RequestResult.Failure(...)without passingrawData, eventhough
Failure()already declares the parameter. This restores it.Note that the
ignoreUnsuccessfulStatusCodebranch a few lines above alreadypassed the body to
Success()— the failure path was simply inconsistent withthe success path next to it.
Verification
The two 404 cases the API distinguishes, confirmed against a running instance:
404with an entity-level message404with<action>Wrong endpoint</action>Both are indistinguishable in logs today; with this change the
<action>and<message>reachCommunicationException.Response.45/45tests pass; no new compiler warnings.Blast radius
Diagnostics only — one line, one file. Request behaviour, caching, retries and
threading are all unchanged; the only difference is that a field which was
always empty is now populated.