[ONCALL-264] fix: axios double stringifying request body string when sent with application/json #272
Conversation
…sent with application/json
WalkthroughUpdates to src/main/actions/makeApiClientRequest.js: multipart/form-data handling reworked to map entries to { stream, fileName }, filter non-streams, and append files via destructuring; content-type header lookup now uses case-insensitive header key before falling back to apiRequest.contentType; adds JSON-aware Axios transformRequest: detects application/json (header or apiRequest.contentType), attempts JSON.parse for string bodies and, on parse failure, sets a transform to bypass Axios default JSON stringification to send the raw body. Minor formatting and indentation adjustments. Lines changed: +51/-14. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/main/actions/makeApiClientRequest.js`:
- Around line 79-86: The code that determines requestContentType uses raw header
values, so normalize the Content-Type before comparison: when computing
requestContentType from headers (symbol: requestContentType and headers) trim
it, lowercase it and strip any parameters after ';' (e.g., split on ';' and take
the first token), and do the same normalization to apiRequest.contentType before
any equality checks; update places that compare requestContentType to
apiRequest.contentType to use these normalized values to ensure
JSON/content-type matching works regardless of case or charset parameters.
- Around line 71-73: The Content-Type bug is caused by template-stringifying
formData.getHeaders(), which yields "[object Object]"; in
makeApiClientRequest.js, change the headers construction to merge the object
returned by formData.getHeaders() instead of stringifying it: retrieve
formData.getHeaders() (via variable like formHeaders) and spread its properties
into the headers object (e.g., ...formHeaders, ...headers) so the multipart
boundary header is preserved and not turned into a string.
🧹 Nitpick comments (1)
src/main/actions/makeApiClientRequest.js (1)
41-62: Usepath.basenamefor cross‑platform filenames.
entry.path.split("/")fails on Windows paths. Preferpath.basenameto handle both/and\.♻️ Suggested change
+const path = require("path"); ... - fileName: entry.name || entry.path.split("/").pop(), + fileName: entry.name || path.basename(entry.path),
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/main/actions/makeApiClientRequest.js`:
- Around line 90-94: The check in makeApiClientRequest.js calls
requestContentType.includes(...) without ensuring requestContentType exists;
update the guard so it first ensures a string (or non-null) before calling
includes — for example coerce requestContentType to an empty string or use a
truthy check like requestContentType &&
requestContentType.includes("application/json") — then proceed with the typeof
body === "string" check; make this change around the requestContentType, body
and apiRequest.contentType usage so missing headers/apiRequest.contentType won't
throw.
♻️ Duplicate comments (1)
src/main/actions/makeApiClientRequest.js (1)
70-73: Fix multipart Content‑Type header assembly.Stringifying
formData.getHeaders()yields"[object Object]", producing an invalid header. Merge the returned object instead.✅ Suggested fix
body = formData; headers = { - "content-type": `${formData.getHeaders()}`, + ...formData.getHeaders(), ...headers, };
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/main/actions/makeApiClientRequest.js`:
- Around line 84-91: The fallback path for JSON detection uses strict equality
on apiRequest.contentType which fails for casing or charset variants; update the
fallback in makeApiClientRequest (the block using contentTypeHeader and
apiRequest.contentType) to mirror the header normalization: check
apiRequest.contentType?.toLowerCase()?.includes("application/json") (guarding
for null/undefined) instead of === "application/json" so charset and case
variants are treated as JSON.
♻️ Duplicate comments (1)
src/main/actions/makeApiClientRequest.js (1)
70-73: Bug: multipart Content-Type header becomes "[object Object]".
formData.getHeaders()returns an object; template-stringifying it yields"[object Object]", which is an invalid header value and breaks multipart uploads.Suggested fix
headers = { - "content-type": `${formData.getHeaders()}`, + ...formData.getHeaders(), ...headers, };
| if (contentTypeHeader) { | ||
| return headers[contentTypeHeader] | ||
| ?.toLowerCase() | ||
| ?.includes("application/json"); | ||
| } | ||
|
|
||
| return apiRequest.contentType === "application/json"; | ||
| })(); |
There was a problem hiding this comment.
Inconsistent Content-Type normalization in fallback path.
The header check normalizes (lowercase + includes), but the fallback uses strict equality on apiRequest.contentType. If apiRequest.contentType contains charset params or different casing (e.g., "application/json; charset=utf-8"), the fallback will incorrectly return false.
Suggested fix
if (contentTypeHeader) {
return headers[contentTypeHeader]
?.toLowerCase()
?.includes("application/json");
}
- return apiRequest.contentType === "application/json";
+ return apiRequest.contentType
+ ?.toLowerCase()
+ ?.includes("application/json");
})();🤖 Prompt for AI Agents
In `@src/main/actions/makeApiClientRequest.js` around lines 84 - 91, The fallback
path for JSON detection uses strict equality on apiRequest.contentType which
fails for casing or charset variants; update the fallback in
makeApiClientRequest (the block using contentTypeHeader and
apiRequest.contentType) to mirror the header normalization: check
apiRequest.contentType?.toLowerCase()?.includes("application/json") (guarding
for null/undefined) instead of === "application/json" so charset and case
variants are treated as JSON.
There was a problem hiding this comment.
apiRequest.contentType would not have anything else because it uses our custom enum in the UI
There was a problem hiding this comment.
@nafees87n, thank you for the clarification! That makes sense—if apiRequest.contentType is restricted to a custom enum in the UI, then the strict equality check is appropriate and safe. The normalization concern only applies to the headers path where values can be arbitrary.
The current implementation is fine as-is.
✏️ Learnings added
Learnt from: nafees87n
Repo: requestly/requestly-desktop-app PR: 272
File: src/main/actions/makeApiClientRequest.js:84-91
Timestamp: 2026-01-23T12:09:53.004Z
Learning: In `src/main/actions/makeApiClientRequest.js`, the `apiRequest.contentType` field uses a custom enum defined in the UI, so it will always be an exact value like `"application/json"` without charset parameters or casing variants. Strict equality checks against `apiRequest.contentType` are safe and appropriate.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
In application/json requests when the body string is not a valid json axios automatically stringifies it.
Refer axios' implementation: default transformRequest and safelyStringify.
As this is core of our API client, we want the users to have control over what data format they need to send instead of implicitly transforming the data.
Solution
This PR bypasses axios' default transformation when content type is
application/jsonand allows the data to be sent as is, in raw form, if request data is not a valid JSON string.Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.