-
Notifications
You must be signed in to change notification settings - Fork 5
feat: Add cache and retry support #14
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
📝 WalkthroughSummary by CodeRabbit
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
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 |
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.
Actionable comments posted: 2
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: McQuenji <60017181+mcquenji@users.noreply.github.com>
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.
Actionable comments posted: 2
♻️ Duplicate comments (1)
lib/flutter_single_instance.dart (1)
153-172: Loop still performs no retries whenmaxRetries=1.With
while (attempt < maxRetries)the defaultmaxRetries = 1yields exactly one attempt and zero retries, which is the same problem previously flagged. Please iterate from the initial attempt through the configured retry count (inclusive) so callers actually get the number of retries they requested.- bool result = false; - int attempt = 0; - - while (attempt < maxRetries) { + bool result = false; + + for (var attempt = 0; attempt <= maxRetries; attempt++) { // Wait before retrying (skip for first attempt). if (attempt > 0) { logger.finest( "Retrying activateInstance (attempt $attempt/$maxRetries)"); await Future.delayed(retryInterval); } - result = await activateInstance(processName); + result = await activateInstance(resolvedProcessName); if (result) { break; // Exit loop if activation succeeds. } - - attempt++; }
lib/flutter_single_instance.dart
Outdated
| assert(maxRetries >= 1, 'maxRetries must be greater than or equal to 1'); | ||
| assert(retryInterval != Duration.zero, 'retryInterval must be non-zero'); |
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.
Replace debug-only asserts with runtime argument validation.
assert disappears in release builds, so bad inputs like maxRetries = 0 reach production and the method never even attempts activation. Please guard these parameters with real checks (e.g., throw ArgumentError) so the runtime contract is enforced outside debug mode.(dart.dev)
- if (_isFirstInstance != null) {
- return _isFirstInstance!;
- }
-
-assert(maxRetries >= 1, 'maxRetries must be greater than or equal to 1');
-assert(retryInterval != Duration.zero, 'retryInterval must be non-zero');
+ if (_isFirstInstance != null) {
+ return _isFirstInstance!;
+ }
+
+ if (maxRetries < 1) {
+ throw ArgumentError.value(
+ maxRetries,
+ 'maxRetries',
+ 'must be greater than or equal to 1',
+ );
+ }
+ if (retryInterval == Duration.zero) {
+ throw ArgumentError.value(
+ retryInterval,
+ 'retryInterval',
+ 'must be non-zero',
+ );
+ }🤖 Prompt for AI Agents
In lib/flutter_single_instance.dart around lines 140 to 141, the code uses
debug-only asserts for validating maxRetries and retryInterval; replace those
asserts with runtime argument validation by throwing ArgumentError (e.g.,
ArgumentError.value) when maxRetries < 1 or retryInterval == Duration.zero,
providing clear messages and parameter names so the contract is enforced in
release builds.
mcquenji
left a comment
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.
Thanks for your contribution!
Introduce the maxRetries and retryInterval in isFirstInstance