Add support for socks5h (use domain name rather than resolved IP)#8341
Add support for socks5h (use domain name rather than resolved IP)#8341eikelang-vm wants to merge 2 commits into
Conversation
|
Caution Review failedAn error occurred during the review process. Please try again later. WalkthroughAdds Changessocks5h Proxy Protocol Support
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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: 2
🤖 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.
Inline comments:
In `@packages/bruno-app/src/components/CollectionSettings/ProxySettings/index.js`:
- Around line 304-311: The radio input element with value="socks5h" in the
ProxySettings component is missing a data-testid attribute, which is required
for Playwright test automation. Add a data-testid attribute to this radio input
with a descriptive value that identifies it as the SOCKS5h protocol selector
(for example, something like "proxy-protocol-socks5h" or similar following the
existing naming conventions in the codebase).
In `@packages/bruno-app/src/components/Preferences/ProxySettings/index.js`:
- Around line 261-268: The radio input element for the SOCKS5h protocol option
(the input with value="socks5h" and name="config.protocol") is missing a
data-testid attribute required for Playwright test coverage. Add a data-testid
attribute to this radio input element with a descriptive test identifier
following the naming convention used in the component. This will allow test
selectors to target the element reliably without relying on brittle CSS or
attribute-based selectors.
🪄 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: 36a3f5b6-2fa7-4065-a4b2-09fb0ecaf2c8
📒 Files selected for processing (4)
packages/bruno-app/src/components/CollectionSettings/ProxySettings/index.jspackages/bruno-app/src/components/Preferences/ProxySettings/index.jspackages/bruno-electron/src/store/preferences.jspackages/bruno-electron/test/proxy-util.test.js
Description
Adds SOCKS5h as a proxy protocol option (Preferences ▸ Proxy and Collection ▸ Proxy) so DNS resolution happens on the proxy side instead of locally — needed when the target only resolves from the proxy's network.
socks5resolves the hostname locally and sends the IP;socks5hforwards the hostname to the proxy. The agent layer already routes anysocks*protocol throughsocks-proxy-agent(which parsessocks5has remote-DNS), so only the protocol enums + UI radios needed adding, plus a regression test.I made this change primarily to address a personal need, but the resulting diff was small and focused enough that I considered offering it as a PR, especially considering that I was able to find at least two issues directly requesting this feature/pointing out the fact that SOCKS proxy requests currently fail if the name can only be resolved on the side of the proxy.
Depending on your point of view, the SOCKS5h support might be considered incomplete, because
The option to use the SOCKS5h flavor is exposed next to the existing proxy options in the preferences:

Contribution Checklist: