[MAIN-Feat] Automated proxy testing system with intelligent selection strategies#94
Merged
Merged
Conversation
strategy is saved in the job param Updated the proxy schema to include proxy usage and injection numbers using socks agent lib to handle axios socks proxy injection
…ectly yet adding netBun fetch lib to handle socks proxies (this is temporary) Adding a system proxy testing job Adding system jobs database seeding feature, including the proxy testing job Adding testProxy endpoint Adding proxy strategy to jobConsumer
Contributor
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughAdds proxy selection strategies, proxy injection/use counters and migration, SOCKS/non‑SOCKS Axios injection utilities, a concurrent ProxyTestJob and API, system-job seeding, config/types, and repository functions for testing and counting proxies. ChangesProxy Testing and Usage Tracking Infrastructure
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
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 docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 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 `@src/config/config.ts`:
- Around line 307-313: The config schema for proxies.proxyTestingUrl accepts any
string and should validate at load time; update the proxyTestingUrl entry in the
config schema (symbol: proxyTestingUrl) to validate URLs by changing its format
to a URL validator or a custom validation function that accepts null but throws
on malformed URL strings (e.g. use format: 'url' if your config library supports
it or provide a format: (val) => val === null || isValidUrl(val)). Keep default:
null and nullable: true so only non-null values are validated.
In `@src/repositories/proxies.ts`:
- Around line 239-246: testProxy can hang because the outbound call uses
axiosInstance.get(targetUrl) with no timeout; ensure the proxy test always uses
a finite timeout by setting a timeout value when creating the instance via
ProxyTestingHttpService.create (or by passing a timeout option to
axiosInstance.get), e.g., add a configurable numeric timeout (e.g., 5000 ms)
into the axiosConfig used by ProxyTestingHttpService.create or include { timeout
} in the get call, and preserve existing error handling in the .catch so timeout
errors are handled the same way; update places referencing
ProxyTestingHttpService.create and injectProxyIntoInstance to use the
timeout-aware axios instance.
In `@src/systemJobs/proxyTestingJob.ts`:
- Around line 54-63: In the catch blocks inside the proxy testing routine (the
catch that logs "Error encountered when testing the proxy" and the similar one
at lines 85-88), stop throwing raw runtime errors and instead await the
emitError call and throw a normalized, serializable object: await
this.emitError(...) before throwing, and throw an object containing serializable
fields such as { message: String(err), name: err?.name, stack: err?.stack,
proxyName } (omit non-serializable properties); update both the catch after the
proxy test and the later catch (the one emitting `proxy test failed`) to follow
this pattern so exported results remain JSON-serializable.
- Around line 19-24: Validate and sanitize the job params before constructing
the config: ensure concurrentTestRequests and requestTimeout (from
job.param?.config and in proxyTestingJobParams) are numeric and greater than
zero (or clamp to safe defaults like 1 for concurrentTestRequests and 1000/60000
for requestTimeout) to prevent zero, negative or non-numeric values breaking
PromisePool/test logic; perform these checks where config is built (the const
config assignment) and replace invalid values with defaults or throw a clear
error so downstream functions consuming config (concurrentTestRequests,
requestTimeout) always receive valid positive numbers.
- Around line 18-26: The run method declares an unused parameter options which
triggers the linter; either consume that parameter or remove it. Fix by using
the passed-in options object where appropriate (replace references to
this.options?.config?.proxies?.proxyTestingUrl with
options?.config?.proxies?.proxyTestingUrl and any other places referring to
this.options inside async run) or, if the instance field is intended, remove the
options parameter from the run signature and update callers accordingly; key
symbols to check are the run method signature, the options parameter, and the
this.options?.config?.proxies?.proxyTestingUrl reference.
In `@src/utils/proxyUtils.ts`:
- Around line 68-70: When handling proxyPickingStrategy.SPECIFIC in the switch
for selecting a proxy, don't return undefined on a missing target; instead
detect if proxies.find((p) => p.proxy_id === targetId) returns falsy and throw a
descriptive error (e.g., include targetId and available proxy IDs) so the caller
fails fast and surfaces a clear configuration/runtime error; update the SPECIFIC
case in the proxy selection logic to perform this check and throw rather than
falling through to the default path.
- Around line 62-67: The code currently sorts descending and returns the
most-used proxy for both proxyPickingStrategy.ROUND_ROBIN and
proxyPickingStrategy.LEAST_USED; change the sort to ascending so the least-used
proxy is selected (e.g., use a.injection_count > b.injection_count ? 1 : -1 for
ROUND_ROBIN and a.use_count > b.use_count ? 1 : -1 for LEAST_USED), and avoid
mutating the original array by sorting a copy (e.g., [...proxies].sort(...))[0];
update references to proxyPickingStrategy, ROUND_ROBIN, LEAST_USED,
injection_count, and use_count accordingly.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 69828ed3-82d5-4ff8-8090-f21dca9b3502
⛔ Files ignored due to path filters (8)
bun.lockis excluded by!**/*.locksrc/generated/prisma/edge.jsis excluded by!**/generated/**src/generated/prisma/index-browser.jsis excluded by!**/generated/**src/generated/prisma/index.d.tsis excluded by!**/generated/**src/generated/prisma/index.jsis excluded by!**/generated/**src/generated/prisma/package.jsonis excluded by!**/generated/**src/generated/prisma/schema.prismais excluded by!**/generated/**src/generated/prisma/wasm.jsis excluded by!**/generated/**
📒 Files selected for processing (14)
package.jsonprisma/migrations/20260519183852_add_proxy_injection_and_use_count/migration.sqlprisma/schema.prismasrc/api/proxies/proxies.controller.tssrc/config/config.tssrc/initialization/dbSeed.tssrc/initialization/index.tssrc/jobConsumer/jobConsumer.tssrc/repositories/proxies.tssrc/systemJobs/proxyTestingJob.tssrc/types/proxies.tssrc/utils/httpRequestConfig.tssrc/utils/proxyUtils.tstsconfig.json
adding axios error redactor
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/systemJobs/proxyTestingJob.ts (1)
60-63:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon’t persist full probe responses in the scheduled job output.
testResponse.datais arbitrary remote content. Writing it for every proxy run can bloat output files and leak whatever the probe endpoint returns, while the PR objective only calls for status/counts and duration-style metrics.Suggested fix
return { - data: testResponse.data, + status: testResponse.status, duration, proxyName, };🤖 Prompt for 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. In `@src/systemJobs/proxyTestingJob.ts` around lines 60 - 63, The returned probe result currently includes testResponse.data which may contain arbitrary remote content; remove testResponse.data from the returned object in proxyTestingJob (the block that returns { data: testResponse.data, duration, proxyName, ... }) and instead return only safe metrics such as duration, proxyName, status (e.g., testResponse.status or success boolean), and any counts you need (responseSize as a number if you must but not raw body). Update any callers that expect data to use the new status/metrics fields.src/utils/proxyUtils.ts (1)
91-118:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftMake selection and counter updates atomic for strategy-based picking.
injectProxyreads the current counts, picks a proxy from that snapshot, and only incrementsinjection_countafterward. Under concurrent workers,ROUND_ROBINandLEAST_USEDcan repeatedly choose the same proxy because they all race on the same stale counts. Move the selection plus increment into one database-side atomic step if these strategies need to stay fair under load.🤖 Prompt for 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. In `@src/utils/proxyUtils.ts` around lines 91 - 118, The current flow calls getJobProxies + getProxyConfigWithStrategy to pick a proxy and then calls incrementProxyInjection afterwards, which allows races; instead implement a single DB-side atomic operation that selects a proxy according to the given strategy and increments its injection_count in the same transaction. Concretely: add a new DB helper (e.g., pickAndIncrementProxy or selectAndIncrementProxy) that takes jobId, proxyStrategy, optional targetProxyId and returns the chosen proxy row with its id; replace the getJobProxies/getProxyConfigWithStrategy + incrementProxyInjection sequence in this block with a single call to that new helper and use its returned proxyJob for injectProxyIntoInstance and for wiring the axios interceptor; leave incrementProxyUsage as an async per-request increment inside the interceptor. Ensure injectProxyIntoInstance and the interceptor reference the returned proxyJob.id.
🤖 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 `@src/systemJobs/proxyTestingJob.ts`:
- Line 6: Remove the unused import defaultRedactor from the top of
src/systemJobs/proxyTestingJob.ts; locate the import statement "import
defaultRedactor from \"`@utils/httpUtils/redactors`\";" and delete it (or replace
with a used export if intended), then run the linter to ensure no other unused
imports remain.
---
Outside diff comments:
In `@src/systemJobs/proxyTestingJob.ts`:
- Around line 60-63: The returned probe result currently includes
testResponse.data which may contain arbitrary remote content; remove
testResponse.data from the returned object in proxyTestingJob (the block that
returns { data: testResponse.data, duration, proxyName, ... }) and instead
return only safe metrics such as duration, proxyName, status (e.g.,
testResponse.status or success boolean), and any counts you need (responseSize
as a number if you must but not raw body). Update any callers that expect data
to use the new status/metrics fields.
In `@src/utils/proxyUtils.ts`:
- Around line 91-118: The current flow calls getJobProxies +
getProxyConfigWithStrategy to pick a proxy and then calls
incrementProxyInjection afterwards, which allows races; instead implement a
single DB-side atomic operation that selects a proxy according to the given
strategy and increments its injection_count in the same transaction. Concretely:
add a new DB helper (e.g., pickAndIncrementProxy or selectAndIncrementProxy)
that takes jobId, proxyStrategy, optional targetProxyId and returns the chosen
proxy row with its id; replace the getJobProxies/getProxyConfigWithStrategy +
incrementProxyInjection sequence in this block with a single call to that new
helper and use its returned proxyJob for injectProxyIntoInstance and for wiring
the axios interceptor; leave incrementProxyUsage as an async per-request
increment inside the interceptor. Ensure injectProxyIntoInstance and the
interceptor reference the returned proxyJob.id.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4842de86-91ce-42a0-822c-d52d64bb65e1
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
package.jsonsrc/api/proxies/proxies.controller.tssrc/repositories/proxies.tssrc/systemJobs/proxyTestingJob.tssrc/utils/httpRequestConfig.tssrc/utils/httpUtils/redactors.tssrc/utils/proxyUtils.ts
…ing message instead of an object
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
1 participant
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.
features:
/proxies/testProxyendpoint for manual on-demand testing of individual proxies with immediate status feedbackjobParamsobjectNotes
proxies.proxyTestingUrlenvironment variable so the tests function properly. This can also be configured in the config via the UI/ APIjobs.seedSystemJobsconfiguration optionMotivation and Context
Adding this feature has no goal to replace any more capable proxy setup, but to be able to give the local tasks more options natively as well as more information on error origins. This will help future debugging and monitoring
How Has This Been Tested?
/proxies/testProxyendpointScreenshots (if applicable)
N/A - Backend feature changes
Types of changes
Related Issues
Additional Context
Summary by CodeRabbit
New Features
Chores