Adds proxy support using the Alpha client - #1178
Conversation
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull Request Overview
This pull request adds proxy support to the JupiterOne integration SDK by integrating proxy configuration with the Alpha HTTP client. The changes enable integration developers to route API requests through HTTP/HTTPS proxies for network environments that require them.
Key changes:
- Extended the
createApiClientfunction to accept an optionalproxyUrlparameter and automatically detect proxy settings from environment variables - Added proxy URL parsing functionality with authentication support and error handling
- Implemented comprehensive test coverage for various proxy configuration scenarios
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/integration-sdk-runtime/src/api/index.ts | Core implementation adding proxy support with URL parsing and environment variable detection |
| packages/integration-sdk-runtime/src/api/tests/index.test.ts | Comprehensive test suite covering proxy configuration scenarios and edge cases |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| const url = new URL(proxyUrl); | ||
| const proxy: AxiosProxyConfig = { | ||
| host: url.hostname, | ||
| port: parseInt(url.port) || (url.protocol === 'https:' ? 443 : 80), |
There was a problem hiding this comment.
The parseInt function should specify a radix parameter to avoid potential parsing issues. Use parseInt(url.port, 10) to ensure decimal parsing.
| port: parseInt(url.port) || (url.protocol === 'https:' ? 443 : 80), | |
| port: parseInt(url.port, 10) || (url.protocol === 'https:' ? 443 : 80), |
| const proxy: AxiosProxyConfig = { | ||
| host: url.hostname, | ||
| port: parseInt(url.port) || (url.protocol === 'https:' ? 443 : 80), | ||
| protocol: url.protocol.replace(':', ''), |
There was a problem hiding this comment.
[nitpick] The protocol manipulation could be more explicit. Consider using url.protocol.slice(0, -1) or add a comment explaining that this removes the trailing colon from protocols like 'https:' to get 'https'.
| protocol: url.protocol.replace(':', ''), | |
| // Remove the trailing colon from protocol (e.g., 'https:' -> 'https') | |
| protocol: url.protocol.slice(0, -1), |
| return proxy; | ||
| } catch (error) { | ||
| const parsedError = error instanceof TypeError ? error : new TypeError(String(error)); | ||
| console.warn('Failed to parse proxy URL:', proxyUrl, parsedError); |
There was a problem hiding this comment.
this code only executes if proxyUrlString is populated, so if there is a proxy config, and we cannot parse it, its not safe to continue without the proxy
There was a problem hiding this comment.
Agreed! I modified it to not catch the error.
For https://jupiterone.atlassian.net/browse/INT-12936