-
Notifications
You must be signed in to change notification settings - Fork 2.1k
fix: improve Content-Type handling when request body is none (#6486) #6540
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughType guards were added to ensure Changes
Sequence Diagram(s)(omitted — changes are localized and do not introduce multi-component sequential flows requiring a diagram) Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used📓 Path-based instructions (1)**/*.{js,jsx,ts,tsx}📄 CodeRabbit inference engine (CODING_STANDARDS.md)
Files:
🧠 Learnings (2)📚 Learning: 2025-12-17T21:41:24.730ZApplied to files:
📚 Learning: 2025-12-02T07:24:50.311ZApplied to files:
🧬 Code graph analysis (1)packages/bruno-cli/src/runner/interpolate-vars.js (5)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
🔇 Additional comments (2)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/bruno-cli/src/runner/prepare-request.js (1)
383-383: Consider case consistency for Content-Type header.Line 383 uses
'Content-Type'(capitalized), while earlier in the file (lines 292, 303, 310, 317, 324, 332, 356, 363, 374) the code uses'content-type'(lowercase). While Axios may normalize header names, maintaining consistent casing improves code readability.🔎 Suggested change
- axiosRequest.headers['Content-Type'] = null; + axiosRequest.headers['content-type'] = null;
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/bruno-cli/src/runner/interpolate-vars.jspackages/bruno-cli/src/runner/prepare-request.js
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation. No tabs, just spaces
Stick to single quotes for strings. For JSX/TSX attributes, use double quotes (e.g., )
Always add semicolons at the end of statements
No trailing commas
Always use parentheses around parameters in arrow functions, even for single params
For multiline constructs, put opening braces on the same line, and ensure consistency. Minimum 2 elements for multiline
No newlines inside function parentheses
Space before and after the arrow in arrow functions.() => {}is good
No space between function name and parentheses.func()notfunc ()
Semicolons go at the end of the line, not on a new line
Names for functions need to be concise and descriptive
Add in JSDoc comments to add more details to the abstractions if needed
Add in meaningful comments instead of obvious ones where complex code flow is explained properly
Files:
packages/bruno-cli/src/runner/prepare-request.jspackages/bruno-cli/src/runner/interpolate-vars.js
🧠 Learnings (2)
📚 Learning: 2025-12-02T07:24:50.311Z
Learnt from: bijin-bruno
Repo: usebruno/bruno PR: 6263
File: packages/bruno-requests/src/auth/oauth2-helper.ts:249-249
Timestamp: 2025-12-02T07:24:50.311Z
Learning: In OAuth2 Basic Auth headers for Bruno, clientSecret is optional and can be omitted. When constructing the Authorization header in `packages/bruno-requests/src/auth/oauth2-helper.ts`, use `clientSecret || ''` instead of `clientSecret!` to properly handle cases where only clientId is provided, per community requests.
Applied to files:
packages/bruno-cli/src/runner/prepare-request.js
📚 Learning: 2025-12-17T21:41:24.730Z
Learnt from: naman-bruno
Repo: usebruno/bruno PR: 6407
File: packages/bruno-app/src/components/Environments/ConfirmCloseEnvironment/index.js:5-41
Timestamp: 2025-12-17T21:41:24.730Z
Learning: Do not suggest PropTypes validation for React components in the Bruno codebase. The project does not use PropTypes, so reviews should avoid proposing PropTypes and rely on the existing typing/validation approach (e.g., TypeScript or alternative runtime checks) if applicable. This guideline applies broadly to all JavaScript/JSX components in the repo.
Applied to files:
packages/bruno-cli/src/runner/prepare-request.jspackages/bruno-cli/src/runner/interpolate-vars.js
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Playwright E2E Tests
- GitHub Check: Unit Tests
- GitHub Check: CLI Tests
- GitHub Check: SSL Tests - macOS
- GitHub Check: SSL Tests - Linux
- GitHub Check: SSL Tests - Windows
🔇 Additional comments (4)
packages/bruno-cli/src/runner/interpolate-vars.js (4)
13-14: LGTM! Proper type guard for contentType.The guard prevents runtime errors when
contentTypeis null or other non-string values, which can now happen with the changes in prepare-request.js. The empty string fallback is appropriate.
69-69: LGTM! Type guard prevents runtime error.The guard ensures
.includes()is only called on string values. This prevents TypeErrors when contentType is null.
83-83: LGTM! Consistent type guard.Type guard ensures string comparison is safe and aligns with the pattern used throughout the file.
90-90: LGTM! Complete type guard coverage.This completes the type guard pattern for all contentType checks in the interpolation logic. All branches now safely handle non-string contentType values.
a42dab3 to
8e91d30
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/bruno-cli/src/runner/interpolate-vars.js (1)
69-69: Type guards are technically redundant but acceptable.Since
getContentTypenow always returns a string (line 14), thesetypeofchecks are redundant. However, they make the code more explicit and provide defense-in-depth ifgetContentTypeis modified later. The consistency across all three branches is also a plus.Also applies to: 83-83, 90-90
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/bruno-cli/src/runner/interpolate-vars.jspackages/bruno-cli/src/runner/prepare-request.js
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/bruno-cli/src/runner/prepare-request.js
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation. No tabs, just spaces
Stick to single quotes for strings. For JSX/TSX attributes, use double quotes (e.g., )
Always add semicolons at the end of statements
No trailing commas
Always use parentheses around parameters in arrow functions, even for single params
For multiline constructs, put opening braces on the same line, and ensure consistency. Minimum 2 elements for multiline
No newlines inside function parentheses
Space before and after the arrow in arrow functions.() => {}is good
No space between function name and parentheses.func()notfunc ()
Semicolons go at the end of the line, not on a new line
Names for functions need to be concise and descriptive
Add in JSDoc comments to add more details to the abstractions if needed
Add in meaningful comments instead of obvious ones where complex code flow is explained properly
Files:
packages/bruno-cli/src/runner/interpolate-vars.js
🧠 Learnings (1)
📚 Learning: 2025-12-17T21:41:24.730Z
Learnt from: naman-bruno
Repo: usebruno/bruno PR: 6407
File: packages/bruno-app/src/components/Environments/ConfirmCloseEnvironment/index.js:5-41
Timestamp: 2025-12-17T21:41:24.730Z
Learning: Do not suggest PropTypes validation for React components in the Bruno codebase. The project does not use PropTypes, so reviews should avoid proposing PropTypes and rely on the existing typing/validation approach (e.g., TypeScript or alternative runtime checks) if applicable. This guideline applies broadly to all JavaScript/JSX components in the repo.
Applied to files:
packages/bruno-cli/src/runner/interpolate-vars.js
🧬 Code graph analysis (1)
packages/bruno-cli/src/runner/interpolate-vars.js (4)
packages/bruno-tests/src/index.js (1)
contentType(30-30)packages/bruno-electron/src/ipc/network/interpolate-vars.js (2)
contentType(6-6)contentType(76-76)packages/bruno-app/src/utils/codegenerator/har.js (2)
contentType(39-39)contentType(69-69)packages/bruno-app/src/utils/collections/index.js (2)
request(588-588)request(1065-1065)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Playwright E2E Tests
- GitHub Check: SSL Tests - Linux
- GitHub Check: CLI Tests
- GitHub Check: Unit Tests
- GitHub Check: SSL Tests - Windows
- GitHub Check: SSL Tests - macOS
🔇 Additional comments (1)
packages/bruno-cli/src/runner/interpolate-vars.js (1)
13-14: LGTM! Defensive type guard improves robustness.Returning empty string for non-string content-type values prevents potential runtime errors and ensures consistent behavior when headers contain unexpected values.
8e91d30 to
ceada14
Compare
Fixes: #6486
Related to: #1693
Description
The Bruno CLI was sending an unwanted
Content-Type: application/x-www-form-urlencodedheader for requests with no body. This fix ensures that requests with no body do not send a Content-Type header, matching the behavior of the Electron version.Contribution Checklist:
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.