Repository navigation
Conversation
`post_multipart` applied `headers()` — which sets `Content-Type: application/json` for the crate's JSON routes — and then called `.multipart(form)`. reqwest's `RequestBuilder::header` appends rather than replaces, so every upload went out with two `Content-Type` lines: the inherited JSON one first, the real multipart one second. Node/Express keeps only the first per RFC 7230 §3.2.2, so the backend saw `application/json` on a multipart body, handed it to `express.json()`, and 500'd on the `--<boundary>` delimiter. Every SDK multipart upload was mislabelled: `/agent-integrations/file-storage/files`, `/agent-integrations/history-rewards/uploads` and `/openai/v1/audio/transcriptions` (backend Sentry BACKEND-NODEJS-49, client Sentry TAURI-RUST-QBN). Drop the inherited `Content-Type` before `.multipart()` so reqwest's value — the only one that carries the boundary — is the sole one on the wire. Credential and static headers are untouched. Tests assert a multipart request carries exactly one `Content-Type` and that it is `multipart/form-data; boundary=…`, that auth and static headers survive, and that JSON routes still send a single `application/json`. The first fails against the previous behaviour with both values present. Fixes tinyhumansai#8
There was a problem hiding this comment.
M3gA-Mind has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 27 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
Comment |
|
Closing as superseded — the fix this PR describes already landed on
That is why this branch now reports The one thing here that is not on |
Summary
post_multipartsent twoContent-Typeheader lines on every upload.headers()setsContent-Type: application/jsonfor the crate's JSON routes, andpost_multipartapplied it and then called.multipart(form). reqwest'sRequestBuilder::header— which.multipart()uses to set the boundary type —appends rather than replaces (
reqwest-0.12.28/src/async_impl/request.rs:223), soboth values survived: the inherited JSON one first, the real multipart one second.
Node/Express keeps only the first per RFC 7230 §3.2.2, so the backend saw
application/jsonon a multipart body, handed it toexpress.json(), and 500'd on the--<boundary>delimiter.Fix
Drop the inherited
Content-Typebefore.multipart(), so reqwest's value — the only onethat carries the boundary — is the sole one on the wire:
Removing from the shared
headers()rather than building a parallel header map keepsauth/
accept/x-sdk-clienton a single source of truth.HeaderMap::removeclears allvalues for the key, so a host-supplied
Content-Typefromwith_default_headersisdropped too — correct here, since only reqwest can know the boundary.
Also documented the footgun on
headers()itself, so the next body-typed method does notreintroduce it.
Impact
Every SDK multipart upload was mislabelled:
POST /agent-integrations/file-storage/filesPOST /agent-integrations/history-rewards/uploadsPOST /openai/v1/audio/transcriptionsBackend Sentry
BACKEND-NODEJS-49(14 users), client SentryTAURI-RUST-QBN(11 users).Tests
tests/multipart.rs(new, wiremock per repo convention):Content-Type, and it ismultipart/form-data; boundary=…application/jsonThe first test was run against the unfixed code before any change and failed with
precisely the production shape:
— the same two-header ordering that produced
--7a302effin the backend Sentry event, sothis is a confirmed reproduction rather than an inferred one. (wiremock's hyper-based
server preserves duplicate header values in its
HeaderMap, unlike Node, which is whatmakes the bug observable in-process.)
Notes
Consumers need a re-pin.
openhumanvendors this crate as a submodule, soTAURI-RUST-QBNstays open on the client side until an SDK release lands and openhumanbumps its pin.
Already-shipped clients are unblocked separately. tinyhumansai/backend#1182 adds a
server-side repair that recovers the real media type from
req.rawHeaders. That is whathelps users running today's builds; this PR removes the cause.
Fixes #8