feat: Debounce lifecycle, network, and setMode signals#281
feat: Debounce lifecycle, network, and setMode signals#281tanderson-ld wants to merge 3 commits into
Conversation
Implements CSFDV2 CONNMODE section 3.5 state-change debouncing. Lifecycle, network, and user-requested-mode signals are accumulated over a configurable window (default one second) before driving automatic mode resolution. Per spec, identify calls do not participate. StateDebounceManager (common_client/data_sources/fdv2): - DebouncedState holds networkAvailable, inForeground, requestedMode. - Per-setter early-return on unchanged value. - Duration.zero bypasses the timer (synchronous fire) for tests and FDv1-style immediate behavior. - Injectable DebounceTimerFactory for fake_async-based testing. - close() cancels any pending timer; setters after close are no-ops. Flutter ConnectionManager integration: - ConnectionManagerConfig.debounceWindow (default 1s). - Lifecycle and network listeners feed the debouncer instead of calling _handleState() directly. The debouncer's reconcile callback invokes _handleState() once the window closes. - setMode(ConnectionMode? mode) sets _modeOverride synchronously (CONNMODE 2.0.3 - automatic transitions suppressed immediately) and pushes through the debouncer (CONNMODE 3.5.5 - mode application is debounced). - Foreground -> background transition flushes synchronously per CONNMODE 3.3.1 (the process may be killed before the window closes); the debouncer still handles the resulting mode change. - Network-availability propagation to the destination remains synchronous; only the mode-resolution outcome is debounced. Tests: state_debounce_manager_test covers single-fire, flap-and-return, multi-axis change, requested-mode, close cancellation, immediate mode. connection_manager_test pins existing assertions to Duration.zero (preserving FDv1-style synchronous semantics) and adds three new debounce-window tests using fake_async. Resolves the two TODO sites in connection_manager.dart that were previously tagged SDK-2187.
2efbef6 to
3315a48
Compare
Addresses four findings from the multi-agent review of PR #281: - Wrap onReconcile in try/catch. An exception in the reconcile callback previously left _pending advanced to the new value but the destination uncalled; subsequent setters with the same value would dedupe and the failed reconcile never retried. Now exceptions are caught and (when an LDLogger is provided) logged at error level. The Flutter ConnectionManager passes its logger through. - Document the Duration.zero reentry contract. Class-level docstring now states that with a zero window, onReconcile fires synchronously inside the setter and must not call back into another setter on the same manager instance. - Skip the redundant background-mode flush in _handleState when _onApplicationStateChanged already performed a synchronous flush on the foreground->background transition. _pendingSyncFlush is set in the lifecycle listener and cleared on the next _handleState run. - Add a regression test pinning the setMode-override-wins-over- network-event-mid-debounce-window scenario the PR description calls out: setMode(FDv2Streaming) at t=0, network drops at t=500ms, setNetworkAvailability(false) propagates synchronously to the destination, but the resolved-mode application (after the window) is ResolvedStreaming -- the override suppresses the network-driven switch. - Add a regression test that an exception in onReconcile is swallowed and a subsequent state change still drives a reconcile.
Closes two follow-ups from the multi-agent review of #281: - Initial-state seeding (review finding 4): the SDK no longer assumes foreground at startup when the host platform has already reported background. FlutterStateDetector now exposes initialApplicationState as an instance field, resolved synchronously in the initializer list from SchedulerBinding.instance.lifecycleState. The read is cached at construction time and does not depend on the lifecycle stream. LDClient hoists FlutterStateDetector into a local, reads the seed, and passes it via the new ConnectionManagerConfig.initialApplicationState field. ConnectionManager seeds both _applicationState and the debouncer's inForeground from the config value. Network state stays optimistic at construction time -- Flutter's connectivity_plus API is async-only, and assuming "available" and paying a debounce window if we're wrong gives the best performance in the common case where the network actually is available. - Offline-setter asymmetry (review finding 6): adds a doc comment on ConnectionManager.offline noting the bypass of the debounce window is intentional. A direct "be offline now" should take effect immediately rather than waiting for the window to close. Tests: new connection_manager_test case exercises the seed path by constructing the manager with initialApplicationState: background and verifying _handleState resolves against background state.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 8edb1a6. Configure here.
| }); | ||
| _networkStateSub = detector.networkState.listen(_onNetworkStateChanged); | ||
| } | ||
| } |
There was a problem hiding this comment.
Startup never reconciles mode
Medium Severity
After construction, ConnectionManager only applies resolved mode when the debouncer fires, and StateDebounceManager skips scheduling when lifecycle/network values match its seeded pending state. Initial detector events that confirm foreground plus available network therefore never trigger _handleState, so automatic resolution (e.g. background launch with runInBackground: false) may not run until a later change plus the debounce window.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 8edb1a6. Configure here.
| _onReconcile(_pending); | ||
| } catch (error, stackTrace) { | ||
| _logger?.error( | ||
| 'State debounce reconcile callback threw: $error\n$stackTrace'); |
There was a problem hiding this comment.
Raw exception text in logs
Low Severity
The reconcile error handler logs the caught value with string interpolation ($error), which can embed sensitive request URIs or other PII from exception toString() output instead of a fixed, categorized message.
Triggered by learned rule: Never expose raw exception toString() in logs or StatusEvent messages in data sources
Reviewed by Cursor Bugbot for commit 8edb1a6. Configure here.


Summary
Implements CSFDV2 CONNMODE section 3.5 state-change debouncing for the Flutter SDK. Lifecycle, network, and user-requested-mode signals accumulate over a configurable window (default 1 second) before driving automatic mode resolution. Per spec 3.5.6,
identifycalls do not participate.Stacked on #280 (
-flutter); rebase ontomainonce that merges.StateDebounceManager (
common_client/data_sources/fdv2)DebouncedStateholdsnetworkAvailable,inForeground,requestedMode.Duration.zerobypasses the timer (synchronous fire) for tests and FDv1-style immediate behavior.DebounceTimerFactoryforfake_async-based testing.close()cancels any pending timer; setters after close are no-ops.Flutter
ConnectionManagerintegrationConnectionManagerConfig.debounceWindow(default 1s)._handleState()directly. The debouncer's reconcile callback invokes_handleState()once the window closes.setMode(ConnectionMode? mode)sets_modeOverridesynchronously (CONNMODE 2.0.3 -- automatic transitions suppressed immediately) and pushes through the debouncer (CONNMODE 3.5.5 -- mode application is debounced).Tests
state_debounce_manager_test: single-fire, flap-and-return, multi-axis change, requested-mode, close cancellation, immediate mode.connection_manager_test: existing assertions pinned toDuration.zero(preserve FDv1-style synchronous semantics); three new debounce-window tests usingfake_async.Resolves the two TODO sites in
connection_manager.dartpreviously tagged SDK-2187.Note
Medium Risk
Changes when the SDK reconnects or changes mode during rapid lifecycle/network flaps; critical paths (background flush, offline, sync network availability) are explicitly preserved but timing of mode switches shifts by up to the debounce window.
Overview
Adds
StateDebounceManagerincommon_clientto batch lifecycle, network, and usersetModesignals over a configurable window (default 1s) before a single reconcile callback, withDuration.zerofor immediate/test behavior and exports from the public package API.Flutter
ConnectionManagernow routes lifecycle and network updates through the debouncer (replacing direct_handleStateon every signal), addsdebounceWindowandinitialApplicationStateon config, seeds lifecycle fromFlutterStateDetector.initialApplicationStateat client startup, and keeps foreground→background flush and destination network availability synchronous while debouncing resolved mode application.setModesets the override immediately but applies the resolved mode after debounce;offlinestill bypasses debouncing.Tests add
state_debounce_manager_test, pin existingconnection_manager_testexpectations toDuration.zero, and addfake_asyncdebounce integration cases.Reviewed by Cursor Bugbot for commit 8edb1a6. Bugbot is set up for automated code reviews on this repo. Configure here.