fix(client): profile picture upload + display on android (#253, #256) #263

Merged
james merged 11 commits from fix-formdata-detection into main 2026-06-23 20:37:57 +00:00
Owner

Summary

Closes #253 and #256 after a long debugging path through five attempts (PRs #257, #260, #263, plus the iterative commits on this branch). The actual root causes were two distinct Expo SDK 56-specific issues:

Root causes

Upload (#253): Expo SDK 56's @expo/winter fetch polyfill (the default fetch on every Expo build) doesn't support RN's { uri, name, type } file-descriptor FormData parts. The upstream comment is explicit: uri is not supported for React Native's FormData. See expo/src/winter/fetch/convertFormData.ts — only string, Blob, or { bytes } parts are accepted; anything else throws "Unsupported FormDataPart implementation" from the JS layer, before the network ever sees the request.

This explains why every prior attempt failed: PRs #257, #260, #263 each chased a different layer (RN's networking module, URL rewriter, Content-Type generation, Headers iteration) — but the winter polyfill rejects the FormData before any of that code runs.

Display (#256): RN's <Image> on Android has documented inconsistent support for source.headers. The runtime log proved it definitively: source.headers.Authorization was set, but the resulting OkHttp GET hit the server with no Authorization header and returned 401.

Fixes

  1. Native upload uses expo-file-system's File instead of the RN file-descriptor. File extends Blob, which is exactly what the winter polyfill accepts.
  2. Avatar uses expo-image (Expo's enhanced image component, built on Glide on Android / Nuke on iOS) which honors source.headers reliably.
  3. The upload goes through a screen-local mutation calling uploadProfilePicture() (bypassing the typed @carol/api-client and its URL-rewriter middleware) — that ensures RN's native fetch receives an untouched Request and handles the multipart serialization end-to-end. Web continues to work via the same path same-origin.
  4. New loadAccessToken() eager preload + subscribeAccessToken() pub/sub in lib/auth/storage.ts so the avatar's bearer header is available on the first paint after cold-launch.

What this PR added vs reverted

Added:

  • expo-file-system, expo-image deps.
  • lib/profile/pictureFetch.ts — raw fetch upload + buildPictureFormData native branch (kept here so the expo-file-system import chain doesn't pollute the node-runtime vitest suite).
  • loadAccessToken() / subscribeAccessToken().

Refined:

  • Off-origin URL rewriter detects FormData via the polyfill's private _bodyFormData field (in addition to Content-Type) — kept because it's the right shape for any future non-upload multipart endpoint, even though uploads now bypass the rewriter.

Test plan

  • pnpm -F @carol/client typecheck / lint / test (88 passing).
  • pnpm -F @carol/client export:web (web bundle still produces dist/).
  • Manual: Android via Expo Go — pick image, upload, avatar refreshes without page reload, picture renders on subsequent cold-launches.

Out of scope (filing as follow-ups if you want)

  • A native E2E smoke that would have caught these earlier — #234 tracks this.

Closes #253.
Closes #256.

## Summary Closes #253 and #256 after a long debugging path through five attempts (PRs #257, #260, #263, plus the iterative commits on this branch). The actual root causes were two distinct Expo SDK 56-specific issues: ## Root causes **Upload (#253):** Expo SDK 56's `@expo/winter` fetch polyfill (the default fetch on every Expo build) **doesn't support RN's `{ uri, name, type }` file-descriptor FormData parts**. The upstream comment is explicit: `uri is not supported for React Native's FormData.` See `expo/src/winter/fetch/convertFormData.ts` — only string, Blob, or `{ bytes }` parts are accepted; anything else throws "Unsupported FormDataPart implementation" from the JS layer, before the network ever sees the request. This explains why every prior attempt failed: PRs #257, #260, #263 each chased a different layer (RN's networking module, URL rewriter, Content-Type generation, Headers iteration) — but the winter polyfill rejects the FormData before any of that code runs. **Display (#256):** RN's `<Image>` on Android has documented inconsistent support for `source.headers`. The runtime log proved it definitively: `source.headers.Authorization` was set, but the resulting OkHttp GET hit the server with no Authorization header and returned 401. ## Fixes 1. **Native upload uses `expo-file-system`'s `File`** instead of the RN file-descriptor. `File` extends `Blob`, which is exactly what the winter polyfill accepts. 2. **Avatar uses `expo-image`** (Expo's enhanced image component, built on Glide on Android / Nuke on iOS) which honors `source.headers` reliably. 3. The upload goes through a screen-local mutation calling `uploadProfilePicture()` (bypassing the typed `@carol/api-client` and its URL-rewriter middleware) — that ensures RN's native fetch receives an untouched Request and handles the multipart serialization end-to-end. Web continues to work via the same path same-origin. 4. New `loadAccessToken()` eager preload + `subscribeAccessToken()` pub/sub in `lib/auth/storage.ts` so the avatar's bearer header is available on the first paint after cold-launch. ## What this PR added vs reverted Added: - `expo-file-system`, `expo-image` deps. - `lib/profile/pictureFetch.ts` — raw fetch upload + `buildPictureFormData` native branch (kept here so the `expo-file-system` import chain doesn't pollute the node-runtime vitest suite). - `loadAccessToken()` / `subscribeAccessToken()`. Refined: - Off-origin URL rewriter detects FormData via the polyfill's private `_bodyFormData` field (in addition to Content-Type) — kept because it's the right shape for any future non-upload multipart endpoint, even though uploads now bypass the rewriter. ## Test plan - [x] `pnpm -F @carol/client typecheck` / `lint` / `test` (88 passing). - [x] `pnpm -F @carol/client export:web` (web bundle still produces dist/). - [x] Manual: Android via Expo Go — pick image, upload, avatar refreshes without page reload, picture renders on subsequent cold-launches. ## Out of scope (filing as follow-ups if you want) - A native E2E smoke that would have caught these earlier — [#234](https://forge.wynning.tech/james/carol/issues/234) tracks this. Closes #253. Closes #256.
fix(client): detect FormData via RN polyfill private field, not just Content-Type
All checks were successful
Commits / Conventional Commits (pull_request) Successful in 7s
PR / OSV-Scanner (pull_request) Successful in 1m36s
PR / OpenAPI (pull_request) Successful in 2m6s
PR / Client (web export smoke) (pull_request) Successful in 2m17s
PR / Typecheck (pull_request) Successful in 2m27s
PR / pnpm audit (pull_request) Successful in 2m33s
PR / Static analysis (pull_request) Successful in 2m36s
PR / Lint (pull_request) Successful in 2m54s
PR / Package age policy (soft) (pull_request) Successful in 40s
Secrets / gitleaks (pull_request) Successful in 37s
PR / Build (pull_request) Successful in 3m21s
PR / Test (sqlite) (pull_request) Successful in 3m21s
PR / Test (postgres) (pull_request) Successful in 3m21s
PR / Coverage (soft) (pull_request) Successful in 1m30s
PR / Trivy (image) (pull_request) Successful in 2m0s
d17bd65493
PR #260's multipart detection only checked
`request.headers.get("content-type")`, but the Content-Type header
isn't set on the outgoing Request at middleware time — RN's
whatwg-fetch generates the multipart Content-Type (with boundary) at
send time, not at Request construction. So `isMultipart` evaluated
false for every Android upload, the rewriter fell through to the
JSON-rebuild branch, and `extractRequestBody` called `request.text()`
on a FormData body → "could not read FormData body as text" thrown by
RN's polyfill. The upload screen caught the exception and surfaced the
generic "Upload failed. Try again." Three fix attempts (#257, #260,
this one) chased the wrong layer because the detection logic was
silently wrong — local node tests pass because undici sets the header
eagerly, so the regression was Android-only.

Probe the polyfill's private `_bodyFormData` (and `_bodyInit instanceof
FormData` as belt-and-braces) alongside the Content-Type check. Either
signal flips the multipart branch.

Closes #253.
Closes #256.

📊 Test coverage

Patch coverage: no testable lines changed.

Overall (app/, lib/, db/, excluding UI per ADR-0019):

Metric Value Soft target
Lines 82.9% ≥ 50%
Branches 75.6% ≥ 75%
Functions 91.8% informational

Soft thresholds per ADR-0019. Coverage is informational and does not block merge.

<!-- coverage-comment --> ## 📊 Test coverage **Patch coverage:** no testable lines changed. **Overall** (`app/`, `lib/`, `db/`, excluding UI per ADR-0019): | Metric | Value | Soft target | |---|---|---| | Lines | 82.9% ✅ | ≥ 50% | | Branches | 75.6% ✅ | ≥ 75% | | Functions | 91.8% | informational | Soft thresholds per [ADR-0019](docs/adr/0019-coverage-soft-targets.md). Coverage is informational and does not block merge.
chore(client): instrument rewriter + upload catch to diagnose Android FormData path
All checks were successful
Commits / Conventional Commits (pull_request) Successful in 7s
PR / OSV-Scanner (pull_request) Successful in 1m35s
PR / Static analysis (pull_request) Successful in 1m46s
PR / pnpm audit (pull_request) Successful in 1m57s
PR / OpenAPI (pull_request) Successful in 2m28s
PR / Client (web export smoke) (pull_request) Successful in 2m51s
PR / Package age policy (soft) (pull_request) Successful in 58s
PR / Lint (pull_request) Successful in 2m56s
PR / Typecheck (pull_request) Successful in 3m2s
PR / Build (pull_request) Successful in 3m15s
PR / Test (postgres) (pull_request) Successful in 3m15s
Secrets / gitleaks (pull_request) Successful in 53s
PR / Test (sqlite) (pull_request) Successful in 3m25s
PR / Coverage (soft) (pull_request) Successful in 1m56s
PR / Trivy (image) (pull_request) Successful in 2m8s
9a543645c6
Adds console.log inside the off-origin URL rewriter showing which
branch fires (multipart vs JSON) and what the polyfill's _bodyInit /
_bodyFormData fields look like. Also keeps the upload catch's error
detail in the toast + Metro log. Strip both before merge once the
FormData detection fix is confirmed on a real device.
fix(client): strip wrong Content-Type on multipart so RN regenerates it
All checks were successful
Commits / Conventional Commits (pull_request) Successful in 13s
PR / OpenAPI (pull_request) Successful in 2m14s
PR / Client (web export smoke) (pull_request) Successful in 2m35s
PR / Static analysis (pull_request) Successful in 2m35s
PR / Typecheck (pull_request) Successful in 2m41s
PR / Lint (pull_request) Successful in 2m46s
PR / pnpm audit (pull_request) Successful in 1m46s
PR / Build (pull_request) Successful in 2m58s
PR / OSV-Scanner (pull_request) Successful in 43s
PR / Test (postgres) (pull_request) Successful in 2m56s
PR / Package age policy (soft) (pull_request) Successful in 32s
Secrets / gitleaks (pull_request) Successful in 28s
PR / Test (sqlite) (pull_request) Successful in 2m38s
PR / Coverage (soft) (pull_request) Successful in 1m8s
PR / Trivy (image) (pull_request) Successful in 1m49s
b71fbe88d4
PR #260's clone-via-new-Request preserved every header from the
original Request — including the bogus Content-Type: application/json
that openapi-fetch forces before its bodySerializer hands the
FormData back. On RN the polyfill doesn't regenerate the multipart
Content-Type until send time, so the cloned Request shipped multipart
bytes labelled as application/json. The server's req.formData() then
rejected with "Expected a multipart/form-data body."

Strip Content-Type only when it's NOT already multipart-shaped — on
undici / browsers the constructor sets the multipart Content-Type
with boundary eagerly and we want to keep it. On RN the polyfill
sets nothing (or JSON forced by openapi-fetch); stripping lets the
send-time regeneration run.
chore(client): bump rewriter diagnostic from console.log to console.warn
All checks were successful
Commits / Conventional Commits (pull_request) Successful in 13s
PR / OSV-Scanner (pull_request) Successful in 28s
PR / Static analysis (pull_request) Successful in 45s
PR / pnpm audit (pull_request) Successful in 2m5s
PR / OpenAPI (pull_request) Successful in 2m14s
PR / Typecheck (pull_request) Successful in 2m20s
PR / Test (postgres) (pull_request) Successful in 2m39s
PR / Lint (pull_request) Successful in 3m23s
PR / Client (web export smoke) (pull_request) Successful in 3m31s
PR / Build (pull_request) Successful in 3m36s
PR / Package age policy (soft) (pull_request) Successful in 1m28s
PR / Coverage (soft) (pull_request) Successful in 2m57s
Secrets / gitleaks (pull_request) Successful in 1m30s
PR / Test (sqlite) (pull_request) Successful in 3m46s
PR / Trivy (image) (pull_request) Successful in 3m4s
8639d291d8
fix(client): bypass URL rewriter for picture upload (#253, fourth attempt)
All checks were successful
Commits / Conventional Commits (pull_request) Successful in 14s
PR / OSV-Scanner (pull_request) Successful in 28s
PR / Static analysis (pull_request) Successful in 45s
PR / pnpm audit (pull_request) Successful in 1m56s
PR / Test (sqlite) (pull_request) Successful in 3m20s
PR / Client (web export smoke) (pull_request) Successful in 3m20s
PR / Package age policy (soft) (pull_request) Successful in 1m23s
PR / OpenAPI (pull_request) Successful in 3m30s
PR / Lint (pull_request) Successful in 3m37s
PR / Build (pull_request) Successful in 3m46s
PR / Typecheck (pull_request) Successful in 3m46s
PR / Trivy (image) (pull_request) Successful in 3m2s
PR / Test (postgres) (pull_request) Successful in 3m54s
Secrets / gitleaks (pull_request) Successful in 35s
PR / Coverage (soft) (pull_request) Successful in 3m41s
b9e3989da0
PRs #257, #260, and #263 all chased FormData handling inside the
off-origin URL-rewriter middleware. Each fixed the previous attempt's
specific failure mode and surfaced a new one. The deeper issue is
that RN's whatwg-fetch polyfill auto-generates the multipart
Content-Type at SEND TIME from the FormData body, and any middleware
that touches the Request (clones it, re-applies headers, reads the
body) breaks that magic.

Diagnostic confirmed the rewriter takes the multipart branch
correctly with isMultipart=true, hasBodyFormData=true,
url='http://10.10.2.7:3000/api/profile/picture' — but the cloned
Request reaches the server WITHOUT a multipart Content-Type
regardless of what the rewriter does with the JSON Content-Type that
openapi-fetch sets.

Bypass the typed client + rewriter entirely. `uploadProfilePicture()`
lives in lib/profile/pictureFetch.ts (separate from pictureUpload.ts
so its `react-native` import chain doesn't pull __DEV__ into the
pure-function vitest suite). It computes the URL (relative on web,
absolute on native via getCachedServerUrl), attaches the bearer on
native, and calls fetch() directly with the FormData body. RN's
polyfill receives the FormData on a fresh untouched Request and
auto-generates the right multipart/form-data; boundary=... Content-
Type at send time — the path it was designed for.

Error path matches @carol/api-client's errorMiddleware so the
screen's catch sees CarolApiError on non-2xx.

The screen swaps `useUpdateProfilePicture` for a screen-local
useMutation wrapping uploadProfilePicture, replicating the
queryClient invalidations that the hook used to do.

Closes #253.
chore(client): log picker asset shape to diagnose FormDataPart error
All checks were successful
Commits / Conventional Commits (pull_request) Successful in 10s
PR / OSV-Scanner (pull_request) Successful in 1m41s
PR / Static analysis (pull_request) Successful in 2m4s
PR / pnpm audit (pull_request) Successful in 2m21s
PR / OpenAPI (pull_request) Successful in 2m27s
PR / Typecheck (pull_request) Successful in 2m42s
PR / Client (web export smoke) (pull_request) Successful in 2m41s
PR / Build (pull_request) Successful in 2m58s
PR / Lint (pull_request) Successful in 3m7s
PR / Package age policy (soft) (pull_request) Successful in 45s
Secrets / gitleaks (pull_request) Successful in 48s
PR / Test (sqlite) (pull_request) Successful in 3m16s
PR / Test (postgres) (pull_request) Successful in 3m25s
PR / Coverage (soft) (pull_request) Successful in 1m38s
PR / Trivy (image) (pull_request) Successful in 2m0s
16f9e35f51
chore(client): introspect FormData ctor + parts to diagnose native send
All checks were successful
Commits / Conventional Commits (pull_request) Successful in 9s
PR / OSV-Scanner (pull_request) Successful in 1m29s
PR / pnpm audit (pull_request) Successful in 2m8s
PR / OpenAPI (pull_request) Successful in 2m26s
PR / Static analysis (pull_request) Successful in 2m26s
PR / Lint (pull_request) Successful in 2m37s
PR / Client (web export smoke) (pull_request) Successful in 2m49s
PR / Typecheck (pull_request) Successful in 2m56s
PR / Build (pull_request) Successful in 3m3s
PR / Package age policy (soft) (pull_request) Successful in 52s
Secrets / gitleaks (pull_request) Successful in 52s
PR / Test (postgres) (pull_request) Successful in 3m19s
PR / Test (sqlite) (pull_request) Successful in 3m20s
PR / Trivy (image) (pull_request) Successful in 2m0s
PR / Coverage (soft) (pull_request) Successful in 1m40s
acb9e87eb8
fix(client): use expo-file-system File for native picture upload (#253, fifth attempt)
All checks were successful
Commits / Conventional Commits (pull_request) Successful in 12s
PR / OSV-Scanner (pull_request) Successful in 1m27s
PR / pnpm audit (pull_request) Successful in 2m6s
PR / OpenAPI (pull_request) Successful in 2m23s
PR / Static analysis (pull_request) Successful in 2m23s
PR / Lint (pull_request) Successful in 2m41s
PR / Typecheck (pull_request) Successful in 2m49s
PR / Client (web export smoke) (pull_request) Successful in 2m55s
PR / Test (postgres) (pull_request) Successful in 3m7s
PR / Test (sqlite) (pull_request) Successful in 3m9s
PR / Package age policy (soft) (pull_request) Successful in 45s
Secrets / gitleaks (pull_request) Successful in 46s
PR / Build (pull_request) Successful in 3m32s
PR / Coverage (soft) (pull_request) Successful in 2m5s
PR / Trivy (image) (pull_request) Successful in 2m34s
d1705d7d98
Root cause finally found: Expo SDK 56's @expo/winter fetch polyfill
(the default fetch on Expo) doesn't support RN's `{ uri, name, type }`
file-descriptor FormData parts. The error comment in upstream is
explicit: "`uri` is not supported for React Native's FormData."
See expo/src/winter/fetch/convertFormData.ts — only string / Blob /
`{ bytes }` parts are accepted; anything else throws "Unsupported
FormDataPart implementation" from the JS side, before the request
ever reaches the network.

This explains all four prior attempts. They were all chasing the
wrong layer — RN's networking module, the URL rewriter, Content-Type
generation, Headers iteration — none of which mattered, because the
winter polyfill rejects the FormData before any of that code runs.

Switch native to `expo-file-system`'s `File`, which extends Blob and
is exactly the shape the winter polyfill accepts. Web continues to
use the existing fetch-the-blob-URL path.

The native FormData builder moves to lib/profile/pictureFetch.ts
because expo-file-system's import chain can't load under vitest's
node environment (rolldown parse error on Expo's transitives). The
node-runtime test suite stays pure with the web-only helper renamed
to buildPictureFormDataWeb. Native-branch tests deleted — they
asserted the old RN file-descriptor shape that no longer ships, and
the new path can only be exercised on a device.

Closes #253.
Closes #256.
chore(client): log Avatar source + load events to diagnose blank picture
All checks were successful
Commits / Conventional Commits (pull_request) Successful in 9s
PR / Static analysis (pull_request) Successful in 47s
PR / OSV-Scanner (pull_request) Successful in 1m8s
PR / pnpm audit (pull_request) Successful in 2m10s
PR / Test (sqlite) (pull_request) Successful in 2m34s
PR / Client (web export smoke) (pull_request) Successful in 2m40s
PR / Typecheck (pull_request) Successful in 2m46s
PR / OpenAPI (pull_request) Successful in 2m56s
PR / Lint (pull_request) Successful in 3m1s
PR / Trivy (image) (pull_request) Successful in 2m17s
PR / Package age policy (soft) (pull_request) Successful in 54s
Secrets / gitleaks (pull_request) Successful in 34s
PR / Build (pull_request) Successful in 3m15s
PR / Test (postgres) (pull_request) Successful in 3m21s
PR / Coverage (soft) (pull_request) Successful in 2m36s
dbd0fa1c86
fix(client): switch Avatar to expo-image so source.headers reach the network (#256)
Some checks failed
Commits / Conventional Commits (pull_request) Successful in 10s
PR / Static analysis (pull_request) Successful in 1m33s
PR / OpenAPI (pull_request) Successful in 2m34s
PR / Lint (pull_request) Successful in 2m41s
PR / Client (web export smoke) (pull_request) Successful in 2m48s
PR / Typecheck (pull_request) Successful in 2m54s
PR / Test (postgres) (pull_request) Failing after 3m4s
PR / Build (pull_request) Successful in 3m11s
PR / Test (sqlite) (pull_request) Successful in 3m11s
PR / OSV-Scanner (pull_request) Successful in 1m43s
PR / pnpm audit (pull_request) Successful in 3m7s
Secrets / gitleaks (pull_request) Successful in 33s
PR / Package age policy (soft) (pull_request) Successful in 41s
PR / Coverage (soft) (pull_request) Successful in 1m57s
PR / Trivy (image) (pull_request) Successful in 3m9s
986948583b
RN's `<Image>` on Android has documented inconsistent support for
`source.headers` — the runtime log proves it here: the source carries
`Authorization: Bearer cat_…` but the resulting OkHttp GET hits the
server without it and returns 401. PR #260 / PR #257's attempt to
authenticate the avatar via `source.headers` was fundamentally
correct, but the underlying RN Image component drops the header.

`expo-image` (Expo SDK 56's enhanced image component, built on top of
Glide / Nuke) honours `source.headers` reliably across platforms.
Same API as RN's `<Image>` for the surface we use, so the swap is a
one-line import change.

Diagnostic stays in place (the load-error / loaded logs) until on-
device confirmation that the picture renders.

Closes #256.
chore(client): strip diagnostics from picture upload + display path
All checks were successful
Commits / Conventional Commits (pull_request) Successful in 14s
PR / OSV-Scanner (pull_request) Successful in 18s
PR / Static analysis (pull_request) Successful in 1m13s
PR / OpenAPI (pull_request) Successful in 2m18s
PR / Typecheck (pull_request) Successful in 2m30s
PR / pnpm audit (pull_request) Successful in 2m59s
PR / Client (web export smoke) (pull_request) Successful in 3m26s
PR / Test (postgres) (pull_request) Successful in 3m26s
PR / Package age policy (soft) (pull_request) Successful in 1m8s
Secrets / gitleaks (pull_request) Successful in 59s
PR / Test (sqlite) (pull_request) Successful in 3m45s
PR / Trivy (image) (pull_request) Successful in 3m15s
PR / Lint (pull_request) Successful in 3m35s
PR / Build (pull_request) Successful in 3m40s
PR / Coverage (soft) (pull_request) Successful in 2m42s
6521f574a9
Bug confirmed fixed on Android. Removing the rewriter [rewriter] log,
the picker [picker] asset dump, the upload-catch detail toast, and
the avatar onError/onLoad logging. The fix changes (expo-file-system
File on native upload, expo-image for the avatar) stay.
james changed title from fix(client): detect FormData via RN polyfill private field (#253 #256 — third attempt) to fix(client): profile picture upload + display on android (#253, #256) 2026-06-23 20:28:53 +00:00
james merged commit 951bc511e8 into main 2026-06-23 20:37:57 +00:00
james deleted branch fix-formdata-detection 2026-06-23 20:37:58 +00:00
Sign in to join this conversation.
No description provided.