fix(client): omit Content-Type on DELETE requests - #198
Conversation
defaultHeaders set Content-Type: application/json for every non-GET/HEAD method, including bodyless DELETE. That makes contexts.delete() (and any other DELETE) fail with 400 "Body cannot be empty when content-type is set to application/json" — the same class of bug as extensions.delete (browserbase#169 / browserbase#180). Exclude delete alongside get and head when applying the default Content-Type header.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 08982a7824
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return { | ||
| Accept: 'application/json', | ||
| ...(['head', 'get'].includes(opts.method) ? {} : { 'Content-Type': 'application/json' }), | ||
| ...(['head', 'get', 'delete'].includes(opts.method) ? {} : { 'Content-Type': 'application/json' }), |
There was a problem hiding this comment.
When a caller uses the supported generic API for a JSON-bearing request such as client.delete('/some/path', { body: { ... } }), buildRequest still serializes and sends that body, but this method-only condition now omits Content-Type: application/json. Fetch implementations may consequently label the serialized string as text/plain or leave it untyped, causing DELETE endpoints that parse JSON to reject or misinterpret the request. Omit the header only when the DELETE request has no body rather than for every DELETE.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed — DELETE with a body keeps Content-Type: application/json when a JSON body is present (method alone no longer strips it).
Bodyless DELETE still omits Content-Type (browserbase#180). DELETE with a JSON body restores application/json so fetch does not treat it as text/plain.
|
Addressed Codex P2: bodyless DELETE still omits `Content-Type` (#180), but DELETE with a JSON body now keeps `application/json`. Added a regression test for both cases. |
|
Codex P2 is addressed in `14d7925`: bodyless DELETE still omits `Content-Type` (#180), but DELETE with a JSON body keeps `application/json`. Regression tests cover both paths. |
1 participant
Summary
defaultHeaderspreviously setContent-Type: application/jsonfor every non-GET/HEAD method, including bodyless DELETE.contexts.delete()fail with400 Body cannot be empty when content-type is set to 'application/json'— same class of bug asextensions.delete()(error when trying to delete extension #169).deletealongsideget/headwhen applying the default Content-Type (as suggested in [Bug] contexts.delete() fails with 400 — same Content-Type issue as extensions.delete() (#169) #180).Fixes #180
Test plan
content-type; POST still getsapplication/jsoncontent-type: application/json; with fix, both tests passMade with Cursor