Skip to content

chore: adds connection mode resolution and source factories#265

Open
tanderson-ld wants to merge 8 commits intomainfrom
ta/SDK-2187/connection-mode-and-resolution
Open

chore: adds connection mode resolution and source factories#265
tanderson-ld wants to merge 8 commits intomainfrom
ta/SDK-2187/connection-mode-and-resolution

Conversation

@tanderson-ld
Copy link
Copy Markdown
Contributor

@tanderson-ld tanderson-ld commented May 6, 2026

Requirements

  • I have added test coverage for new or changed functionality
  • I have followed the repository's pull request submission guidelines
  • I have validated my changes against all supported platform versions

Note

Medium Risk
Changes connection-mode selection and event-sending behavior across lifecycle/network transitions, which can affect update delivery and analytics flushing in production. Adds new FDv2 mode-definition/source-factory plumbing that is partially implemented (streaming entries throw UnsupportedError).

Overview
Introduces a new ConnectionMode.background and updates the common client to support it end-to-end (factories, DataSourceManager switching, offline fallbacks, and public exports).

Refactors Flutter ConnectionManager to use a table-driven automatic mode resolver (resolveConnectionMode/flutterDefaultResolutionTable) with separate foreground/background mode slots, plus an explicit override (setMode(ConnectionMode?)) that can be cleared to resume automatic behavior; also flushes on backgrounding when network is available and disables event sending when offline or backgrounded without runInBackground.

Adds FDv2 mode modeling and factory-building utilities (ModeDefinition, built-in modes, per-entry endpoint overrides, SourceFactoryContext, initializer/synchronizer factories) with tests; streaming FDv2 entries are defined but not yet implemented and currently throw.

Reviewed by Cursor Bugbot for commit 2275df1. Bugbot is set up for automated code reviews on this repo. Configure here.

kinyoklion and others added 6 commits April 29, 2026 14:50
Implements the four building blocks Phase 3 needs from Stream A:

- calculate_poll_delay.dart: pure helper. Given freshness and
  interval, returns the time remaining in the interval; zero when
  overdue; full interval when there's no prior freshness or the
  freshness is in the future (clock skew clamp).
- polling_initializer.dart: one-shot Initializer. Calls the injected
  PollFunction up to 3 times. ChangeSetResult and terminal status
  results return immediately. Interrupted results retry after a 1s
  delay. After 3 interrupted attempts the last is escalated to
  terminalError so the orchestrator stops retrying at this layer.
  close() interrupts a pending retry delay and yields shutdown.
- polling_synchronizer.dart: long-lived Synchronizer. Single-
  subscription StreamController that polls immediately on subscribe,
  then schedules subsequent polls via calculatePollDelay over the
  freshness of the most recent successful result. Interrupted
  results pass through but do not advance freshness, so a transient
  failure does not delay the catch-up poll. Injected TimerFactory
  and now() for deterministic tests.
- cache_initializer.dart: Initializer that reads the persistence
  cache via an injected CachedFlagsReader. Cache hit emits a full
  ChangeSetResult with persist=false and an empty selector (the
  cache does not track server-side selector state). Cache miss or
  reader exception emits a none-type ChangeSetResult so the
  initializer chain advances. The reader typedef leaves the actual
  persistence wiring to the orchestrator (Phase C1).

PollFunction, DelayFunction, TimerFactory, and CachedFlagsReader are
typedefs rather than concrete dependencies so the orchestrator can
wire real implementations and tests can inject scripted ones,
mirroring the abstraction style established in SDK-2183.
- polling_synchronizer: replace `bool _closed` with
  `Completer<void> _stoppedSignal` to match the polling initializer
  and have a single source of truth for closed-ness. Drop the
  redundant `_controller.isClosed` checks (the signal is set before
  the controller is closed, so the signal check covers both cases).
  Replace `Future<void>.microtask(_doPoll)` in onListen with
  `unawaited(_doPoll())` -- the microtask wrapping was unnecessary;
  fire-and-forget of an async function is what unawaited expresses.
- polling_synchronizer: fix typo in the unexpected-throw message:
  "raised unexpectedly" -> "raised error unexpectedly".
- calculate_poll_delay: split the run-on docstring describing the
  null-freshness and overdue-freshness cases into two separate
  sentences for readability.
- cache_initializer: reword the awkward parenthetical
  "(the data came from the cache; writing it back is a no-op)" to
  "(the data is already cached)".
…king branch 'origin' into ta/SDK-2187/connection-mode-and-resolution
@tanderson-ld tanderson-ld requested a review from a team as a code owner May 6, 2026 20:09
this.disableAutomaticNetworkHandling = false});
ConnectionManagerConfig({
this.foregroundConnectionMode = ConnectionMode.streaming,
this.backgroundConnectionMode = ConnectionMode.offline,
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to double check if this is a breaking change.

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Fix All in Cursor

❌ 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 b6bd1de. Configure here.

if (_offline) {
resolved = ConnectionMode.offline;
} else if (_modeOverride != null) {
resolved = _modeOverride!;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bang operator used instead of Dart 3 case pattern

Low Severity

The _modeOverride nullable field is checked with != null and then accessed with the ! bang operator. The codebase's preferred idiom (and the one used in other files like requestor.dart and flag_eval_mapper.dart) is Dart 3 case-pattern narrowing, e.g. else if (_modeOverride case final override?), which avoids the bang operator and leverages compiler flow analysis.

Fix in Cursor Fix in Web

Triggered by learned rule: Prefer Dart 3 case-pattern null narrowing over bang operator

Reviewed by Cursor Bugbot for commit b6bd1de. Configure here.

dataSourceConfig: pollingDataSourceConfig,
httpProperties: httpProperties);
},
ConnectionMode.background: streaming,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Background connection mode incorrectly mapped to streaming factory

Medium Severity

ConnectionMode.background is mapped to the streaming data source factory, which creates a persistent StreamingDataSource. This contradicts the design intent of background mode, which per BuiltInModes.background and its fdv1Fallback config specifies reduced-rate polling (3600s interval). If a user configures backgroundConnectionMode: ConnectionMode.background, the SDK would open a full streaming connection in the background instead of using a resource-efficient polling approach.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit b6bd1de. Configure here.

@tanderson-ld
Copy link
Copy Markdown
Contributor Author

Need to bench test to confirm no regressions in connection manager wrt event flushing.

Base automatically changed from rlamb/sdk-2184/fdv2-polling-cache-sources to main May 6, 2026 22:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants