fix: restore default axios request headers#7820
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughChanged Axios instance initialization to set Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/bruno-cli/src/utils/axios-instance.js`:
- Line 96: The CLI axios instance sets default headers
(instance.defaults.headers.common['User-Agent'] and the default Accept header)
but lacks test coverage; add a unit test mirroring the Electron tests that loads
the CLI axios instance module and asserts the default 'User-Agent' equals
`bruno-runtime/${CLI_VERSION}` and that the default 'Accept' header is
present/expected. Create or update a test file (e.g., bruno-cli tests under
tests/network/axios-instance.spec.js) to import the axios-instance module used
in packages/bruno-cli/src/utils/axios-instance.js, and add assertions for
instance.defaults.headers.common['User-Agent'] and
instance.defaults.headers.common['Accept'] to ensure parity with the Electron
test suite.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1a780940-290c-41d2-88c1-d4139a1d951e
📒 Files selected for processing (3)
packages/bruno-cli/src/utils/axios-instance.jspackages/bruno-electron/src/ipc/network/axios-instance.jspackages/bruno-electron/tests/network/axios-instance.spec.js
|
Hey @j4ln, thanks for your contribution to this PR. Could you please review and merge the PR I have raised here: j4ln#1? I’ve slightly updated your tests, consolidated your commits into one, and added proper comments. Let me know if you have any doubts. Please merge it and let me know so I can pass it to the team for review. |
Assigning `defaults.headers.common = { 'User-Agent': ... }` replaced the
entire common headers object, nuking axios's built-in default:
Accept: application/json, text/plain, */*
This caused servers relying on content-negotiation to receive requests
with no Accept header. Fix by extending the existing object with a
property assignment instead.
Add regression tests for both electron and CLI axios instances verifying
that Accept is preserved and User-Agent is set correctly.
Co-Authored-By: James <j4ln@posteo.net>
Co-Authored-By: Pragadesh-45 <temporaryg7904@gmail.com>
38de3f6 to
bb1ccda
Compare
|
Hey @Pragadesh44-Bruno thanks for the review. I've merged those changes in now. I've re consolidated the commits into a single one and rebased against main. |
Description
Fixes #7819.
Contribution Checklist:
Summary by CodeRabbit
Bug Fixes
Tests