fix: Request Filters improvements & SOCKS proxy fix#501
Conversation
…lters The getDistinctProviderGroupsAction was showing group tags from soft-deleted providers. Added deletedAt IS NULL filter to match the behavior of getDistinctProviderGroups in repository/provider.ts. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add a switch toggle to filter providers by circuit breaker state. The toggle appears only when there are providers with open circuit breaker. When enabled, shows only providers with broken circuits. - Filter works on top of existing filters (type, status, group, search) - Visual feedback: icon and text turn red when filter is active - Added i18n translations for all 5 languages 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…patibility SocksProxyAgent from socks-proxy-agent is a Node.js HTTP Agent that doesn't implement undici's Dispatcher interface, causing "this.dispatch is not a function" error when used with undici.request(). Replace with fetch-socks which provides socksDispatcher - a native undici Dispatcher implementation for SOCKS4/SOCKS5 proxies. - Replace socks-proxy-agent with fetch-socks dependency - Update createProxyAgentForProvider to use socksDispatcher - Update next.config.ts outputFileTracingIncludes - Update comments in forwarder.ts 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary of ChangesHello @miraserver, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request delivers several key improvements and fixes across the application. It refines the Request Filters by ensuring that only active providers are considered when displaying group tags, preventing stale data from appearing. A new user interface feature has been added to the Providers page, allowing users to easily filter providers based on their circuit breaker status, enhancing monitoring capabilities. Furthermore, a critical fix for SOCKS proxy connections has been implemented, resolving compatibility issues with the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This PR delivers three valuable improvements. The SOCKS proxy fix by switching to fetch-socks is a great move for better compatibility with undici. The new circuit breaker filter on the providers page is a useful feature, and its implementation is clean and efficient. The fix to exclude deleted providers from the request filters dropdown is also a necessary correction. The code is well-written, but I have one suggestion for src/actions/request-filters.ts to improve code consistency.
| .where( | ||
| and( | ||
| isNull(providers.deletedAt), | ||
| sql`${providers.groupTag} IS NOT NULL AND ${providers.groupTag} != ''` | ||
| ) | ||
| ); |
There was a problem hiding this comment.
While using sql works, it's more idiomatic and type-safe to use drizzle-orm's built-in operators for this query. This would also make the code more consistent with other queries in the codebase. You can achieve the same logic using isNotNull, ne, and a nested and. You'll also need to add isNotNull and ne to your drizzle-orm import.
| .where( | |
| and( | |
| isNull(providers.deletedAt), | |
| sql`${providers.groupTag} IS NOT NULL AND ${providers.groupTag} != ''` | |
| ) | |
| ); | |
| .where( | |
| and( | |
| isNull(providers.deletedAt), | |
| and(isNotNull(providers.groupTag), ne(providers.groupTag, "")) | |
| ) | |
| ); |
There was a problem hiding this comment.
Code Review Summary
No significant issues identified in this PR.
PR Size: S
- Lines changed: 156 (112 additions, 44 deletions)
- Files changed: 11
Changes Reviewed
1. Request Filters: Exclude deleted providers from dropdown
- Added
deletedAt IS NULLfilter togetDistinctProviderGroupsAction - Matches repository pattern in
getDistinctProviderGroups - Correctly prevents soft-deleted provider groups from appearing in UI
2. Providers Page: Circuit breaker filter toggle
- Clean implementation of circuit breaker state filter
- Proper memoization with correct dependencies
- Good UX with visual feedback and conditional visibility
3. SOCKS Proxy: Replace socks-proxy-agent with fetch-socks
- Replaced non-undici-compatible
SocksProxyAgentwithsocksDispatcher - Fixes "this.dispatch is not a function" error
- Proper type safety: removed
anytype, usesDispatcherinterface - Error handling and logging preserved
- Dependency properly added to package.json and next.config.ts
Review Coverage
- Logic and correctness - Clean
- Security (OWASP Top 10) - Clean
- Error handling - Clean
- Type safety - Improved (removed
anytype) - Documentation accuracy - Clean
- Test coverage - Adequate
- Code clarity - Good
Automated review by Claude AI
…filtering Replace sql template with isNotNull and ne operators for better type-safety and consistency with drizzle-orm patterns. Changes in both: - src/actions/request-filters.ts - src/repository/provider.ts 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
63d0aa6 to
1b8577f
Compare
Summary
This PR contains three fixes/improvements:
1. Fix: Exclude deleted providers from group tags dropdown in Request Filters
getDistinctProviderGroupsActionwas showing group tags from soft-deleted providersdeletedAt IS NULLfilter to match the behavior ofgetDistinctProviderGroupsin repository2. Feature: Add circuit breaker filter toggle on Providers page
3. Fix: SOCKS proxy not working with undici ("this.dispatch is not a function")
SocksProxyAgentfromsocks-proxy-agentis a Node.js HTTP Agent that doesn't implement undici'sDispatcherinterfacefetch-sockswhich providessocksDispatcher- a native undici Dispatcher implementationChanges
src/actions/request-filters.ts- Added deletedAt filtersrc/app/[locale]/settings/providers/_components/provider-manager.tsx- Circuit breaker toggle UIsrc/lib/proxy-agent.ts- Replace SocksProxyAgent with socksDispatchernext.config.ts- Update outputFileTracingIncludesmessages/*/settings.json- i18n for circuit breaker filterTest plan
🤖 Generated with Claude Code
Greptile Summary
This PR addresses three distinct improvements:
1. SOCKS Proxy Compatibility Fix: Replaced
socks-proxy-agentwithfetch-socksto resolve undici compatibility issues. The originalSocksProxyAgentis a Node.js HTTP Agent that doesn't implement undici'sDispatcherinterface, causing "this.dispatch is not a function" errors. The newsocksDispatcherfromfetch-socksis a native undici Dispatcher implementation that properly integrates with the undici request pipeline.2. Request Filters Bug Fix: Fixed
getDistinctProviderGroupsActionto exclude soft-deleted providers (deletedAt IS NULL), aligning with the existinggetDistinctProviderGroupsrepository method. This prevents deleted provider group tags from appearing in the dropdown filter.3. Circuit Breaker Filter Feature: Added a toggle switch on the Providers page to filter providers by circuit breaker state. The toggle only appears when there are providers with open circuit breakers, provides visual feedback (red icon and text when active), and works seamlessly with existing filters (type, status, group, search).
All changes are well-tested, properly documented, and include i18n translations for all supported languages.
Confidence Score: 5/5
Important Files Changed
Sequence Diagram
sequenceDiagram participant Client participant ProxyAgent participant SocksDispatcher participant UndiciRequest participant TargetAPI Note over ProxyAgent: Before: SocksProxyAgent (incompatible) Note over ProxyAgent: After: fetch-socks socksDispatcher Client->>ProxyAgent: createProxyAgentForProvider(provider, targetUrl) alt SOCKS5/SOCKS4 Protocol ProxyAgent->>SocksDispatcher: socksDispatcher({type, host, port}, {timeout}) Note right of SocksDispatcher: Native undici Dispatcher ProxyAgent-->>Client: {agent: Dispatcher, http2Enabled: false} else HTTP/HTTPS Protocol ProxyAgent->>ProxyAgent: new ProxyAgent({uri, allowH2, timeouts}) ProxyAgent-->>Client: {agent: ProxyAgent, http2Enabled: true} end Client->>UndiciRequest: undiciRequest(url, {dispatcher: agent}) UndiciRequest->>SocksDispatcher: dispatch(request) SocksDispatcher->>TargetAPI: Forward via SOCKS proxy TargetAPI-->>SocksDispatcher: Response SocksDispatcher-->>UndiciRequest: Response UndiciRequest-->>Client: Response