fix(http): don't force application/json on multipart uploads - #11
Conversation
The shared header builder sets a fixed Content-Type: application/json, which
overrode the multipart/form-data boundary reqwest's .multipart(form) sets, so
the backend JSON-parsed the multipart body and 500'd ("Unexpected token '-',
"--<boundary>"... is not valid JSON"). Strip Content-Type in post_multipart so
.multipart() owns it; every other verb (incl. body-less POSTs that rely on the
json content-type) is untouched. + wiremock regression tests.
There was a problem hiding this comment.
graycyrus has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
📝 WalkthroughWalkthrough
ChangesMultipart request handling
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/multipart.rs (1)
59-66: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winComplete the header-regression coverage.
Line 60 accepts
multipart/form-datawithout aboundary=parameter. Assert the boundary parameter because the server needs it to parse the form.Lines 83-106 send a JSON body.
.json()can setapplication/jsonindependently. Add a body-lessRawClient::send(Method::POST, ..., None, ...)test that assertsapplication/json. This covers the preserved contract stated for body-less POST requests.Also applies to: 83-106
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/multipart.rs` around lines 59 - 66, Extend the multipart header assertions near the existing content_type checks to require a boundary= parameter in the multipart/form-data value. In the body-less POST coverage around the JSON request in the test flow, add a RawClient::send(Method::POST, ..., None, ...) request and assert its response Content-Type is application/json, preserving the existing multipart and JSON-body assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/multipart.rs`:
- Around line 59-66: Extend the multipart header assertions near the existing
content_type checks to require a boundary= parameter in the multipart/form-data
value. In the body-less POST coverage around the JSON request in the test flow,
add a RawClient::send(Method::POST, ..., None, ...) request and assert its
response Content-Type is application/json, preserving the existing multipart and
JSON-body assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ee72cddd-3ec3-409a-bf07-2df995f5f9d0
📒 Files selected for processing (2)
src/lib.rstests/multipart.rs
Summary
RawClient::headers()sets a fixedContent-Type: application/jsonon every request. Onpost_multipartthat overrode themultipart/form-data; boundary=…reqwest's.multipart(form)must set, so the backend JSON-parsed the multipart body and returned500 "Unexpected token '-', \"--<boundary>\"... is not valid JSON". This broke all file uploads (OpenHumanstorage_upload_file+ the whole file→attachment chain).Fix
Strip
Content-Typeinpost_multipartbefore.multipart(form)so reqwest owns the multipart content-type. Scoped to the multipart path — every other verb is byte-identical, including body-less POSTs (teams,payments/stripe/portal,auth,channels) that rely on the sharedapplication/json. (A cleaner refactor would move content-type entirely to the body methods, but that would drop the json type from those body-less POSTs — deferred to avoid regressions.)Proof
Live against the (now-healthy) backend: a plain multipart curl → 200; the identical body with a forced
Content-Type: application/json→ the exact 500. Masked until now by the separately-fixed storage-bucket 500.Tests
tests/multipart.rs(wiremock): assertspost_multipartsends amultipart/form-datacontent-type and NOTapplication/json, plus a no-regression check that a JSON POST still sendsapplication/json. Full suite green.Summary by CodeRabbit
multipart/form-datacontent type, including the required boundary.application/jsonhandling for standard JSON requests.