feat: Add optional exponential backoff for polling interval (#99)#139
feat: Add optional exponential backoff for polling interval (#99)#139mem-5514-tahara wants to merge 13 commits into
Conversation
When `useExponentialBackoff` is enabled, the retry interval grows exponentially on consecutive failures and resets to `checkInterval` on reconnect, reducing unnecessary battery drain and network traffic during prolonged outages. New `createInstance` parameters: - `useExponentialBackoff` (bool, default false) - `backoffInitialDelay` (Duration?, default: checkInterval) - `backoffMaxDelay` (Duration, default: 60s) - `backoffMultiplier` (double, default: 2.0) Closes OutdatedGuy#99
There was a problem hiding this comment.
Pull request overview
Adds an opt-in exponential backoff strategy to the InternetConnection.createInstance polling loop to reduce polling frequency during extended disconnects (while preserving existing behavior by default), and extends the test suite to cover the new timing behavior.
Changes:
- Introduces
useExponentialBackoffand related backoff configuration parameters onInternetConnection.createInstance. - Implements adaptive timer rescheduling in
_maybeEmitStatusUpdate()based on connection status and backoff state. - Adds a new
exponentialBackofftest group to validate backoff growth, capping, and reset behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
lib/src/internet_connection.dart |
Adds backoff configuration/state and uses it to compute the next polling delay. |
test/internet_connection_test.dart |
Adds tests intended to validate backoff timing and reset behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
As pointed out by Copilot, calling `setIntervalAndResetTimer` while already disconnected would cause the next poll to be treated as an ongoing failure (immediately multiplying the delay) because `previousStatus` remained disconnected. This commit introduces a `_backoffNeedsReset` flag to force the next poll to be treated as a "first failure" without mutating `_lastStatus` (which would have caused duplicate disconnected events). Also includes a new test to verify implicit initial delay synchronization.
- Update assertion for `backoffMultiplier` to require a value >= 1.0. Values between 0 and 1 would cause the polling delay to decay toward 0ms, leading to a tight polling loop during an outage.
- Add assertions to ensure `backoffInitialDelay` (or its fallback `checkInterval`) is greater than zero to prevent immediate/negative timer loops. - Add assertion to ensure `backoffInitialDelay` is less than or equal to `backoffMaxDelay`, keeping the backoff sequence logically sound and preventing the delay from shrinking on the second poll.
…y on first failure
Replace always-true expect(sub, isNotNull) with real behavioral checks: verify call count (~4-5 in 550ms) and that consecutive gaps stay under 200ms, catching regressions where backoff activates unintentionally.
|
Hi @OutdatedGuy! I’ve addressed the issues GitHub Copilot flagged, so could you please take another look at my proposal? |
|
@mem-5514-tahara can you disclose if this PR heavily AI generated or just AI assisted? |
|
@OutdatedGuy I used Claude Code to help investigate the issue and draft the initial logic and tests. I manually guided the design decisions (such as keeping strict backward compatibility) and reviewed the code, but I definitely missed a few things—as Copilot just rightly pointed out with that race condition during cancellation! 😅 I'm taking full responsibility for the code. I will apply Copilot's suggested fixes (the race condition check and dart format) and push them right away. Please let me know if you spot anything else! |
Re-check hasListener immediately before scheduling the next timer so that a cancel arriving during the await internetStatus suspension does not restart polling with zero listeners.
… builds Replace assert()-only guards with ArgumentError throws so that invalid backoff parameters (multiplier < 1.0, zero delays, initialDelay > maxDelay) are caught in release/AOT builds, not just debug mode.
Introduce a monotonically-increasing `_generation` counter that is bumped on every reschedule (setIntervalAndResetTimer) and on cancel (_handleStatusChangeCancel). Each _maybeEmitStatusUpdate invocation captures the counter on entry and bails out before mutating _currentBackoffDelay / _backoffNeedsReset if the value has changed, preventing a stale callback from silently overwriting the reset that the newer owner already applied.
… initial delay computation
…scribers Introduce _cancelGeneration, bumped only on _handleStatusChangeCancel, to distinguish cancel+resubscribe cycles from setIntervalAndResetTimer calls. _maybeEmitStatusUpdate now snapshots _cancelGeneration before the async gap and skips the emit if a mismatch is detected, ensuring the new subscriber receives only its own fresh check result. Also fix the doc example (await listener.cancel()), document the intentionally unawaited _triggerSubscription?.cancel(), and add three tests: initial-yield caching, setIntervalAndResetTimer during connected period, and the stale-emit regression.
Hi @OutdatedGuy! Thanks for maintaining this awesome package.
Closes #99
Problem
The
onStatusChangepolling loop retries at a fixedcheckInterval(default 10 s) regardless of connection state. During extended outages this results in unnecessary battery drain, wasted CPU cycles, and pointless network traffic on mobile devices.Analysis
Inspecting
_maybeEmitStatusUpdate()inlib/src/internet_connection.dartconfirmed the timer is always rescheduled with the same_checkInterval:Key design constraints identified before implementation:
_lastStatusis mutated inside_maybeEmitStatusUpdate, so distinguishing "first failure" from "ongoing failure" requires snapshotting its value before the update.useExponentialBackoff = false) to leave the singletonInternetConnection()and existing callers completely unaffected.setIntervalAndResetTimer(), and on last-listener cancel.backoffInitialDelayis not explicitly provided, it should track_checkIntervalso asetIntervalAndResetTimer()call does not leave a stale initial delay from construction time.Changes
Files modified:
lib/src/internet_connection.darttest/internet_connection_test.dartNew
createInstanceparametersuseExponentialBackoffboolfalsefalsepreserves existing behaviour exactlybackoffInitialDelayDuration?checkIntervalbackoffMaxDelayDurationDuration(seconds: 60)backoffMultiplierdouble2.0Backoff logic
previousStatusis snapshotted before the_lastStatusmutation so that the branch correctly identifies the first failure even whenpreviousStatusisnull(very first poll):Backoff state reset conditions
currentStatus == connected_currentBackoffDelayto_backoffInitialDelaysetIntervalAndResetTimer(duration)backoffInitialDelaywas not explicitly provided, also update_backoffInitialDelaytodurationbefore resetting_currentBackoffDelay; otherwise keep the caller-specified value_handleStatusChangeCancel)_currentBackoffDelayto_backoffInitialDelayso the next subscription starts cleanThe
backoffInitialDelaytracking is implemented with afinal bool _backoffInitialDelayExplicitflag set in the initializer list. Whenfalse,_backoffInitialDelayis updated insetIntervalAndResetTimerto stay in sync with_checkInterval, preventing a stale initial delay from lingering after the interval is changed dynamically.Usage example
Tests
Added
group('exponentialBackoff', ...)totest/internet_connection_test.dart. Tests usecustomConnectivityCheckwithDateTime.now()call logging to avoid real network calls and enable deterministic timing assertions.backoffInitialDelayis used on the first disconnectbackoffMaxDelaycheckIntervalafter reconnectbackoffInitialDelayafter interval changepreviousStatus == nullis treated as first failure, not ongoing