Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds SOCKS proxy support to both the Rust core (impit) and the Node binding (impit-node), and includes a basic end‐to‐end test for the new feature.
- Enabled the
socksfeature inreqwestfor Rust proxy support - Added
socksv5dependency and SOCKS server setup in the Node tests - Introduced a new
supports socks proxytest inbasics.test.ts
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| impit/Cargo.toml | Enabled the socks feature flag for the reqwest crate |
| impit-node/package.json | Added socksv5 as a development/test dependency |
| impit-node/test/basics.test.ts | Imported and started a SOCKS server; added new test case |
Comments suppressed due to low confidence (2)
impit-node/test/basics.test.ts:131
- [nitpick] For consistency with other tests prefixed by
impit, consider renaming this test toimpit supports socks proxy.
test('supports socks proxy', async (t) => {
impit/Cargo.toml:15
- [nitpick] While the Rust client now supports SOCKS, there are no integration tests verifying proxy usage on the Rust side. Consider adding a minimal end-to-end test to cover the new feature.
reqwest = { version = "0.12.9", features = ["json", "gzip", "brotli", "zstd", "deflate", "rustls-tls", "http3", "cookies", "stream", "socks"] }
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Enables support for
socksproxies toimpit-node. This theoretically enablessocksproxies for CLI and the Python binding as well, but this behaviour is untested due to a lack of working socks proxy server implementations in Python.