Improved: lowered timeout and connect_timeout to 10 and 5 seconds.#290
Improved: lowered timeout and connect_timeout to 10 and 5 seconds.#290
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthrough
Changes
Sequence Diagram(s)sequenceDiagram
participant WP as WordPress (filters)
participant Client as Client::__construct
participant Guzzle as GuzzleClient
WP->>Client: provide `plausible_analytics_api_connect_timeout` (default 5.0)
WP->>Client: provide `plausible_analytics_api_timeout` (default 10.0)
Client->>Guzzle: instantiate with { connect_timeout: 5.0, timeout: 10.0 }
Guzzle-->>Client: configured HTTP client returned
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/Client.php (1)
43-43: Make timeout values filterable instead of hardcoded.Line 43 hardcodes network limits globally. Consider exposing these via WP filters so slower environments can tune values without code edits.
Diff suggestion
- $this->api_instance = new DefaultApi( new GuzzleClient( [ 'timeout' => 10, 'connect_timeout' => 5 ] ), $config ); + $timeout = (float) apply_filters( 'plausible_analytics_http_timeout', 10.0 ); + $connect_timeout = (float) apply_filters( 'plausible_analytics_http_connect_timeout', 5.0 ); + $this->api_instance = new DefaultApi( + new GuzzleClient( + [ + 'timeout' => $timeout, + 'connect_timeout' => $connect_timeout, + ] + ), + $config + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Client.php` at line 43, The hardcoded timeouts passed to the GuzzleClient in the Client class (where DefaultApi is instantiated) should be exposed via WP filters so environments can override them; replace the literal 10 and 5 with calls to apply_filters (e.g. apply_filters('your_plugin_api_timeout', 10) and apply_filters('your_plugin_connect_timeout', 5)), cast the results to int/float as needed, and use those filtered values when constructing the new GuzzleClient in the Client.php constructor where DefaultApi is created.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/Client.php`:
- Line 43: The hardcoded timeouts passed to the GuzzleClient in the Client class
(where DefaultApi is instantiated) should be exposed via WP filters so
environments can override them; replace the literal 10 and 5 with calls to
apply_filters (e.g. apply_filters('your_plugin_api_timeout', 10) and
apply_filters('your_plugin_connect_timeout', 5)), cast the results to int/float
as needed, and use those filtered values when constructing the new GuzzleClient
in the Client.php constructor where DefaultApi is created.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Client.php`:
- Around line 43-45: The filtered timeout values can end up as 0.0 if
non-numeric input is returned; before passing to DefaultApi/GuzzleClient
normalize $timeout and $connect_timeout by validating and coercing them to
positive numbers and falling back to the intended defaults (10.0 and 5.0).
Specifically, after calling apply_filters('plausible_analytics_api_timeout',
10.0) and apply_filters('plausible_analytics_api_connect_timeout', 5.0) validate
the results (e.g., is_numeric and greater than 0) and assign the default if not,
then use the sanitized $timeout and $connect_timeout when constructing new
GuzzleClient and DefaultApi.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 63c95eb0-429b-40bb-9a4a-bb952c2716d1
📒 Files selected for processing (2)
README.mdsrc/Client.php
✅ Files skipped from review due to trivial changes (1)
- README.md
1 participant
Summary by CodeRabbit
Bug Fixes
Documentation
Chores