[mikepitre]

Summary

ClientSyncingMiddleware.syncResponse reads the response body with body.string(), which uses up the OkHttp body, so it can sync the piggybacked client. On success it rebuilt the response from that string. But if syncing threw SerializationException, IOException or IllegalArgumentException, the catch blocks only logged and returned the original response, whose body had already been read.

So any client that failed to decode broke a valid call. This happens when the piggybacked client has a missing required field or a new enum value with no fallback. Retrofit read an empty body and the call came back as ClerkResult.Failure(UNKNOWN), even though the endpoint's own response payload was fine.

The response is now rebuilt from the saved body string after the try/catch, so it happens on every path. A failed client sync is logged and the endpoint's body is passed through unchanged.

Testing

• New test intercept preserves response body when piggybacked client fails to decode in ClientSyncingMiddlewareTest serves a 200 with a valid response and a client whose session is missing required fields. It asserts the body after the middleware matches the original exactly and that Clerk.client is unchanged. Before this change it failed with expected:<[{ "response": ... }]> but was:<[]>, and it passes after.
• ./gradlew :source:api:testDebugUnitTest :source:api:spotlessCheck :source:api:detekt passes on JDK 21 (762 tests, 0 failures).

Overlap with open PRs

• feat(api): add framework device token compare-and-set #964 edits syncResponse lines ~67–77 inside the try, plus syncClientFromResponse below. This PR only touches lines ~80–91 (it moves the rebuild and return to after the catch blocks) and leaves those lines unchanged.
• feat(api): add framework device token compare-and-set #964 and test: audit the unit test suite for tests that don't test anything #985 both edit ClientSyncingMiddlewareTest.kt, at the imports, around line 128, and appended after line 422. This PR adds its test at line ~341, away from all three spots.

Not in this PR

When the client decode fails, any SignInCompleted/SignUpCompleted events decoded from the same response are still dropped. That's because authEvents.forEach(Clerk.auth::send) runs inside the same try, after the client sync.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Hosted-auth responses with incomplete session data now preserve the original response body and leave the existing client unchanged.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: clerk/clerk-android/.coderabbit.yaml

Review profile: CHILL

Plan: Team

Run ID: 01e30d94-8b7c-4fd5-86f8-7985b9512138

📥 Commits

Reviewing files that changed from the base of the PR and between c58b14e and 86f6e79.

📒 Files selected for processing (2)

• source/api/src/main/kotlin/com/clerk/api/network/middleware/incoming/ClientSyncingMiddleware.kt
• source/api/src/test/java/com/clerk/api/network/middleware/incoming/ClientSyncingMiddlewareTest.kt

Limit details: You’ve used all 4 included reviews currently available. Your 60 included PR review attempts over the past 7 days set your current allowance at 4 reviews per hour.

---

📝 Walkthrough

Walkthrough

The middleware now rebuilds and returns the saved response body after caught parsing or processing errors. A new test checks this behavior for a hosted-auth response with a malformed piggybacked client.

Changes

Client response recovery

Layer / File(s)	Summary
Response body recovery and validation source/api/src/main/kotlin/com/clerk/api/network/middleware/incoming/ClientSyncingMiddleware.kt, source/api/src/test/java/com/clerk/api/network/middleware/incoming/ClientSyncingMiddlewareTest.kt	After caught errors, syncResponse rebuilds and returns the body from the saved response text. The new test checks that a malformed piggybacked client leaves the response body readable and Clerk’s existing client unchanged.

Priority: ⬇️ Low

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

Change: Bug fix

Suggested reviewers: swolfand

Merge Risk: ⚪ Minimal · up to 86f6e

The change preserves readable endpoint responses when client synchronization fails. No actionable merge-blocking risk is established by the supplied evidence.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main change: preserving the response body when client synchronization fails.
Description check	✅ Passed	The description explains the problem, the fix, test coverage, validation results, overlap with other PRs, and a known limitation. It does not use the template headings exactly, but it provides the req…
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.

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

---

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

[swolfand]

The body is now restored on the caught failure paths, and the test proves it. Non-blocking: an exception outside the three caught types (for example one thrown from Clerk.updateClient) still loses the body, same as before. A finally would close that gap. LGTM.