Skip to content

fix: Keep polling after an unexpected HTTP error response #350

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.

Sign up for GitHub

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

Draft
jsonbailey wants to merge 1 commit into from
Draft

fix: Keep polling after an unexpected HTTP error response #350

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion apps/flutter_client_contract_test_service/bin/contract_test_service.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import 'package:flutter/widgets.dart' as widgets;
import 'package:connectivity_plus_platform_interface/connectivity_plus_platform_interface.dart';
import 'package:launchdarkly_dart_common/launchdarkly_dart_common.dart'
as common;
import 'package:launchdarkly_common_client/src/config/data_source_config.dart'
show PollingDataSourceConfig;
import 'package:launchdarkly_flutter_client_sdk/launchdarkly_flutter_client_sdk.dart';
import 'package:openapi_base/openapi_base.dart';
import 'package:shared_preferences/shared_preferences.dart';
Expand All @@ -32,6 +34,7 @@ class TestApiImpl extends SdkTestApi {
'client-event-source-http-errors',
'fdv1-fallback',
'client-use-post',
'retry-conformance-fdv1-polling',
];

static const clientUrlPrefix = '/client/';
Expand All @@ -57,6 +60,7 @@ class TestApiImpl extends SdkTestApi {
final mappedDataSystem = _mapDataSystem(
body.configuration?.dataSystem?.toJson(),
usePost: body.configuration?.clientSide?.usePost ?? false);
final pollIntervalMs = body.configuration?.polling?.pollIntervalMs;
final config = LDConfig(
body.configuration?.credential ?? '',
AutoEnvAttributes.disabled,
Expand All @@ -76,7 +80,12 @@ class TestApiImpl extends SdkTestApi {
? ConnectionMode.streaming
: ConnectionMode.polling),
evaluationReasons: body.configuration?.clientSide?.evaluationReasons,
useReport: body.configuration?.clientSide?.useReport),
useReport: body.configuration?.clientSide?.useReport,
polling: pollIntervalMs != null
? PollingConfig(
pollingInterval:
Duration(milliseconds: pollIntervalMs.toInt()))
: null),
events: EventsConfig(
eventCapacity: body.configuration?.events?.capacity?.toInt(),
disabled: body.configuration?.events == null,
Expand Down Expand Up @@ -616,6 +625,9 @@ final class _WifiConnected extends ConnectivityPlatform {
void main() async {
test(timeout: Timeout.none, 'Run contract tests', () async {
ConnectivityPlatform.instance = _WifiConnected();
// Tests set short polling intervals and expect the SDK to use them.
// ignore: invalid_use_of_visible_for_testing_member
PollingDataSourceConfig.ignorePollingMinimum = true;
widgets.WidgetsFlutterBinding.ensureInitialized(); // needed before mocking
// ignore: invalid_use_of_visible_for_testing_member
SharedPreferences.setMockInitialValues({}); // required to mock persistence
Expand Down
26 changes: 22 additions & 4 deletions packages/common_client/lib/src/config/data_source_config.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import 'package:launchdarkly_dart_common/launchdarkly_dart_common.dart';
import 'package:meta/meta.dart';

import 'defaults/default_config.dart';

Expand Down Expand Up @@ -28,17 +29,34 @@ final class PollingDataSourceConfig extends DataSourceConfigBase {
/// The current polling interval, if less than min, then the min will be used.
final Duration pollingInterval;

/// When true, the polling interval has no minimum. Only the contract test
/// service sets this, so that tests can poll every few seconds. It has an
/// effect only when asserts are enabled, as in debug builds and tests. A
/// release build always applies the minimum.
@visibleForTesting
static bool ignorePollingMinimum = false;

@jsonbailey jsonbailey Oct 16, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Test-only hook, used only when asserts are on. iOS has the same idea (ignorePollingMinimum, internal, through @testable), and Android has ComponentsImpl...pollIntervalMillisNoMinimum. Neither test service applies pollIntervalMs yet, so sdk-test-harness #501 must not merge until they do.

@jsonbailey jsonbailey Oct 16, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

We may not want to include this in the final iteration, but we can just keep this locally for quick testing, but long term to the long running tests approach.


PollingDataSourceConfig(
{Duration? pollingInterval, bool? withReasons, bool? useReport})
: pollingInterval = durationWithMin(
DefaultConfig.pollingConfig.defaultPollingInterval,
pollingInterval,
DefaultConfig.pollingConfig.minPollingInterval),
: pollingInterval = _resolvePollingInterval(pollingInterval),
super(
withReasons: withReasons ??
DefaultConfig.dataSourceConfig.defaultWithReasons,
useReport:
useReport ?? DefaultConfig.dataSourceConfig.defaultUseReport);

static Duration _resolvePollingInterval(Duration? requested) {
var ignoreMinimum = false;
assert(() {
ignoreMinimum = ignorePollingMinimum;
return true;
}());
return ignoreMinimum
? durationGreaterThanZeroWithDefault(
requested, DefaultConfig.pollingConfig.defaultPollingInterval)
: durationWithMin(DefaultConfig.pollingConfig.defaultPollingInterval,
requested, DefaultConfig.pollingConfig.minPollingInterval);
}
}

final class StreamingDataSourceConfig extends DataSourceConfigBase {
Expand Down
7 changes: 4 additions & 3 deletions packages/common_client/lib/src/data_sources/data_source.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,10 @@ final class DataEvent implements DataSourceEvent {
DataEvent(this.type, this.data, {this.environmentId});
}

/// An FDv2 change set produced by the data source orchestrator. Carries
/// typed flag descriptors translated at acquisition time, not the FDv1
/// JSON string forms.
/// A change set with typed flag descriptors, not the FDv1 JSON string forms.
/// The FDv2 orchestrator produces these for its payloads. The FDv1 data
/// sources produce one of type none when a poll confirms that the data is
/// current.
final class PayloadEvent implements DataSourceEvent {
final ChangeSet changeSet;
final String? environmentId;
Expand Down
20 changes: 10 additions & 10 deletions packages/common_client/lib/src/data_sources/data_source_status.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ enum DataSourceState {
/// initialized.
///
/// If it encounters an error that requires it to retry initialization,
/// the state will remain at kInitializing until it either succeeds and
/// becomes [valid], or permanently fails and becomes [shutdown].
/// the state will remain at [initializing] until it succeeds and becomes
/// [valid].
initializing,

/// Indicates that the data source is currently operational and has not
Expand All @@ -22,19 +22,19 @@ enum DataSourceState {
/// In streaming mode, this means that the stream connection failed, or
/// had to be dropped due to some other error, and will be retried after
/// a backoff delay. In polling mode, it means that the last poll request
/// failed, and a new poll request will be made after the configured
/// polling interval.
/// failed, and a new poll request will be made after a backoff delay that
/// is never shorter than the configured polling interval.
interrupted,

/// Indicates that the application has told the SDK to stay offline.
setOffline,

/// Indicates that the data source has been permanently shut down.
/// Indicates that the data source has been shut down.
///
/// This could be because it encountered an unrecoverable error (for
/// instance, the LaunchDarkly service rejected the SDK key; an invalid
/// SDK key will never become valid), or because the SDK client was
/// explicitly shut down.
/// This could be because the SDK client was explicitly shut down, or
/// because the data source configuration cannot work. An HTTP error or a
/// network error does not cause this state, because the data source keeps
/// retrying after those errors.
shutdown,

/// Indicates that the SDK is aware of a lack of network connectivity.
Expand Down Expand Up @@ -148,7 +148,7 @@ final class DataSourceStatus {
/// [DataSourceState.valid].
///
/// - For [DataSourceState.shutdown], it is the time that the data source
/// encountered an unrecoverable error or that the SDK was explicitly shut down.
/// was shut down.
///
/// - For [DataSourceState.networkUnavailable] or
/// [DataSourceState.backgroundDisabled], it is the time that the SDK switched
Expand Down
3 changes: 2 additions & 1 deletion packages/common_client/lib/src/data_sources/fdv2/fdv1_fallback_synchronizer.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,9 @@ FDv2SourceResult _translate(DataSourceEvent? event) {
);
}
case StatusEvent():
final statusCode = event.statusCode;
return StatusResult(
state: event.shutdown
state: statusCode != null && !isHttpGloballyRecoverable(statusCode)
? SourceState.terminalError
: SourceState.interrupted,
message: event.message,
Expand Down
113 changes: 67 additions & 46 deletions packages/common_client/lib/src/data_sources/polling_data_source.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
import 'dart:async';
import 'dart:convert';
import 'package:launchdarkly_dart_common/launchdarkly_dart_common.dart';
import 'dart:math';

import '../config/data_source_config.dart';
import 'data_source.dart';
import 'data_source_status.dart';
import 'fdv2/payload.dart';
import 'requestor.dart';

HttpClient _defaultHttpClientFactory(HttpProperties httpProperties) {
Expand All @@ -16,13 +15,17 @@ HttpClient _defaultHttpClientFactory(HttpProperties httpProperties) {
// because the etag may not correspond to the current payload the SDK has,
// because streaming could have been used in the interim.

/// A data source that polls for flag updates.
///
/// Each poll is scheduled when the previous poll completes. After a
/// successful poll, the next poll comes one polling interval later. After a
/// failed poll, the wait comes from the [RetryState]. A payload that the SDK
/// cannot apply is a failed poll. No failure stops the data source.
final class PollingDataSource implements DataSource {
final LDLogger _logger;

Timer? _pollTimer;

final Stopwatch _pollStopwatch = Stopwatch();

late final Duration _pollingInterval;

late final String _contextString;
Expand All @@ -35,12 +38,15 @@ final class PollingDataSource implements DataSource {

late final Requestor _requestor;

late final RetryState _retryState;

/// Set when the last poll returned a payload that the SDK has not yet
/// rejected. The SDK rejects a payload through [restart].
bool _payloadPending = false;

@override
Stream<DataSourceEvent> get events => _eventController.stream;

/// Used to track if there has been an unrecoverable error.
bool _permanentShutdown = false;

/// The [httpClientFactory] parameter is primarily intended for testing, but it also
/// could be used for customized clients which support functionality
/// our default client support does not. For instance domain sockets or
Expand All @@ -60,6 +66,7 @@ final class PollingDataSource implements DataSource {
: _logger = logger.subLogger('PollingDataSource'),
_credential = credential {
_pollingInterval = testingInterval ?? dataSourceConfig.pollingInterval;
_retryState = RetryState.polling(_pollingInterval);

final method =
dataSourceConfig.useReport ? RequestMethod.report : RequestMethod.get;
Expand All @@ -84,9 +91,12 @@ final class PollingDataSource implements DataSource {
}

Future<void> _doPoll() async {
_pollStopwatch.reset();
_pollStopwatch.start();

if (_payloadPending) {
// The previous payload was applied, because it was not rejected
// before this poll.
_payloadPending = false;
_retryState.recordSuccess();
}
final event = await _requestor.requestAllFlags();

if (_stopped) {
Expand All @@ -95,69 +105,76 @@ final class PollingDataSource implements DataSource {

switch (event) {
case null:
// No change (e.g. 304 Not Modified). Fall through to schedule
// the next poll -- returning here would leave the timer
// unset and silently stop polling after the first unchanged
// response.
break;
// No change (e.g. 304 Not Modified). This is a successful poll, and
// a change set of type none restores the valid status.
_retryState.recordSuccess();
_eventController.sink.add(_unchanged());
_schedulePoll(_pollingInterval);
case DataEvent():
_payloadPending = true;
_eventController.sink.add(event);
_schedulePoll(_pollingInterval);
case StatusEvent():
_eventController.sink.add(event);
final suffix = event.shutdown ? 'stopping polling' : 'will retry';
final message = event.kind == ErrorKind.errorResponse
? 'received unexpected status code when polling'
: 'encountered error with polling request';
final argument = event.kind == ErrorKind.errorResponse
? event.statusCode
: event.message;
_logger.error('$message: $argument, $suffix');
if (event.shutdown) {
_permanentShutdown = true;
stop();
}
_handleFailure(event);
case PayloadEvent():
case InitializedEvent():
// The FDv1 requestor never produces FDv2 payload or lifecycle events.
break;
}
}

_schedulePoll();
void _handleFailure(StatusEvent event) {
if (event.statusCode case final statusCode?) {
final kind = classifyHttpStatus(statusCode);
final delay = _retryState.recordFailure(kind);
switch (kind) {
case FailureKind.unexpected:
_logger.error('Received unexpected status code $statusCode when '
'polling.${credentialHint(statusCode)} Will retry in '
'${delay.inMilliseconds} ms.');
case FailureKind.normal:
_logger.warn('Received status code $statusCode when polling. Will '
'retry in ${delay.inMilliseconds} ms.');
}
_schedulePoll(delay);
return;
}
final delay = _retryState.recordFailure(FailureKind.normal);
_logger.warn('Encountered error with polling request: ${event.message}. '
'Will retry in ${delay.inMilliseconds} ms.');
_schedulePoll(delay);
}

void _schedulePoll() {
void _schedulePoll(Duration delay) {
if (_stopped) {
return;
}
_pollStopwatch.stop();
final timeSincePoll = _pollStopwatch.elapsed;
// Calculate a delay based on the polling interval and the duration elapsed
// since the last poll.

// Example: If the poll took 5 seconds, and the interval is 30 seconds, then
// we want to poll after 25 seconds.
final delay = Duration(
milliseconds: min(
_pollingInterval.inMilliseconds - timeSincePoll.inMilliseconds,
_pollingInterval.inMilliseconds));

_pollTimer = Timer(delay, _doPoll);
}

@override
void start() {
if (_permanentShutdown || _stopped) {
if (_stopped) {
return;
}

_stopped = false;
_doPoll();
}

/// Records that the SDK could not apply the last payload. The data source
/// fetches a full payload again after the retry delay.
@override
void restart() {
// For polling there is no persistent connection, so this function
// has no effect.
if (_stopped || !_payloadPending) {
return;
}
_payloadPending = false;
_requestor.clearEtag();
_pollTimer?.cancel();
final delay = _retryState.recordFailure(FailureKind.normal);
_logger.warn('Polling response contained invalid data. Will retry in '
'${delay.inMilliseconds} ms.');
_schedulePoll(delay);
}

@override
Expand All @@ -168,3 +185,7 @@ final class PollingDataSource implements DataSource {
_eventController.close();
}
}

/// A change set that confirms that the SDK data is current.
PayloadEvent _unchanged() =>
PayloadEvent(const ChangeSet(type: PayloadType.none, updates: {}));
15 changes: 8 additions & 7 deletions packages/common_client/lib/src/data_sources/requestor.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,12 @@ final class Requestor {
_uri = Uri.parse(completeUrl);
}

/// Forgets the stored ETag, so that the next request fetches a full
/// payload. Call this when the last payload could not be used.
void clearEtag() {
_lastEtag = null;
}

Future<DataSourceEvent?> requestAllFlags() async {
try {
_logger.debug(
Expand All @@ -73,7 +79,7 @@ final class Requestor {
// and the URL embeds the base64url-encoded context in GET mode.
// Categorize into a fixed message and use that everywhere.
final sanitized = _describeError(err);
_logger.warn('Polling request failed: $sanitized');
_logger.debug('Polling request failed: $sanitized');
return StatusEvent(ErrorKind.networkError, null, sanitized);
}
}
Expand All @@ -100,13 +106,8 @@ final class Requestor {

return DataEvent('put', res.body, environmentId: environmentId);
}
if (isHttpGloballyRecoverable(res.statusCode)) {
return StatusEvent(ErrorKind.errorResponse, res.statusCode,
'Received unexpected status code: ${res.statusCode}');
}
return StatusEvent(ErrorKind.errorResponse, res.statusCode,
'Received unexpected status code: ${res.statusCode}',
shutdown: true);
'Received unexpected status code: ${res.statusCode}');
}

String _describeError(Object err) {
Expand Down
Loading
Loading