-
Notifications
You must be signed in to change notification settings - Fork 29.7k
[tool] Further cleanup of proxy logic #178683
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Move getFinalTargetUri to ProxyRule class Fix doc comments Pares URI immediately when creating RegexProxyRule
| final Uri finalTargetUrl = rule.finalTargetUri(request.requestedUri); | ||
| try { | ||
| final shelf.Request proxyBackendRequest = proxyRequest(request, finalTargetUrl); | ||
| final shelf.Response proxyResponse = await proxyHandler(targetUri)(proxyBackendRequest); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't like createFunction(args)(invokeArgs). Fixed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request refactors the proxy logic by moving getFinalTargetUri into the ProxyRule class and parsing the target URI earlier in the RegexProxyRule constructor. This is a good cleanup that improves encapsulation and efficiency. The changes are well-executed, but I've found a couple of issues with error handling that were introduced by this refactoring. Specifically, invalid target URIs in the configuration will now cause an unhandled exception. I've also added a minor comment about missing documentation to adhere to the style guide.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
| RegexProxyRule({required RegExp pattern, required String target, String? replacement}) | ||
| : _pattern = pattern, | ||
| _target = target, | ||
| : targetUri = Uri.parse(target), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the provided target is invalid, better to find out HERE than when the proxy is handling requests!
flutter/flutter@cb7b7df...de4be4f 2025-11-19 [email protected] Roll Skia from 2054d87c6a85 to b5dc8c3494ac (1 revision) (flutter/flutter#178793) 2025-11-19 [email protected] Roll Skia from a30b02d57637 to 2054d87c6a85 (2 revisions) (flutter/flutter#178787) 2025-11-19 [email protected] Roll Skia from 547e3e5441b4 to a30b02d57637 (3 revisions) (flutter/flutter#178782) 2025-11-19 [email protected] Fix train hopping animation status listeners (flutter/flutter#178372) 2025-11-19 [email protected] Make sure that a ReorderableListView doesn't crash in 0x0 environment (flutter/flutter#177646) 2025-11-19 [email protected] Roll Skia from 9ce01a452f63 to 547e3e5441b4 (1 revision) (flutter/flutter#178775) 2025-11-19 [email protected] Roll Dart SDK from 1ed6b56bb323 to f7e9bd245fd9 (1 revision) (flutter/flutter#178774) 2025-11-19 [email protected] Roll Skia from f3ddc700abc7 to 9ce01a452f63 (8 revisions) (flutter/flutter#178769) 2025-11-19 [email protected] Make sure that a TabPageSelector doesn't crash in 0x0 environment (flutter/flutter#178156) 2025-11-19 [email protected] Small cleanup in `DeferredComponentManager.java` (flutter/flutter#178585) 2025-11-19 [email protected] Roll Dart SDK from a33149cb6643 to 1ed6b56bb323 (1 revision) (flutter/flutter#178763) 2025-11-18 [email protected] Roll Skia from 8557300f84c2 to f3ddc700abc7 (5 revisions) (flutter/flutter#178751) 2025-11-18 [email protected] Remove unnecessary `String.valueOf` in `TextInputChannel.java` (flutter/flutter#178592) 2025-11-18 [email protected] [tool] Further cleanup of proxy logic (flutter/flutter#178683) 2025-11-18 [email protected] Restore OpenGL state modified by fl_compositor_opengl_present_layers (flutter/flutter#178697) 2025-11-18 [email protected] Replace `equals("")` with `isEmpty` in `SpellCheckPlugin.java` (flutter/flutter#178596) 2025-11-18 [email protected] Roll Dart SDK from 312845b06afc to a33149cb6643 (2 revisions) (flutter/flutter#178738) 2025-11-18 [email protected] Roll Skia from ca906091e199 to 8557300f84c2 (2 revisions) (flutter/flutter#178739) 2025-11-18 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Add framework-side hitTestBehavior support for Semantics widget and apply to ModalRoute (#177570)" (flutter/flutter#178744) 2025-11-18 [email protected] Roll Packages from ce44ebb to 34746bb (6 revisions) (flutter/flutter#178734) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Move getFinalTargetUri to ProxyRule class Fix doc comments Pares URI immediately when creating RegexProxyRule --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Move getFinalTargetUri to ProxyRule class Fix doc comments Pares URI immediately when creating RegexProxyRule --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Move getFinalTargetUri to ProxyRule class Fix doc comments Pares URI immediately when creating RegexProxyRule --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Move getFinalTargetUri to ProxyRule class
Fix doc comments
Pares URI immediately when creating RegexProxyRule