From d540dd63af4fdc1df561925268fd82a20c06cb8f Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Mon, 27 Apr 2026 15:57:44 -0700 Subject: [PATCH 01/10] feat: add Initializer and Synchronizer source contracts Adds source.dart with the Initializer/Synchronizer interfaces and the SelectorGetter/PingHandler typedefs. These are the contracts for FDv2 data sources: Initializer for one-shot results, Synchronizer for streaming results, SelectorGetter for lazy selector reads, and PingHandler for legacy ping-driven polls. Concrete implementations follow in subsequent commits. --- .../lib/src/data_sources/fdv2/source.dart | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 packages/common_client/lib/src/data_sources/fdv2/source.dart diff --git a/packages/common_client/lib/src/data_sources/fdv2/source.dart b/packages/common_client/lib/src/data_sources/fdv2/source.dart new file mode 100644 index 0000000..bac1c3c --- /dev/null +++ b/packages/common_client/lib/src/data_sources/fdv2/source.dart @@ -0,0 +1,44 @@ +import 'selector.dart'; +import 'source_result.dart'; + +/// A function that returns the current selector for a data source. +/// +/// The orchestrator owns the SDK's current selector. Sources read it +/// lazily on each request or reconnect via this getter, so they always +/// see the latest value across mode switches and recoveries. +typedef SelectorGetter = Selector Function(); + +/// A function that performs a single FDv2 poll and returns the result. +/// +/// Used by streaming sources to handle legacy `ping` events: when a ping +/// is received, the streaming source invokes the ping handler to fetch +/// the current payload via polling. +typedef PingHandler = Future Function(); + +/// A one-shot data source that produces a single result. +/// +/// Used during initialization to bring the SDK into a usable state from +/// cache, polling, or a streaming connection's first payload. +abstract interface class Initializer { + /// Runs the initializer, producing a single result. If [close] is called + /// before a result is produced, the returned future completes with a + /// shutdown [StatusResult]. + Future run(); + + /// Cancels in-progress work. Idempotent. + void close(); +} + +/// A long-lived data source that produces a stream of results. +/// +/// Used during steady-state operation to keep the SDK current via polling +/// or streaming. +abstract interface class Synchronizer { + /// Single-subscription stream of results. Cancelling the subscription + /// stops the synchronizer; starting a new subscription is not supported. + Stream get results; + + /// Cancels active work. Idempotent. A shutdown [StatusResult] is + /// emitted to any active subscriber before the stream closes. + void close(); +} From 1d18961527c466419fa04b7ad3160e9c4f7a88e1 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Mon, 27 Apr 2026 16:09:29 -0700 Subject: [PATCH 02/10] feat: add FDv2 requestor and polling base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the HTTP layer for FDv2 polling: - endpoints.dart: FDv2 polling and streaming path constants, uniform across mobile and browser. - requestor.dart: FDv2Requestor — pure HTTP. Builds GET URLs with the encoded context in the path or POST URLs with the context JSON in the body. Adds basis (when the selector is non-empty) and withReasons query params. Tracks ETag across requests via if-none-match. Returns a RequestorResponse record. - polling_base.dart: FDv2PollingBase — wraps the requestor with FDv2 protocol semantics. Network errors produce interrupted; the x-ld-fd-fallback header produces terminalError with fdv1Fallback set; 304 produces a none-type ChangeSetResult; recoverable 4xx/5xx produce interrupted; non-recoverable 4xx produce terminalError; 200 bodies are parsed as FDv2EventsCollection and run through a fresh FDv2ProtocolHandler. No data source integration yet — that follows in SDK-2184. --- .../lib/src/data_sources/fdv2/endpoints.dart | 23 ++ .../src/data_sources/fdv2/polling_base.dart | 153 +++++++ .../lib/src/data_sources/fdv2/requestor.dart | 107 +++++ .../data_sources/fdv2/endpoints_test.dart | 28 ++ .../data_sources/fdv2/polling_base_test.dart | 390 ++++++++++++++++++ .../data_sources/fdv2/requestor_test.dart | 276 +++++++++++++ 6 files changed, 977 insertions(+) create mode 100644 packages/common_client/lib/src/data_sources/fdv2/endpoints.dart create mode 100644 packages/common_client/lib/src/data_sources/fdv2/polling_base.dart create mode 100644 packages/common_client/lib/src/data_sources/fdv2/requestor.dart create mode 100644 packages/common_client/test/data_sources/fdv2/endpoints_test.dart create mode 100644 packages/common_client/test/data_sources/fdv2/polling_base_test.dart create mode 100644 packages/common_client/test/data_sources/fdv2/requestor_test.dart diff --git a/packages/common_client/lib/src/data_sources/fdv2/endpoints.dart b/packages/common_client/lib/src/data_sources/fdv2/endpoints.dart new file mode 100644 index 0000000..325d023 --- /dev/null +++ b/packages/common_client/lib/src/data_sources/fdv2/endpoints.dart @@ -0,0 +1,23 @@ +/// FDv2 endpoint paths. +/// +/// These paths are uniform across mobile and browser SDKs; FDv2 does +/// not distinguish between platforms at the endpoint level. +abstract final class FDv2Endpoints { + /// Polling path used for POST requests. The evaluation context is + /// sent in the request body. + static const String polling = '/sdk/poll/eval'; + + /// Streaming path used for POST requests. The evaluation context is + /// sent in the request body. + static const String streaming = '/sdk/stream/eval'; + + /// Builds the polling GET path with the base64url-encoded context + /// embedded in the URL path. + static String pollingGet(String encodedContext) => + '$polling/$encodedContext'; + + /// Builds the streaming GET path with the base64url-encoded context + /// embedded in the URL path. + static String streamingGet(String encodedContext) => + '$streaming/$encodedContext'; +} diff --git a/packages/common_client/lib/src/data_sources/fdv2/polling_base.dart b/packages/common_client/lib/src/data_sources/fdv2/polling_base.dart new file mode 100644 index 0000000..ee9e0f0 --- /dev/null +++ b/packages/common_client/lib/src/data_sources/fdv2/polling_base.dart @@ -0,0 +1,153 @@ +import 'dart:convert'; + +import 'package:launchdarkly_dart_common/launchdarkly_dart_common.dart'; + +import 'flag_eval_mapper.dart'; +import 'payload.dart'; +import 'protocol_handler.dart'; +import 'protocol_types.dart'; +import 'requestor.dart'; +import 'selector.dart'; +import 'source_result.dart'; + +/// Performs a single FDv2 poll and translates the response into an +/// [FDv2SourceResult]. +/// +/// Wraps an [FDv2Requestor] with FDv2 protocol semantics: +/// +/// - Network errors → [SourceState.interrupted]. +/// - `x-ld-fd-fallback: true` header → terminal error with +/// `fdv1Fallback: true`. +/// - HTTP `304 Not Modified` → an empty change set with +/// [PayloadType.none], confirming the cached data is current. +/// - Other 4xx/5xx → interrupted (recoverable) or terminalError +/// (non-recoverable) based on [isHttpGloballyRecoverable]. +/// - `200` → body is parsed as an [FDv2EventsCollection] and fed +/// through an [FDv2ProtocolHandler]. The first emitted action +/// determines the result. +final class FDv2PollingBase { + final LDLogger _logger; + final FDv2Requestor _requestor; + final DateTime Function() _now; + + FDv2PollingBase({ + required LDLogger logger, + required FDv2Requestor requestor, + DateTime Function()? now, + }) : _logger = logger.subLogger('FDv2PollingBase'), + _requestor = requestor, + _now = now ?? DateTime.now; + + /// Performs a single poll. Never throws; all errors are reported as + /// [StatusResult]s. + Future pollOnce( + {Selector basis = Selector.empty}) async { + final RequestorResponse response; + try { + response = await _requestor.request(basis: basis); + } catch (err) { + _logger.warn('Polling request failed: $err'); + return FDv2SourceResults.interrupted(message: err.toString()); + } + return _processResponse(response); + } + + FDv2SourceResult _processResponse(RequestorResponse response) { + final fdv1Fallback = response.headers['x-ld-fd-fallback'] == 'true'; + final environmentId = response.headers['x-ld-envid']; + + if (fdv1Fallback) { + return FDv2SourceResults.terminalError( + statusCode: response.status, + message: 'Server requested FDv1 fallback', + fdv1Fallback: true, + ); + } + + // 304 Not Modified means the SDK's cached data is confirmed current. + if (response.status == 304) { + return ChangeSetResult( + payload: const Payload(type: PayloadType.none, updates: []), + environmentId: environmentId, + freshness: _now(), + persist: true, + ); + } + + if (response.status >= 400) { + final message = 'Received unexpected status code: ${response.status}'; + if (isHttpGloballyRecoverable(response.status)) { + return FDv2SourceResults.interrupted( + statusCode: response.status, + message: message, + ); + } + return FDv2SourceResults.terminalError( + statusCode: response.status, + message: message, + ); + } + + return _parseBody(response, environmentId: environmentId); + } + + FDv2SourceResult _parseBody( + RequestorResponse response, { + String? environmentId, + }) { + final Map json; + try { + final decoded = jsonDecode(response.body); + if (decoded is! Map) { + return FDv2SourceResults.interrupted( + statusCode: response.status, + message: 'Polling response was not a JSON object', + ); + } + json = decoded; + } catch (err) { + _logger.error('Failed to parse polling response body as JSON: $err'); + return FDv2SourceResults.interrupted( + statusCode: response.status, + message: 'Polling response body was not valid JSON', + ); + } + + final collection = FDv2EventsCollection.fromJson(json); + final handler = FDv2ProtocolHandler( + objProcessors: {flagEvalKind: processFlagEval}, + logger: _logger, + ); + + for (final event in collection.events) { + final action = handler.processEvent(event); + switch (action) { + case ActionPayload(:final payload): + return ChangeSetResult( + payload: payload, + environmentId: environmentId, + freshness: _now(), + persist: true, + ); + case ActionGoodbye(:final reason): + return FDv2SourceResults.goodbyeResult(message: reason); + case ActionServerError(:final reason): + return FDv2SourceResults.interrupted(message: reason); + case ActionError(:final message): + return FDv2SourceResults.interrupted(message: message); + case ActionNone(): + // Continue accumulating events until a payload-transferred or + // terminal action is reached. + break; + } + } + + // The response had no payload-transferred event. The protocol handler + // is left in a partial state with nothing to emit, which is a + // protocol violation for a polling response. + return FDv2SourceResults.interrupted( + statusCode: response.status, + message: 'Polling response did not include a complete payload', + ); + } +} diff --git a/packages/common_client/lib/src/data_sources/fdv2/requestor.dart b/packages/common_client/lib/src/data_sources/fdv2/requestor.dart new file mode 100644 index 0000000..518f8ed --- /dev/null +++ b/packages/common_client/lib/src/data_sources/fdv2/requestor.dart @@ -0,0 +1,107 @@ +import 'package:launchdarkly_dart_common/launchdarkly_dart_common.dart'; + +import 'endpoints.dart'; +import 'selector.dart'; + +typedef HttpClientFactory = HttpClient Function(HttpProperties httpProperties); + +HttpClient _defaultHttpClientFactory(HttpProperties httpProperties) { + return HttpClient(httpProperties: httpProperties); +} + +/// The shape of a completed HTTP response from the FDv2 polling endpoint. +typedef RequestorResponse = ({ + int status, + Map headers, + String body, +}); + +/// Issues a single HTTP poll against the FDv2 polling endpoint. +/// +/// Pure HTTP layer: builds the URL, sends the request, tracks `ETag` +/// across calls on the same instance, and returns the raw response. It +/// does no FDv2 protocol parsing or error classification — that is the +/// responsibility of the caller (see [FDv2PollingBase]). +/// +/// One [FDv2Requestor] is bound to a single evaluation context. Switching +/// contexts requires a fresh instance so a previous context's `ETag` +/// can never leak into a request for a different context. +final class FDv2Requestor { + final LDLogger _logger; + final HttpClient _client; + final String _baseUrl; + final String _contextEncoded; + final String _contextJson; + final bool _usePost; + final bool _withReasons; + String? _lastEtag; + + FDv2Requestor({ + required LDLogger logger, + required ServiceEndpoints endpoints, + required String contextEncoded, + required String contextJson, + required bool usePost, + required bool withReasons, + required HttpProperties httpProperties, + HttpClientFactory httpClientFactory = _defaultHttpClientFactory, + }) : _logger = logger.subLogger('FDv2Requestor'), + _baseUrl = endpoints.polling, + _contextEncoded = contextEncoded, + _contextJson = contextJson, + _usePost = usePost, + _withReasons = withReasons, + _client = httpClientFactory(usePost + ? httpProperties.withHeaders({'content-type': 'application/json'}) + : httpProperties); + + /// Sends a single poll request, optionally including a [basis] selector + /// for delta updates. Throws on network errors; otherwise returns the + /// raw response. Tracks `ETag` for subsequent calls on this instance. + Future request( + {Selector basis = Selector.empty}) async { + final uri = _buildUri(basis: basis); + final method = _usePost ? RequestMethod.post : RequestMethod.get; + final additionalHeaders = {}; + if (_lastEtag != null) { + additionalHeaders['if-none-match'] = _lastEtag!; + } + + _logger.debug( + 'FDv2 poll: method=$method, uri=$uri, etag=$_lastEtag, basis=${basis.state}'); + + final response = await _client.request( + method, + uri, + additionalHeaders: additionalHeaders.isEmpty ? null : additionalHeaders, + body: _usePost ? _contextJson : null, + ); + + final etag = response.headers['etag']; + if (etag != null) { + _lastEtag = etag; + } + + return ( + status: response.statusCode, + headers: response.headers, + body: response.body, + ); + } + + Uri _buildUri({required Selector basis}) { + final path = _usePost + ? FDv2Endpoints.polling + : FDv2Endpoints.pollingGet(_contextEncoded); + final queryParams = {}; + if (_withReasons) { + queryParams['withReasons'] = 'true'; + } + if (basis.isNotEmpty) { + queryParams['basis'] = basis.state!; + } + final url = appendPath(_baseUrl, path); + final uri = Uri.parse(url); + return queryParams.isEmpty ? uri : uri.replace(queryParameters: queryParams); + } +} diff --git a/packages/common_client/test/data_sources/fdv2/endpoints_test.dart b/packages/common_client/test/data_sources/fdv2/endpoints_test.dart new file mode 100644 index 0000000..69101ee --- /dev/null +++ b/packages/common_client/test/data_sources/fdv2/endpoints_test.dart @@ -0,0 +1,28 @@ +import 'package:launchdarkly_common_client/src/data_sources/fdv2/endpoints.dart'; +import 'package:test/test.dart'; + +void main() { + group('FDv2Endpoints', () { + test('polling path is the FDv2 polling endpoint', () { + expect(FDv2Endpoints.polling, equals('/sdk/poll/eval')); + }); + + test('streaming path is the FDv2 streaming endpoint', () { + expect(FDv2Endpoints.streaming, equals('/sdk/stream/eval')); + }); + + test('pollingGet appends the encoded context', () { + expect( + FDv2Endpoints.pollingGet('eyJrZXkiOiJ0ZXN0In0='), + equals('/sdk/poll/eval/eyJrZXkiOiJ0ZXN0In0='), + ); + }); + + test('streamingGet appends the encoded context', () { + expect( + FDv2Endpoints.streamingGet('eyJrZXkiOiJ0ZXN0In0='), + equals('/sdk/stream/eval/eyJrZXkiOiJ0ZXN0In0='), + ); + }); + }); +} diff --git a/packages/common_client/test/data_sources/fdv2/polling_base_test.dart b/packages/common_client/test/data_sources/fdv2/polling_base_test.dart new file mode 100644 index 0000000..8563e79 --- /dev/null +++ b/packages/common_client/test/data_sources/fdv2/polling_base_test.dart @@ -0,0 +1,390 @@ +import 'dart:convert'; + +import 'package:http/http.dart' as http; +import 'package:http/testing.dart'; +import 'package:launchdarkly_common_client/src/config/service_endpoints.dart'; +import 'package:launchdarkly_common_client/src/data_sources/fdv2/payload.dart'; +import 'package:launchdarkly_common_client/src/data_sources/fdv2/polling_base.dart'; +import 'package:launchdarkly_common_client/src/data_sources/fdv2/requestor.dart'; +import 'package:launchdarkly_common_client/src/data_sources/fdv2/source_result.dart'; +import 'package:launchdarkly_dart_common/launchdarkly_dart_common.dart' + hide ServiceEndpoints; +import 'package:test/test.dart'; + +FDv2PollingBase makePollingBase( + MockClient innerClient, { + DateTime Function()? now, +}) { + final requestor = FDv2Requestor( + logger: LDLogger(), + endpoints: ServiceEndpoints.custom(polling: 'https://example.test'), + contextEncoded: 'CTX', + contextJson: '{"key":"test"}', + usePost: false, + withReasons: false, + httpProperties: HttpProperties(), + httpClientFactory: (props) => + HttpClient(client: innerClient, httpProperties: props), + ); + return FDv2PollingBase( + logger: LDLogger(), + requestor: requestor, + now: now, + ); +} + +/// Builds a complete xfer-full FDv2 events collection JSON body with a +/// single put-object for `flag-eval`. +String buildXferFullBody({ + String state = 'sel-1', + int targetVersion = 1, + int payloadVersion = 1, + String flagKey = 'my-flag', +}) { + return jsonEncode({ + 'events': [ + { + 'event': 'server-intent', + 'data': { + 'payloads': [ + { + 'id': 'p1', + 'target': targetVersion, + 'intentCode': 'xfer-full', + 'reason': 'test', + } + ] + } + }, + { + 'event': 'put-object', + 'data': { + 'kind': 'flag-eval', + 'key': flagKey, + 'version': payloadVersion, + 'object': { + 'value': true, + 'version': payloadVersion, + 'variation': 0, + 'trackEvents': false, + }, + } + }, + { + 'event': 'payload-transferred', + 'data': { + 'state': state, + 'version': payloadVersion, + } + }, + ] + }); +} + +void main() { + group('200 response with valid payload', () { + test('produces a ChangeSetResult with the parsed payload', () async { + final mock = MockClient((request) async { + return http.Response(buildXferFullBody(state: 'sel-99', payloadVersion: 99), + 200); + }); + + final base = makePollingBase(mock); + final result = await base.pollOnce(); + + expect(result, isA()); + final cs = result as ChangeSetResult; + expect(cs.payload.type, equals(PayloadType.full)); + expect(cs.payload.selector.state, equals('sel-99')); + expect(cs.payload.selector.version, equals(99)); + expect(cs.payload.updates, hasLength(1)); + expect(cs.payload.updates[0].key, equals('my-flag')); + expect(cs.persist, isTrue); + }); + + test('propagates the x-ld-envid header to the result', () async { + final mock = MockClient((request) async { + return http.Response(buildXferFullBody(), 200, + headers: {'x-ld-envid': 'env-abc'}); + }); + + final base = makePollingBase(mock); + final result = await base.pollOnce(); + + expect((result as ChangeSetResult).environmentId, equals('env-abc')); + }); + + test('sets freshness to the result of the now function', () async { + final fixedNow = DateTime.utc(2026, 4, 16, 12, 0, 0); + final mock = MockClient((request) async { + return http.Response(buildXferFullBody(), 200); + }); + + final base = makePollingBase(mock, now: () => fixedNow); + final result = await base.pollOnce(); + + expect((result as ChangeSetResult).freshness, equals(fixedNow)); + }); + }); + + group('304 Not Modified', () { + test('produces a ChangeSetResult with PayloadType.none', () async { + final mock = MockClient((request) async { + return http.Response('', 304); + }); + + final base = makePollingBase(mock); + final result = await base.pollOnce(); + + expect(result, isA()); + final cs = result as ChangeSetResult; + expect(cs.payload.type, equals(PayloadType.none)); + expect(cs.payload.updates, isEmpty); + expect(cs.persist, isTrue); + }); + }); + + group('FDv1 fallback', () { + test( + 'returns terminalError with fdv1Fallback=true when ' + 'x-ld-fd-fallback is true', () async { + final mock = MockClient((request) async { + return http.Response('', 200, + headers: {'x-ld-fd-fallback': 'true'}); + }); + + final base = makePollingBase(mock); + final result = await base.pollOnce(); + + expect(result, isA()); + final status = result as StatusResult; + expect(status.state, equals(SourceState.terminalError)); + expect(status.fdv1Fallback, isTrue); + }); + + test('does not engage fallback when header is missing', () async { + final mock = MockClient((request) async { + return http.Response(buildXferFullBody(), 200); + }); + + final base = makePollingBase(mock); + final result = await base.pollOnce(); + + expect(result, isA()); + expect(result.fdv1Fallback, isFalse); + }); + }); + + group('HTTP error classification', () { + test('400 is interrupted (recoverable)', () async { + final mock = MockClient((request) async { + return http.Response('bad request', 400); + }); + + final base = makePollingBase(mock); + final result = await base.pollOnce(); + + final status = result as StatusResult; + expect(status.state, equals(SourceState.interrupted)); + expect(status.statusCode, equals(400)); + }); + + test('408 is interrupted', () async { + final mock = MockClient((request) async { + return http.Response('timeout', 408); + }); + + final base = makePollingBase(mock); + final result = await base.pollOnce(); + + expect((result as StatusResult).state, + equals(SourceState.interrupted)); + }); + + test('429 is interrupted', () async { + final mock = MockClient((request) async { + return http.Response('rate limited', 429); + }); + + final base = makePollingBase(mock); + final result = await base.pollOnce(); + + expect((result as StatusResult).state, + equals(SourceState.interrupted)); + }); + + test('401 is terminalError', () async { + final mock = MockClient((request) async { + return http.Response('unauthorized', 401); + }); + + final base = makePollingBase(mock); + final result = await base.pollOnce(); + + expect((result as StatusResult).state, + equals(SourceState.terminalError)); + }); + + test('403 is terminalError', () async { + final mock = MockClient((request) async { + return http.Response('forbidden', 403); + }); + + final base = makePollingBase(mock); + final result = await base.pollOnce(); + + expect((result as StatusResult).state, + equals(SourceState.terminalError)); + }); + + test('500 is interrupted (5xx is recoverable)', () async { + final mock = MockClient((request) async { + return http.Response('server error', 500); + }); + + final base = makePollingBase(mock); + final result = await base.pollOnce(); + + expect((result as StatusResult).state, + equals(SourceState.interrupted)); + }); + }); + + group('network failures', () { + test('returns interrupted when the requestor throws', () async { + final mock = MockClient((request) async { + throw Exception('connection refused'); + }); + + final base = makePollingBase(mock); + final result = await base.pollOnce(); + + expect((result as StatusResult).state, + equals(SourceState.interrupted)); + }); + }); + + group('malformed bodies', () { + test('returns interrupted when body is not valid JSON', () async { + final mock = MockClient((request) async { + return http.Response('not json', 200); + }); + + final base = makePollingBase(mock); + final result = await base.pollOnce(); + + expect((result as StatusResult).state, + equals(SourceState.interrupted)); + }); + + test('returns interrupted when body is JSON but not an object', + () async { + final mock = MockClient((request) async { + return http.Response('[1, 2, 3]', 200); + }); + + final base = makePollingBase(mock); + final result = await base.pollOnce(); + + expect((result as StatusResult).state, + equals(SourceState.interrupted)); + }); + + test('returns interrupted when no payload-transferred is present', + () async { + final body = jsonEncode({ + 'events': [ + { + 'event': 'server-intent', + 'data': { + 'payloads': [ + { + 'id': 'p1', + 'target': 1, + 'intentCode': 'xfer-full', + 'reason': 'test', + } + ] + } + } + ] + }); + final mock = MockClient((request) async { + return http.Response(body, 200); + }); + + final base = makePollingBase(mock); + final result = await base.pollOnce(); + + expect((result as StatusResult).state, + equals(SourceState.interrupted)); + }); + }); + + group('protocol-level outcomes', () { + test('goodbye event produces a goodbye StatusResult', () async { + final body = jsonEncode({ + 'events': [ + { + 'event': 'server-intent', + 'data': { + 'payloads': [ + { + 'id': 'p1', + 'target': 1, + 'intentCode': 'xfer-full', + 'reason': 'test', + } + ] + } + }, + { + 'event': 'goodbye', + 'data': {'reason': 'maintenance'} + }, + ] + }); + final mock = MockClient((request) async { + return http.Response(body, 200); + }); + + final base = makePollingBase(mock); + final result = await base.pollOnce(); + + expect((result as StatusResult).state, equals(SourceState.goodbye)); + }); + + test('server error event produces interrupted', () async { + final body = jsonEncode({ + 'events': [ + { + 'event': 'server-intent', + 'data': { + 'payloads': [ + { + 'id': 'p1', + 'target': 1, + 'intentCode': 'xfer-full', + 'reason': 'test', + } + ] + } + }, + { + 'event': 'error', + 'data': {'payload_id': 'p1', 'reason': 'oops'} + }, + ] + }); + final mock = MockClient((request) async { + return http.Response(body, 200); + }); + + final base = makePollingBase(mock); + final result = await base.pollOnce(); + + expect((result as StatusResult).state, + equals(SourceState.interrupted)); + }); + }); +} diff --git a/packages/common_client/test/data_sources/fdv2/requestor_test.dart b/packages/common_client/test/data_sources/fdv2/requestor_test.dart new file mode 100644 index 0000000..542606b --- /dev/null +++ b/packages/common_client/test/data_sources/fdv2/requestor_test.dart @@ -0,0 +1,276 @@ +import 'package:http/http.dart' as http; +import 'package:http/testing.dart'; +import 'package:launchdarkly_common_client/src/data_sources/fdv2/requestor.dart'; +import 'package:launchdarkly_common_client/src/data_sources/fdv2/selector.dart'; +import 'package:launchdarkly_common_client/src/config/service_endpoints.dart'; +import 'package:launchdarkly_dart_common/launchdarkly_dart_common.dart' + hide ServiceEndpoints; +import 'package:test/test.dart'; + +FDv2Requestor makeRequestor( + MockClient innerClient, { + bool usePost = false, + bool withReasons = false, + String contextEncoded = 'eyJrZXkiOiJ0ZXN0In0=', + String contextJson = '{"key":"test"}', + HttpProperties? httpProperties, +}) { + return FDv2Requestor( + logger: LDLogger(), + endpoints: ServiceEndpoints.custom(polling: 'https://example.test'), + contextEncoded: contextEncoded, + contextJson: contextJson, + usePost: usePost, + withReasons: withReasons, + httpProperties: httpProperties ?? HttpProperties(), + httpClientFactory: (props) => + HttpClient(client: innerClient, httpProperties: props), + ); +} + +void main() { + group('GET requests', () { + test('builds polling GET URL with encoded context in path', () async { + late Uri capturedUri; + late String capturedMethod; + final mock = MockClient((request) async { + capturedUri = request.url; + capturedMethod = request.method; + return http.Response('{}', 200); + }); + + final requestor = + makeRequestor(mock, contextEncoded: 'ENC123'); + await requestor.request(); + + expect(capturedMethod, equals('GET')); + expect(capturedUri.path, equals('/sdk/poll/eval/ENC123')); + expect(capturedUri.host, equals('example.test')); + }); + + test('does not send a body on GET', () async { + late String capturedBody; + final mock = MockClient((request) async { + capturedBody = request.body; + return http.Response('{}', 200); + }); + final requestor = makeRequestor(mock); + await requestor.request(); + + expect(capturedBody, isEmpty); + }); + }); + + group('POST requests', () { + test('builds polling POST URL without context in path', () async { + late Uri capturedUri; + late String capturedMethod; + final mock = MockClient((request) async { + capturedUri = request.url; + capturedMethod = request.method; + return http.Response('{}', 200); + }); + + final requestor = makeRequestor(mock, usePost: true); + await requestor.request(); + + expect(capturedMethod, equals('POST')); + expect(capturedUri.path, equals('/sdk/poll/eval')); + }); + + test('sends the context JSON as the request body', () async { + late String capturedBody; + final mock = MockClient((request) async { + capturedBody = request.body; + return http.Response('{}', 200); + }); + + final requestor = makeRequestor( + mock, + usePost: true, + contextJson: '{"key":"alice"}', + ); + await requestor.request(); + + expect(capturedBody, equals('{"key":"alice"}')); + }); + + test('sets content-type header on POST', () async { + late http.BaseRequest capturedRequest; + final mock = MockClient((request) async { + capturedRequest = request; + return http.Response('{}', 200); + }); + + final requestor = makeRequestor(mock, usePost: true); + await requestor.request(); + + expect( + capturedRequest.headers, + containsPair('content-type', 'application/json'), + ); + }); + }); + + group('query parameters', () { + test('omits basis when selector is empty', () async { + late Uri capturedUri; + final mock = MockClient((request) async { + capturedUri = request.url; + return http.Response('{}', 200); + }); + + final requestor = makeRequestor(mock); + await requestor.request(); + + expect(capturedUri.queryParameters.containsKey('basis'), isFalse); + }); + + test('includes basis when selector is non-empty', () async { + late Uri capturedUri; + final mock = MockClient((request) async { + capturedUri = request.url; + return http.Response('{}', 200); + }); + + final requestor = makeRequestor(mock); + await requestor.request( + basis: Selector(state: '(p:abc:42)', version: 42)); + + expect(capturedUri.queryParameters['basis'], equals('(p:abc:42)')); + }); + + test('includes withReasons=true when configured', () async { + late Uri capturedUri; + final mock = MockClient((request) async { + capturedUri = request.url; + return http.Response('{}', 200); + }); + + final requestor = makeRequestor(mock, withReasons: true); + await requestor.request(); + + expect(capturedUri.queryParameters['withReasons'], equals('true')); + }); + + test('omits withReasons when not configured', () async { + late Uri capturedUri; + final mock = MockClient((request) async { + capturedUri = request.url; + return http.Response('{}', 200); + }); + + final requestor = makeRequestor(mock); + await requestor.request(); + + expect( + capturedUri.queryParameters.containsKey('withReasons'), isFalse); + }); + }); + + group('etag handling', () { + test('does not send if-none-match on the first request', () async { + Map? capturedHeaders; + final mock = MockClient((request) async { + capturedHeaders = request.headers; + return http.Response('{}', 200); + }); + + final requestor = makeRequestor(mock); + await requestor.request(); + + expect(capturedHeaders!.containsKey('if-none-match'), isFalse); + }); + + test('sends if-none-match on subsequent requests', () async { + var requestNumber = 0; + Map? secondRequestHeaders; + final mock = MockClient((request) async { + requestNumber++; + if (requestNumber == 1) { + return http.Response('{}', 200, headers: {'etag': 'abc-123'}); + } + secondRequestHeaders = request.headers; + return http.Response('{}', 200); + }); + + final requestor = makeRequestor(mock); + await requestor.request(); + await requestor.request(); + + expect( + secondRequestHeaders, + containsPair('if-none-match', 'abc-123'), + ); + }); + + test('updates etag when a new one is returned', () async { + var requestNumber = 0; + Map? thirdRequestHeaders; + final mock = MockClient((request) async { + requestNumber++; + if (requestNumber == 1) { + return http.Response('{}', 200, headers: {'etag': 'abc-123'}); + } + if (requestNumber == 2) { + return http.Response('{}', 200, headers: {'etag': 'xyz-456'}); + } + thirdRequestHeaders = request.headers; + return http.Response('{}', 200); + }); + + final requestor = makeRequestor(mock); + await requestor.request(); + await requestor.request(); + await requestor.request(); + + expect( + thirdRequestHeaders, + containsPair('if-none-match', 'xyz-456'), + ); + }); + }); + + group('response shape', () { + test('returns status, headers, and body', () async { + final mock = MockClient((request) async { + return http.Response('{"key":"value"}', 200, + headers: {'x-ld-envid': 'env-1'}); + }); + + final requestor = makeRequestor(mock); + final response = await requestor.request(); + + expect(response.status, equals(200)); + expect(response.headers, containsPair('x-ld-envid', 'env-1')); + expect(response.body, equals('{"key":"value"}')); + }); + + test('propagates non-success status codes', () async { + final mock = MockClient((request) async { + return http.Response('error', 500); + }); + + final requestor = makeRequestor(mock); + final response = await requestor.request(); + + expect(response.status, equals(500)); + expect(response.body, equals('error')); + }); + }); + + group('errors', () { + test('throws on network error', () async { + final mock = MockClient((request) async { + throw Exception('connection refused'); + }); + + final requestor = makeRequestor(mock); + + expect( + () => requestor.request(), + throwsException, + ); + }); + }); +} From 5c3d6f4f0e131a13d722b5689f819e65dd69c177 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Mon, 27 Apr 2026 16:15:40 -0700 Subject: [PATCH 03/10] style: dart format --- .../lib/src/data_sources/fdv2/endpoints.dart | 3 +- .../src/data_sources/fdv2/polling_base.dart | 3 +- .../lib/src/data_sources/fdv2/requestor.dart | 7 ++-- .../data_sources/fdv2/polling_base_test.dart | 40 +++++++------------ .../data_sources/fdv2/requestor_test.dart | 6 +-- 5 files changed, 22 insertions(+), 37 deletions(-) diff --git a/packages/common_client/lib/src/data_sources/fdv2/endpoints.dart b/packages/common_client/lib/src/data_sources/fdv2/endpoints.dart index 325d023..08d3b4b 100644 --- a/packages/common_client/lib/src/data_sources/fdv2/endpoints.dart +++ b/packages/common_client/lib/src/data_sources/fdv2/endpoints.dart @@ -13,8 +13,7 @@ abstract final class FDv2Endpoints { /// Builds the polling GET path with the base64url-encoded context /// embedded in the URL path. - static String pollingGet(String encodedContext) => - '$polling/$encodedContext'; + static String pollingGet(String encodedContext) => '$polling/$encodedContext'; /// Builds the streaming GET path with the base64url-encoded context /// embedded in the URL path. diff --git a/packages/common_client/lib/src/data_sources/fdv2/polling_base.dart b/packages/common_client/lib/src/data_sources/fdv2/polling_base.dart index ee9e0f0..2bf00d1 100644 --- a/packages/common_client/lib/src/data_sources/fdv2/polling_base.dart +++ b/packages/common_client/lib/src/data_sources/fdv2/polling_base.dart @@ -40,8 +40,7 @@ final class FDv2PollingBase { /// Performs a single poll. Never throws; all errors are reported as /// [StatusResult]s. - Future pollOnce( - {Selector basis = Selector.empty}) async { + Future pollOnce({Selector basis = Selector.empty}) async { final RequestorResponse response; try { response = await _requestor.request(basis: basis); diff --git a/packages/common_client/lib/src/data_sources/fdv2/requestor.dart b/packages/common_client/lib/src/data_sources/fdv2/requestor.dart index 518f8ed..b1b16ca 100644 --- a/packages/common_client/lib/src/data_sources/fdv2/requestor.dart +++ b/packages/common_client/lib/src/data_sources/fdv2/requestor.dart @@ -58,8 +58,7 @@ final class FDv2Requestor { /// Sends a single poll request, optionally including a [basis] selector /// for delta updates. Throws on network errors; otherwise returns the /// raw response. Tracks `ETag` for subsequent calls on this instance. - Future request( - {Selector basis = Selector.empty}) async { + Future request({Selector basis = Selector.empty}) async { final uri = _buildUri(basis: basis); final method = _usePost ? RequestMethod.post : RequestMethod.get; final additionalHeaders = {}; @@ -102,6 +101,8 @@ final class FDv2Requestor { } final url = appendPath(_baseUrl, path); final uri = Uri.parse(url); - return queryParams.isEmpty ? uri : uri.replace(queryParameters: queryParams); + return queryParams.isEmpty + ? uri + : uri.replace(queryParameters: queryParams); } } diff --git a/packages/common_client/test/data_sources/fdv2/polling_base_test.dart b/packages/common_client/test/data_sources/fdv2/polling_base_test.dart index 8563e79..14b9d11 100644 --- a/packages/common_client/test/data_sources/fdv2/polling_base_test.dart +++ b/packages/common_client/test/data_sources/fdv2/polling_base_test.dart @@ -85,8 +85,8 @@ void main() { group('200 response with valid payload', () { test('produces a ChangeSetResult with the parsed payload', () async { final mock = MockClient((request) async { - return http.Response(buildXferFullBody(state: 'sel-99', payloadVersion: 99), - 200); + return http.Response( + buildXferFullBody(state: 'sel-99', payloadVersion: 99), 200); }); final base = makePollingBase(mock); @@ -149,8 +149,7 @@ void main() { 'returns terminalError with fdv1Fallback=true when ' 'x-ld-fd-fallback is true', () async { final mock = MockClient((request) async { - return http.Response('', 200, - headers: {'x-ld-fd-fallback': 'true'}); + return http.Response('', 200, headers: {'x-ld-fd-fallback': 'true'}); }); final base = makePollingBase(mock); @@ -197,8 +196,7 @@ void main() { final base = makePollingBase(mock); final result = await base.pollOnce(); - expect((result as StatusResult).state, - equals(SourceState.interrupted)); + expect((result as StatusResult).state, equals(SourceState.interrupted)); }); test('429 is interrupted', () async { @@ -209,8 +207,7 @@ void main() { final base = makePollingBase(mock); final result = await base.pollOnce(); - expect((result as StatusResult).state, - equals(SourceState.interrupted)); + expect((result as StatusResult).state, equals(SourceState.interrupted)); }); test('401 is terminalError', () async { @@ -221,8 +218,7 @@ void main() { final base = makePollingBase(mock); final result = await base.pollOnce(); - expect((result as StatusResult).state, - equals(SourceState.terminalError)); + expect((result as StatusResult).state, equals(SourceState.terminalError)); }); test('403 is terminalError', () async { @@ -233,8 +229,7 @@ void main() { final base = makePollingBase(mock); final result = await base.pollOnce(); - expect((result as StatusResult).state, - equals(SourceState.terminalError)); + expect((result as StatusResult).state, equals(SourceState.terminalError)); }); test('500 is interrupted (5xx is recoverable)', () async { @@ -245,8 +240,7 @@ void main() { final base = makePollingBase(mock); final result = await base.pollOnce(); - expect((result as StatusResult).state, - equals(SourceState.interrupted)); + expect((result as StatusResult).state, equals(SourceState.interrupted)); }); }); @@ -259,8 +253,7 @@ void main() { final base = makePollingBase(mock); final result = await base.pollOnce(); - expect((result as StatusResult).state, - equals(SourceState.interrupted)); + expect((result as StatusResult).state, equals(SourceState.interrupted)); }); }); @@ -273,12 +266,10 @@ void main() { final base = makePollingBase(mock); final result = await base.pollOnce(); - expect((result as StatusResult).state, - equals(SourceState.interrupted)); + expect((result as StatusResult).state, equals(SourceState.interrupted)); }); - test('returns interrupted when body is JSON but not an object', - () async { + test('returns interrupted when body is JSON but not an object', () async { final mock = MockClient((request) async { return http.Response('[1, 2, 3]', 200); }); @@ -286,8 +277,7 @@ void main() { final base = makePollingBase(mock); final result = await base.pollOnce(); - expect((result as StatusResult).state, - equals(SourceState.interrupted)); + expect((result as StatusResult).state, equals(SourceState.interrupted)); }); test('returns interrupted when no payload-transferred is present', @@ -316,8 +306,7 @@ void main() { final base = makePollingBase(mock); final result = await base.pollOnce(); - expect((result as StatusResult).state, - equals(SourceState.interrupted)); + expect((result as StatusResult).state, equals(SourceState.interrupted)); }); }); @@ -383,8 +372,7 @@ void main() { final base = makePollingBase(mock); final result = await base.pollOnce(); - expect((result as StatusResult).state, - equals(SourceState.interrupted)); + expect((result as StatusResult).state, equals(SourceState.interrupted)); }); }); } diff --git a/packages/common_client/test/data_sources/fdv2/requestor_test.dart b/packages/common_client/test/data_sources/fdv2/requestor_test.dart index 542606b..651487e 100644 --- a/packages/common_client/test/data_sources/fdv2/requestor_test.dart +++ b/packages/common_client/test/data_sources/fdv2/requestor_test.dart @@ -39,8 +39,7 @@ void main() { return http.Response('{}', 200); }); - final requestor = - makeRequestor(mock, contextEncoded: 'ENC123'); + final requestor = makeRequestor(mock, contextEncoded: 'ENC123'); await requestor.request(); expect(capturedMethod, equals('GET')); @@ -163,8 +162,7 @@ void main() { final requestor = makeRequestor(mock); await requestor.request(); - expect( - capturedUri.queryParameters.containsKey('withReasons'), isFalse); + expect(capturedUri.queryParameters.containsKey('withReasons'), isFalse); }); }); From e65bfb01c11b239bd2cafa9ecf9b0a25173c73c5 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Mon, 27 Apr 2026 16:21:03 -0700 Subject: [PATCH 04/10] style: replace unicode arrows with ASCII --- .../lib/src/data_sources/fdv2/polling_base.dart | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/common_client/lib/src/data_sources/fdv2/polling_base.dart b/packages/common_client/lib/src/data_sources/fdv2/polling_base.dart index 2bf00d1..1d312a7 100644 --- a/packages/common_client/lib/src/data_sources/fdv2/polling_base.dart +++ b/packages/common_client/lib/src/data_sources/fdv2/polling_base.dart @@ -15,14 +15,14 @@ import 'source_result.dart'; /// /// Wraps an [FDv2Requestor] with FDv2 protocol semantics: /// -/// - Network errors → [SourceState.interrupted]. -/// - `x-ld-fd-fallback: true` header → terminal error with +/// - Network errors --> [SourceState.interrupted]. +/// - `x-ld-fd-fallback: true` header --> terminal error with /// `fdv1Fallback: true`. -/// - HTTP `304 Not Modified` → an empty change set with +/// - HTTP `304 Not Modified` --> an empty change set with /// [PayloadType.none], confirming the cached data is current. -/// - Other 4xx/5xx → interrupted (recoverable) or terminalError +/// - Other 4xx/5xx --> interrupted (recoverable) or terminalError /// (non-recoverable) based on [isHttpGloballyRecoverable]. -/// - `200` → body is parsed as an [FDv2EventsCollection] and fed +/// - `200` --> body is parsed as an [FDv2EventsCollection] and fed /// through an [FDv2ProtocolHandler]. The first emitted action /// determines the result. final class FDv2PollingBase { From 2b59ed9b13310d34dcdd0070dda8246236663ae2 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Tue, 28 Apr 2026 08:41:09 -0700 Subject: [PATCH 05/10] fix: address review findings on FDv2 requestor and polling base Fixes from review of PR #259: - pollOnce now honors its 'never throws' contract: widen the try/catch in _parseBody to cover the FDv2EventsCollection.fromJson cast and per-event fromJson casts. Malformed event shapes (non-Map elements in events/payloads, wrong-type object fields) return interrupted instead of propagating a TypeError. - ETag is now persisted only on 200. A 5xx with an ETag could otherwise poison the next request and cause a follow-up 304 to be misclassified as 'cache is current' even when the SDK has no successful payload. A 304 leaves the existing ETag alone (it confirms the stored value). - URL building uses Uri parsing instead of string concatenation, so custom polling URLs with embedded query parameters (e.g. a relay proxy with a token) are preserved correctly. Our query params merge with the base URL's. - Network exception messages no longer echo into the public StatusResult.message. They are categorized into fixed strings (Network/TLS/Timeout); the full err.toString() stays in the warn log only. SocketException, TlsException, and HandshakeException details (remote IP, cert CN, OS error codes) no longer surface on dataSourceStatus.lastError.message. - x-ld-fd-fallback header is matched case-insensitively. Doc notes that the header takes precedence over body and status code. - Debug log line no longer interpolates the full URL (which embeds the base64url-encoded context in GET mode). - HTTP error paths (4xx / 5xx) now log warn/error like the network and JSON parse paths do. - FDv2Endpoints.streaming doc now reflects that the constant serves both POST and GET (via streamingGet). - FDv2Requestor doc states the serial-only contract for request(). - Defensive guard in the requestor: basis is omitted when the selector's state string is empty, even if isEmpty=false (covers Selector(state: '', version: N)). New tests cover malformed event shapes, ETag handling on 4xx/5xx and 304, custom polling URL with query parameters, basis/withReasons on POST mode, fallback header precedence over 200/304, case- insensitive header matching, intent-none on 200, heartbeat-only response, and exception-message sanitization. --- .../lib/src/data_sources/fdv2/endpoints.dart | 8 +- .../src/data_sources/fdv2/polling_base.dart | 119 +++++++---- .../lib/src/data_sources/fdv2/requestor.dart | 63 ++++-- .../data_sources/fdv2/polling_base_test.dart | 197 ++++++++++++++++++ .../data_sources/fdv2/requestor_test.dart | 180 ++++++++++++++++ 5 files changed, 499 insertions(+), 68 deletions(-) diff --git a/packages/common_client/lib/src/data_sources/fdv2/endpoints.dart b/packages/common_client/lib/src/data_sources/fdv2/endpoints.dart index 08d3b4b..8b50bda 100644 --- a/packages/common_client/lib/src/data_sources/fdv2/endpoints.dart +++ b/packages/common_client/lib/src/data_sources/fdv2/endpoints.dart @@ -3,12 +3,12 @@ /// These paths are uniform across mobile and browser SDKs; FDv2 does /// not distinguish between platforms at the endpoint level. abstract final class FDv2Endpoints { - /// Polling path used for POST requests. The evaluation context is - /// sent in the request body. + /// Polling path. Used as-is for POST requests (context sent in the + /// request body) and as the prefix for GET requests via [pollingGet]. static const String polling = '/sdk/poll/eval'; - /// Streaming path used for POST requests. The evaluation context is - /// sent in the request body. + /// Streaming path. Used as-is for POST requests (context sent in the + /// request body) and as the prefix for GET requests via [streamingGet]. static const String streaming = '/sdk/stream/eval'; /// Builds the polling GET path with the base64url-encoded context diff --git a/packages/common_client/lib/src/data_sources/fdv2/polling_base.dart b/packages/common_client/lib/src/data_sources/fdv2/polling_base.dart index 1d312a7..0190446 100644 --- a/packages/common_client/lib/src/data_sources/fdv2/polling_base.dart +++ b/packages/common_client/lib/src/data_sources/fdv2/polling_base.dart @@ -15,9 +15,13 @@ import 'source_result.dart'; /// /// Wraps an [FDv2Requestor] with FDv2 protocol semantics: /// -/// - Network errors --> [SourceState.interrupted]. +/// - Network errors --> [SourceState.interrupted] with a sanitized +/// message; the full error detail is logged at warn level. /// - `x-ld-fd-fallback: true` header --> terminal error with -/// `fdv1Fallback: true`. +/// `fdv1Fallback: true`. This check takes precedence over the body +/// and over the status code: if the server signals fallback, the +/// SDK switches to FDv1 regardless of whether a `200`, `304`, or +/// error response carries the header. /// - HTTP `304 Not Modified` --> an empty change set with /// [PayloadType.none], confirming the cached data is current. /// - Other 4xx/5xx --> interrupted (recoverable) or terminalError @@ -38,21 +42,24 @@ final class FDv2PollingBase { _requestor = requestor, _now = now ?? DateTime.now; - /// Performs a single poll. Never throws; all errors are reported as - /// [StatusResult]s. + /// Performs a single poll. Never throws; all failures, including + /// malformed response bodies, are reported as [StatusResult]s. Future pollOnce({Selector basis = Selector.empty}) async { final RequestorResponse response; try { response = await _requestor.request(basis: basis); } catch (err) { _logger.warn('Polling request failed: $err'); - return FDv2SourceResults.interrupted(message: err.toString()); + return FDv2SourceResults.interrupted(message: _describeError(err)); } return _processResponse(response); } FDv2SourceResult _processResponse(RequestorResponse response) { - final fdv1Fallback = response.headers['x-ld-fd-fallback'] == 'true'; + // Match `x-ld-fd-fallback` case-insensitively. Servers shouldn't send + // mixed case, but it costs nothing to be lenient on input. + final fdv1Fallback = + response.headers['x-ld-fd-fallback']?.toLowerCase() == 'true'; final environmentId = response.headers['x-ld-envid']; if (fdv1Fallback) { @@ -76,11 +83,13 @@ final class FDv2PollingBase { if (response.status >= 400) { final message = 'Received unexpected status code: ${response.status}'; if (isHttpGloballyRecoverable(response.status)) { + _logger.warn('$message; will retry'); return FDv2SourceResults.interrupted( statusCode: response.status, message: message, ); } + _logger.error('$message; will not retry'); return FDv2SourceResults.terminalError( statusCode: response.status, message: message, @@ -94,7 +103,10 @@ final class FDv2PollingBase { RequestorResponse response, { String? environmentId, }) { - final Map json; + // The whole parse path is wrapped: jsonDecode plus the structural + // casts inside FDv2EventsCollection.fromJson and the per-event + // PutObjectEvent/DeleteObjectEvent/PayloadIntent/etc. fromJson calls + // can all throw on shapes the protocol types don't accept. try { final decoded = jsonDecode(response.body); if (decoded is! Map) { @@ -103,50 +115,67 @@ final class FDv2PollingBase { message: 'Polling response was not a JSON object', ); } - json = decoded; + + final collection = FDv2EventsCollection.fromJson(decoded); + final handler = FDv2ProtocolHandler( + objProcessors: {flagEvalKind: processFlagEval}, + logger: _logger, + ); + + for (final event in collection.events) { + final action = handler.processEvent(event); + switch (action) { + case ActionPayload(:final payload): + return ChangeSetResult( + payload: payload, + environmentId: environmentId, + freshness: _now(), + persist: true, + ); + case ActionGoodbye(:final reason): + return FDv2SourceResults.goodbyeResult(message: reason); + case ActionServerError(:final reason): + return FDv2SourceResults.interrupted(message: reason); + case ActionError(:final message): + return FDv2SourceResults.interrupted(message: message); + case ActionNone(): + // Continue accumulating events until a payload-transferred or + // terminal action is reached. + break; + } + } + + // The response had no payload-transferred event. The protocol + // handler is left in a partial state with nothing to emit, which + // is a protocol violation for a polling response. + return FDv2SourceResults.interrupted( + statusCode: response.status, + message: 'Polling response did not include a complete payload', + ); } catch (err) { - _logger.error('Failed to parse polling response body as JSON: $err'); + _logger.error('Failed to parse polling response: $err'); return FDv2SourceResults.interrupted( statusCode: response.status, - message: 'Polling response body was not valid JSON', + message: 'Polling response body was malformed', ); } + } - final collection = FDv2EventsCollection.fromJson(json); - final handler = FDv2ProtocolHandler( - objProcessors: {flagEvalKind: processFlagEval}, - logger: _logger, - ); - - for (final event in collection.events) { - final action = handler.processEvent(event); - switch (action) { - case ActionPayload(:final payload): - return ChangeSetResult( - payload: payload, - environmentId: environmentId, - freshness: _now(), - persist: true, - ); - case ActionGoodbye(:final reason): - return FDv2SourceResults.goodbyeResult(message: reason); - case ActionServerError(:final reason): - return FDv2SourceResults.interrupted(message: reason); - case ActionError(:final message): - return FDv2SourceResults.interrupted(message: message); - case ActionNone(): - // Continue accumulating events until a payload-transferred or - // terminal action is reached. - break; - } + /// Categorizes a network exception into a fixed, sanitized message. + /// The full exception (which for `SocketException` / `TlsException` + /// can include remote address, certificate detail, and OS error + /// strings) stays in the warn log, not in the public status surface. + String _describeError(Object err) { + final type = err.runtimeType.toString(); + if (type.contains('Timeout')) { + return 'Polling request timed out'; } - - // The response had no payload-transferred event. The protocol handler - // is left in a partial state with nothing to emit, which is a - // protocol violation for a polling response. - return FDv2SourceResults.interrupted( - statusCode: response.status, - message: 'Polling response did not include a complete payload', - ); + if (type.contains('Tls') || type.contains('Handshake')) { + return 'TLS error during polling request'; + } + if (type.contains('Socket') || type.contains('HttpException')) { + return 'Network error during polling request'; + } + return 'Polling request failed'; } } diff --git a/packages/common_client/lib/src/data_sources/fdv2/requestor.dart b/packages/common_client/lib/src/data_sources/fdv2/requestor.dart index b1b16ca..14f2e63 100644 --- a/packages/common_client/lib/src/data_sources/fdv2/requestor.dart +++ b/packages/common_client/lib/src/data_sources/fdv2/requestor.dart @@ -20,16 +20,21 @@ typedef RequestorResponse = ({ /// /// Pure HTTP layer: builds the URL, sends the request, tracks `ETag` /// across calls on the same instance, and returns the raw response. It -/// does no FDv2 protocol parsing or error classification — that is the +/// does no FDv2 protocol parsing or error classification -- that is the /// responsibility of the caller (see [FDv2PollingBase]). /// /// One [FDv2Requestor] is bound to a single evaluation context. Switching /// contexts requires a fresh instance so a previous context's `ETag` /// can never leak into a request for a different context. +/// +/// Calls to [request] are not safe to interleave on a single instance -- +/// `ETag` tracking assumes serial requests. Callers (the polling +/// synchronizer) must wait for each [request] to complete before issuing +/// the next. final class FDv2Requestor { final LDLogger _logger; final HttpClient _client; - final String _baseUrl; + final Uri _baseUri; final String _contextEncoded; final String _contextJson; final bool _usePost; @@ -46,7 +51,7 @@ final class FDv2Requestor { required HttpProperties httpProperties, HttpClientFactory httpClientFactory = _defaultHttpClientFactory, }) : _logger = logger.subLogger('FDv2Requestor'), - _baseUrl = endpoints.polling, + _baseUri = Uri.parse(endpoints.polling), _contextEncoded = contextEncoded, _contextJson = contextJson, _usePost = usePost, @@ -57,7 +62,8 @@ final class FDv2Requestor { /// Sends a single poll request, optionally including a [basis] selector /// for delta updates. Throws on network errors; otherwise returns the - /// raw response. Tracks `ETag` for subsequent calls on this instance. + /// response. Tracks `ETag` across successful (`200`) responses on this + /// instance. Future request({Selector basis = Selector.empty}) async { final uri = _buildUri(basis: basis); final method = _usePost ? RequestMethod.post : RequestMethod.get; @@ -66,8 +72,10 @@ final class FDv2Requestor { additionalHeaders['if-none-match'] = _lastEtag!; } - _logger.debug( - 'FDv2 poll: method=$method, uri=$uri, etag=$_lastEtag, basis=${basis.state}'); + // Avoid logging the full URI -- in GET mode it embeds the + // base64url-encoded context, which is reversible PII. + _logger.debug('FDv2 poll: method=$method, hasEtag=${_lastEtag != null}, ' + 'hasBasis=${basis.isNotEmpty}'); final response = await _client.request( method, @@ -76,9 +84,15 @@ final class FDv2Requestor { body: _usePost ? _contextJson : null, ); - final etag = response.headers['etag']; - if (etag != null) { - _lastEtag = etag; + // Only persist the ETag from a successful response. Non-200 responses + // could carry stale or hostile ETag values that would taint future + // conditional requests. A 304 confirms the existing ETag still matches, + // so leaving the stored value alone is correct. + if (response.statusCode == 200) { + final etag = response.headers['etag']; + if (etag != null) { + _lastEtag = etag; + } } return ( @@ -89,20 +103,31 @@ final class FDv2Requestor { } Uri _buildUri({required Selector basis}) { - final path = _usePost + final addedPath = _usePost ? FDv2Endpoints.polling : FDv2Endpoints.pollingGet(_contextEncoded); - final queryParams = {}; + + // Compose against the parsed base URI so a custom polling URL + // carrying its own query parameters (e.g. a relay proxy with a token) + // is preserved correctly. String concatenation against `_baseUri` + // would land the appended path inside the query component. + final basePath = _baseUri.path.endsWith('/') + ? _baseUri.path.substring(0, _baseUri.path.length - 1) + : _baseUri.path; + final mergedPath = '$basePath$addedPath'; + + final mergedQuery = {}; + mergedQuery.addAll(_baseUri.queryParameters); if (_withReasons) { - queryParams['withReasons'] = 'true'; + mergedQuery['withReasons'] = 'true'; } - if (basis.isNotEmpty) { - queryParams['basis'] = basis.state!; + if (basis.isNotEmpty && basis.state!.isNotEmpty) { + mergedQuery['basis'] = basis.state!; } - final url = appendPath(_baseUrl, path); - final uri = Uri.parse(url); - return queryParams.isEmpty - ? uri - : uri.replace(queryParameters: queryParams); + + return _baseUri.replace( + path: mergedPath, + queryParameters: mergedQuery.isEmpty ? null : mergedQuery, + ); } } diff --git a/packages/common_client/test/data_sources/fdv2/polling_base_test.dart b/packages/common_client/test/data_sources/fdv2/polling_base_test.dart index 14b9d11..c01dc44 100644 --- a/packages/common_client/test/data_sources/fdv2/polling_base_test.dart +++ b/packages/common_client/test/data_sources/fdv2/polling_base_test.dart @@ -374,5 +374,202 @@ void main() { expect((result as StatusResult).state, equals(SourceState.interrupted)); }); + + test('intent-none on a 200 produces a none change set', () async { + final body = jsonEncode({ + 'events': [ + { + 'event': 'server-intent', + 'data': { + 'payloads': [ + { + 'id': 'p1', + 'target': 7, + 'intentCode': 'none', + 'reason': 'up-to-date', + } + ] + } + }, + ] + }); + final mock = MockClient((request) async { + return http.Response(body, 200); + }); + + final base = makePollingBase(mock); + final result = await base.pollOnce(); + + expect(result, isA()); + final cs = result as ChangeSetResult; + expect(cs.payload.type, equals(PayloadType.none)); + expect(cs.payload.updates, isEmpty); + }); + + test('heartbeat-only response is interrupted', () async { + final body = jsonEncode({ + 'events': [ + {'event': 'heart-beat', 'data': {}} + ] + }); + final mock = MockClient((request) async { + return http.Response(body, 200); + }); + + final base = makePollingBase(mock); + final result = await base.pollOnce(); + + expect((result as StatusResult).state, equals(SourceState.interrupted)); + }); + }); + + group('malformed event shapes (do not throw)', () { + test('non-Map element in events array produces interrupted', () async { + final body = jsonEncode({ + 'events': [42] + }); + final mock = MockClient((request) async { + return http.Response(body, 200); + }); + + final base = makePollingBase(mock); + final result = await base.pollOnce(); + + expect((result as StatusResult).state, equals(SourceState.interrupted)); + }); + + test('non-Map element in payloads array produces interrupted', () async { + final body = jsonEncode({ + 'events': [ + { + 'event': 'server-intent', + 'data': { + 'payloads': ['not-an-object'] + } + } + ] + }); + final mock = MockClient((request) async { + return http.Response(body, 200); + }); + + final base = makePollingBase(mock); + final result = await base.pollOnce(); + + expect((result as StatusResult).state, equals(SourceState.interrupted)); + }); + + test('object field of put-object that is not a Map produces interrupted', + () async { + final body = jsonEncode({ + 'events': [ + { + 'event': 'server-intent', + 'data': { + 'payloads': [ + { + 'id': 'p1', + 'target': 1, + 'intentCode': 'xfer-full', + 'reason': 'test', + } + ] + } + }, + { + 'event': 'put-object', + 'data': { + 'kind': 'flag-eval', + 'key': 'k', + 'version': 1, + 'object': 'not-a-map', + } + }, + ] + }); + final mock = MockClient((request) async { + return http.Response(body, 200); + }); + + final base = makePollingBase(mock); + final result = await base.pollOnce(); + + // Either interrupted (if the cast throws) or interrupted (if the + // event is silently skipped and no payload-transferred follows). + // Both outcomes are acceptable; the contract is "do not throw". + expect((result as StatusResult).state, equals(SourceState.interrupted)); + }); + }); + + group('FDv1 fallback precedence', () { + test('fallback header takes precedence over a 200 with valid payload', + () async { + final mock = MockClient((request) async { + return http.Response(buildXferFullBody(), 200, + headers: {'x-ld-fd-fallback': 'true'}); + }); + + final base = makePollingBase(mock); + final result = await base.pollOnce(); + + expect(result, isA()); + final status = result as StatusResult; + expect(status.state, equals(SourceState.terminalError)); + expect(status.fdv1Fallback, isTrue); + }); + + test('fallback header takes precedence over a 304', () async { + final mock = MockClient((request) async { + return http.Response('', 304, headers: {'x-ld-fd-fallback': 'true'}); + }); + + final base = makePollingBase(mock); + final result = await base.pollOnce(); + + expect((result as StatusResult).state, equals(SourceState.terminalError)); + expect(result.fdv1Fallback, isTrue); + }); + + test('fallback header is matched case-insensitively', () async { + final mock = MockClient((request) async { + return http.Response('', 200, headers: {'x-ld-fd-fallback': 'TRUE'}); + }); + + final base = makePollingBase(mock); + final result = await base.pollOnce(); + + expect((result as StatusResult).state, equals(SourceState.terminalError)); + expect(result.fdv1Fallback, isTrue); + }); + + test('fallback header value other than true is ignored', () async { + final mock = MockClient((request) async { + return http.Response(buildXferFullBody(), 200, + headers: {'x-ld-fd-fallback': 'false'}); + }); + + final base = makePollingBase(mock); + final result = await base.pollOnce(); + + expect(result, isA()); + expect(result.fdv1Fallback, isFalse); + }); + }); + + group('error message sanitization', () { + test('exception message is not echoed verbatim into the result', () async { + const sensitive = '203.0.113.5:443 cert CN=internal.example.com'; + final mock = MockClient((request) async { + throw Exception(sensitive); + }); + + final base = makePollingBase(mock); + final result = await base.pollOnce(); + + final status = result as StatusResult; + expect(status.state, equals(SourceState.interrupted)); + expect(status.message, isNotNull); + expect(status.message, isNot(contains(sensitive))); + }); }); } diff --git a/packages/common_client/test/data_sources/fdv2/requestor_test.dart b/packages/common_client/test/data_sources/fdv2/requestor_test.dart index 651487e..8d60d90 100644 --- a/packages/common_client/test/data_sources/fdv2/requestor_test.dart +++ b/packages/common_client/test/data_sources/fdv2/requestor_test.dart @@ -271,4 +271,184 @@ void main() { ); }); }); + + group('etag is only persisted on 200', () { + test('etag returned on 4xx is not sent on next request', () async { + var requestNumber = 0; + Map? secondRequestHeaders; + final mock = MockClient((request) async { + requestNumber++; + if (requestNumber == 1) { + return http.Response('error', 400, headers: {'etag': 'poisoned'}); + } + secondRequestHeaders = request.headers; + return http.Response('{}', 200); + }); + + final requestor = makeRequestor(mock); + await requestor.request(); + await requestor.request(); + + expect(secondRequestHeaders!.containsKey('if-none-match'), isFalse); + }); + + test('etag returned on 5xx is not sent on next request', () async { + var requestNumber = 0; + Map? secondRequestHeaders; + final mock = MockClient((request) async { + requestNumber++; + if (requestNumber == 1) { + return http.Response('error', 500, headers: {'etag': 'poisoned'}); + } + secondRequestHeaders = request.headers; + return http.Response('{}', 200); + }); + + final requestor = makeRequestor(mock); + await requestor.request(); + await requestor.request(); + + expect(secondRequestHeaders!.containsKey('if-none-match'), isFalse); + }); + + test( + '304 does not overwrite the previously stored etag ' + '(it confirms the existing one)', () async { + var requestNumber = 0; + Map? thirdRequestHeaders; + final mock = MockClient((request) async { + requestNumber++; + if (requestNumber == 1) { + return http.Response('{}', 200, headers: {'etag': 'first'}); + } + if (requestNumber == 2) { + // 304 returned without an etag header -- the SDK should still + // remember "first" from the prior 200 response. + return http.Response('', 304); + } + thirdRequestHeaders = request.headers; + return http.Response('{}', 200); + }); + + final requestor = makeRequestor(mock); + await requestor.request(); + await requestor.request(); + await requestor.request(); + + expect(thirdRequestHeaders, containsPair('if-none-match', 'first')); + }); + }); + + group('custom polling URL with embedded query parameters', () { + test( + 'preserves query parameters from the base URL ' + 'and appends our own', () async { + late Uri capturedUri; + final mock = MockClient((request) async { + capturedUri = request.url; + return http.Response('{}', 200); + }); + + final requestor = FDv2Requestor( + logger: LDLogger(), + endpoints: ServiceEndpoints.custom( + polling: 'https://relay.example.com/prefix?token=abc123'), + contextEncoded: 'CTX', + contextJson: '{"key":"x"}', + usePost: false, + withReasons: true, + httpProperties: HttpProperties(), + httpClientFactory: (props) => + HttpClient(client: mock, httpProperties: props), + ); + await requestor.request(basis: Selector(state: 'sel-1', version: 1)); + + expect(capturedUri.host, equals('relay.example.com')); + expect(capturedUri.path, equals('/prefix/sdk/poll/eval/CTX')); + expect(capturedUri.queryParameters['token'], equals('abc123')); + expect(capturedUri.queryParameters['withReasons'], equals('true')); + expect(capturedUri.queryParameters['basis'], equals('sel-1')); + }); + }); + + group('basis and withReasons with POST', () { + test('sends basis as query parameter even on POST', () async { + late Uri capturedUri; + final mock = MockClient((request) async { + capturedUri = request.url; + return http.Response('{}', 200); + }); + + final requestor = makeRequestor(mock, usePost: true); + await requestor.request(basis: Selector(state: 'sel-2', version: 2)); + + expect(capturedUri.queryParameters['basis'], equals('sel-2')); + }); + + test('sends withReasons=true as query parameter on POST', () async { + late Uri capturedUri; + final mock = MockClient((request) async { + capturedUri = request.url; + return http.Response('{}', 200); + }); + + final requestor = makeRequestor(mock, usePost: true, withReasons: true); + await requestor.request(); + + expect(capturedUri.queryParameters['withReasons'], equals('true')); + }); + }); + + group('selector edge cases', () { + test('basis is omitted when state is empty even if isNotEmpty', () async { + // Defensive: a Selector(state: '', version: 1) constructs as + // isEmpty=false. The requestor should still not send basis=. + late Uri capturedUri; + final mock = MockClient((request) async { + capturedUri = request.url; + return http.Response('{}', 200); + }); + + final requestor = makeRequestor(mock); + await requestor.request(basis: Selector(state: '', version: 1)); + + expect(capturedUri.queryParameters.containsKey('basis'), isFalse); + }); + }); + + group('debug logging does not leak the encoded context', () { + test('the context segment of the URL is not logged', () async { + final captured = _CapturingAdapter(); + final logger = LDLogger(adapter: captured, level: LDLogLevel.debug); + final mock = MockClient((request) async { + return http.Response('{}', 200); + }); + + final requestor = FDv2Requestor( + logger: logger, + endpoints: ServiceEndpoints.custom(polling: 'https://example.test'), + contextEncoded: 'SECRET-ENCODED-CONTEXT', + contextJson: '{"key":"x"}', + usePost: false, + withReasons: false, + httpProperties: HttpProperties(), + httpClientFactory: (props) => + HttpClient(client: mock, httpProperties: props), + ); + await requestor.request(); + + for (final message in captured.messages) { + expect(message, isNot(contains('SECRET-ENCODED-CONTEXT'))); + } + }); + }); +} + +class _CapturingAdapter implements LDLogAdapter { + final List messages = []; + + @override + void log(LDLogRecord record) { + messages.add(record.message); + } } From 70fa4c7bc0f73ac61f64107395cfca61521659c4 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Tue, 28 Apr 2026 14:54:53 -0700 Subject: [PATCH 06/10] fix: address round-2 review findings on FDv2 polling base - Drop the raw exception interpolation from the warn log on network errors. http.ClientException's toString() formats as 'ClientException: , uri=', which embeds the base64url-encoded context in GET mode. Log only the sanitized category string in both warn and the public StatusResult.message. - Use 'is' checks (TimeoutException, http.ClientException) instead of substring matches against runtimeType.toString(). The previous code was dead in practice because IOClient wraps SocketException / HttpException as ClientException before they reach _describeError, and runtimeType.toString() is mangled by dart2js minification on web. The 'is' approach is minification-safe. - Reject empty-string ETags. An empty unquoted token is invalid per RFC 7232 and 'if-none-match: ' on the next request can be interpreted by some servers as 'match anything', pinning the SDK to a permanent 304. - Use queryParametersAll instead of queryParameters when reading the base URL's query so duplicate-key params like ?dup=1&dup=2 round-trip both values. - Drop server-controlled exception detail from the parse-error log; log the type name at error level and the full err+stack at debug. Capture the stack trace. - Fix misleading test comment that suggested two outcome paths existed when only one does. Tests added: warn-log content does not contain encoded context on a network-error path (the round-1 leak through a different channel), per-type pinning of _describeError (TimeoutException -> 'timed out', http.ClientException -> 'Network error', other -> 'Polling request failed'), empty-string ETag is not stored, 304-with-new-ETag does not overwrite the stored ETag, base URL with trailing slash does not produce a double-slash, base URL with duplicate-key query params preserves all values. Shared CapturingLogAdapter extracted to test/data_sources/fdv2/support/. --- .../src/data_sources/fdv2/polling_base.dart | 48 +++++--- .../lib/src/data_sources/fdv2/requestor.dart | 13 +- .../data_sources/fdv2/polling_base_test.dart | 76 +++++++++++- .../data_sources/fdv2/requestor_test.dart | 111 ++++++++++++++++-- .../fdv2/support/capturing_log_adapter.dart | 15 +++ 5 files changed, 234 insertions(+), 29 deletions(-) create mode 100644 packages/common_client/test/data_sources/fdv2/support/capturing_log_adapter.dart diff --git a/packages/common_client/lib/src/data_sources/fdv2/polling_base.dart b/packages/common_client/lib/src/data_sources/fdv2/polling_base.dart index 0190446..2e4ebe8 100644 --- a/packages/common_client/lib/src/data_sources/fdv2/polling_base.dart +++ b/packages/common_client/lib/src/data_sources/fdv2/polling_base.dart @@ -1,5 +1,7 @@ +import 'dart:async'; import 'dart:convert'; +import 'package:http/http.dart' as http; import 'package:launchdarkly_dart_common/launchdarkly_dart_common.dart'; import 'flag_eval_mapper.dart'; @@ -16,7 +18,7 @@ import 'source_result.dart'; /// Wraps an [FDv2Requestor] with FDv2 protocol semantics: /// /// - Network errors --> [SourceState.interrupted] with a sanitized -/// message; the full error detail is logged at warn level. +/// message. /// - `x-ld-fd-fallback: true` header --> terminal error with /// `fdv1Fallback: true`. This check takes precedence over the body /// and over the status code: if the server signals fallback, the @@ -49,8 +51,13 @@ final class FDv2PollingBase { try { response = await _requestor.request(basis: basis); } catch (err) { - _logger.warn('Polling request failed: $err'); - return FDv2SourceResults.interrupted(message: _describeError(err)); + // Log only the sanitized form. The raw exception's `toString()` can + // embed PII (e.g. `http.ClientException` formats as + // `'ClientException: , uri='`, and the URL contains + // the base64url-encoded context in GET mode). + final sanitized = _describeError(err); + _logger.warn('Polling request failed: $sanitized'); + return FDv2SourceResults.interrupted(message: sanitized); } return _processResponse(response); } @@ -152,8 +159,13 @@ final class FDv2PollingBase { statusCode: response.status, message: 'Polling response did not include a complete payload', ); - } catch (err) { - _logger.error('Failed to parse polling response: $err'); + } catch (err, stack) { + // Log only the type at error level (not the message — `jsonDecode` + // includes a slice of the offending body, which is server-supplied). + // The full detail goes to debug, where it is gated by the user's + // log level. + _logger.error('Failed to parse polling response (${err.runtimeType})'); + _logger.debug('Polling response parse failure detail: $err\n$stack'); return FDv2SourceResults.interrupted( statusCode: response.status, message: 'Polling response body was malformed', @@ -161,21 +173,29 @@ final class FDv2PollingBase { } } - /// Categorizes a network exception into a fixed, sanitized message. - /// The full exception (which for `SocketException` / `TlsException` - /// can include remote address, certificate detail, and OS error - /// strings) stays in the warn log, not in the public status surface. + /// Categorizes an exception thrown by the requestor into a fixed, + /// sanitized message. The raw exception's string form (which can carry + /// remote address, certificate detail, OS error strings, or — in the + /// case of `http.ClientException` — the full request URL) is never + /// echoed to the public status surface or to the warn log. + /// + /// Type checks via `is` are minification-safe (unlike substring + /// matches against `runtimeType.toString()`). String _describeError(Object err) { - final type = err.runtimeType.toString(); - if (type.contains('Timeout')) { + if (err is TimeoutException) { return 'Polling request timed out'; } + if (err is http.ClientException) { + return 'Network error during polling request'; + } + // dart:io's TlsException / HandshakeException can't be caught by `is` + // here without making this file io-only, so fall back to the type + // name. This is a best-effort label; if minification mangles the + // type name we land in the default branch below, which is still safe. + final type = err.runtimeType.toString(); if (type.contains('Tls') || type.contains('Handshake')) { return 'TLS error during polling request'; } - if (type.contains('Socket') || type.contains('HttpException')) { - return 'Network error during polling request'; - } return 'Polling request failed'; } } diff --git a/packages/common_client/lib/src/data_sources/fdv2/requestor.dart b/packages/common_client/lib/src/data_sources/fdv2/requestor.dart index 14f2e63..1412cec 100644 --- a/packages/common_client/lib/src/data_sources/fdv2/requestor.dart +++ b/packages/common_client/lib/src/data_sources/fdv2/requestor.dart @@ -88,9 +88,14 @@ final class FDv2Requestor { // could carry stale or hostile ETag values that would taint future // conditional requests. A 304 confirms the existing ETag still matches, // so leaving the stored value alone is correct. + // + // Reject empty-string ETags: an unquoted empty token is invalid per + // RFC 7232 §2.1, and sending `if-none-match: ` on the next request + // could be interpreted by some servers as "match anything" and pin + // the SDK to a permanent 304. if (response.statusCode == 200) { final etag = response.headers['etag']; - if (etag != null) { + if (etag != null && etag.isNotEmpty) { _lastEtag = etag; } } @@ -116,8 +121,10 @@ final class FDv2Requestor { : _baseUri.path; final mergedPath = '$basePath$addedPath'; - final mergedQuery = {}; - mergedQuery.addAll(_baseUri.queryParameters); + // Use queryParametersAll so a base URL like `?dup=1&dup=2` round-trips + // both values; the simpler `queryParameters` map collapses duplicates. + final mergedQuery = {}; + mergedQuery.addAll(_baseUri.queryParametersAll); if (_withReasons) { mergedQuery['withReasons'] = 'true'; } diff --git a/packages/common_client/test/data_sources/fdv2/polling_base_test.dart b/packages/common_client/test/data_sources/fdv2/polling_base_test.dart index c01dc44..3c7bcd9 100644 --- a/packages/common_client/test/data_sources/fdv2/polling_base_test.dart +++ b/packages/common_client/test/data_sources/fdv2/polling_base_test.dart @@ -1,3 +1,4 @@ +import 'dart:async'; import 'dart:convert'; import 'package:http/http.dart' as http; @@ -11,6 +12,8 @@ import 'package:launchdarkly_dart_common/launchdarkly_dart_common.dart' hide ServiceEndpoints; import 'package:test/test.dart'; +import 'support/capturing_log_adapter.dart'; + FDv2PollingBase makePollingBase( MockClient innerClient, { DateTime Function()? now, @@ -494,9 +497,8 @@ void main() { final base = makePollingBase(mock); final result = await base.pollOnce(); - // Either interrupted (if the cast throws) or interrupted (if the - // event is silently skipped and no payload-transferred follows). - // Both outcomes are acceptable; the contract is "do not throw". + // The cast inside PutObjectEvent.fromJson throws TypeError; the + // widened catch in _parseBody converts it to interrupted. expect((result as StatusResult).state, equals(SourceState.interrupted)); }); }); @@ -571,5 +573,73 @@ void main() { expect(status.message, isNotNull); expect(status.message, isNot(contains(sensitive))); }); + + test('warn log on a network error does not contain the encoded context', + () async { + // http.ClientException's toString() formats as + // 'ClientException: , uri='. The full URL embeds the + // base64url-encoded context in GET mode. The polling base must + // never log the raw exception. + final captured = CapturingLogAdapter(); + final logger = LDLogger(adapter: captured, level: LDLogLevel.debug); + final mock = MockClient((request) async { + throw http.ClientException('Connection refused', request.url); + }); + + final requestor = FDv2Requestor( + logger: logger, + endpoints: ServiceEndpoints.custom(polling: 'https://example.test'), + contextEncoded: 'SECRET-ENCODED-CONTEXT', + contextJson: '{"key":"x"}', + usePost: false, + withReasons: false, + httpProperties: HttpProperties(), + httpClientFactory: (props) => + HttpClient(client: mock, httpProperties: props), + ); + final base = FDv2PollingBase(logger: logger, requestor: requestor); + await base.pollOnce(); + + for (final message in captured.messages) { + expect(message, isNot(contains('SECRET-ENCODED-CONTEXT'))); + } + }); + + test('TimeoutException maps to "timed out"', () async { + final mock = MockClient((request) async { + throw TimeoutException('exceeded', const Duration(seconds: 1)); + }); + final base = makePollingBase(mock); + final result = await base.pollOnce(); + expect( + (result as StatusResult).message, + equals('Polling request timed out'), + ); + }); + + test('http.ClientException maps to "Network error"', () async { + final mock = MockClient((request) async { + throw http.ClientException( + 'Connection refused', Uri.parse('https://example.test')); + }); + final base = makePollingBase(mock); + final result = await base.pollOnce(); + expect( + (result as StatusResult).message, + equals('Network error during polling request'), + ); + }); + + test('an unknown exception type falls back to a generic message', () async { + final mock = MockClient((request) async { + throw Exception('something'); + }); + final base = makePollingBase(mock); + final result = await base.pollOnce(); + expect( + (result as StatusResult).message, + equals('Polling request failed'), + ); + }); }); } diff --git a/packages/common_client/test/data_sources/fdv2/requestor_test.dart b/packages/common_client/test/data_sources/fdv2/requestor_test.dart index 8d60d90..3b7418e 100644 --- a/packages/common_client/test/data_sources/fdv2/requestor_test.dart +++ b/packages/common_client/test/data_sources/fdv2/requestor_test.dart @@ -1,12 +1,14 @@ import 'package:http/http.dart' as http; import 'package:http/testing.dart'; +import 'package:launchdarkly_common_client/src/config/service_endpoints.dart'; import 'package:launchdarkly_common_client/src/data_sources/fdv2/requestor.dart'; import 'package:launchdarkly_common_client/src/data_sources/fdv2/selector.dart'; -import 'package:launchdarkly_common_client/src/config/service_endpoints.dart'; import 'package:launchdarkly_dart_common/launchdarkly_dart_common.dart' hide ServiceEndpoints; import 'package:test/test.dart'; +import 'support/capturing_log_adapter.dart'; + FDv2Requestor makeRequestor( MockClient innerClient, { bool usePost = false, @@ -418,7 +420,7 @@ void main() { group('debug logging does not leak the encoded context', () { test('the context segment of the URL is not logged', () async { - final captured = _CapturingAdapter(); + final captured = CapturingLogAdapter(); final logger = LDLogger(adapter: captured, level: LDLogLevel.debug); final mock = MockClient((request) async { return http.Response('{}', 200); @@ -442,13 +444,104 @@ void main() { } }); }); -} -class _CapturingAdapter implements LDLogAdapter { - final List messages = []; + group('etag edge cases', () { + test('empty-string etag is not stored', () async { + var requestNumber = 0; + Map? secondHeaders; + final mock = MockClient((request) async { + requestNumber++; + if (requestNumber == 1) { + return http.Response('{}', 200, headers: {'etag': ''}); + } + secondHeaders = request.headers; + return http.Response('{}', 200); + }); + + final requestor = makeRequestor(mock); + await requestor.request(); + await requestor.request(); + + expect(secondHeaders!.containsKey('if-none-match'), isFalse); + }); + + test('304 with a new etag does NOT overwrite the stored etag', () async { + // Pinning current behavior: a 304 response confirms the ETag the + // client already sent; if the server attaches a different ETag we + // ignore it and continue to use the original. Updating on 304 would + // mean trusting an unverified value. + var requestNumber = 0; + Map? thirdHeaders; + final mock = MockClient((request) async { + requestNumber++; + if (requestNumber == 1) { + return http.Response('{}', 200, headers: {'etag': 'first'}); + } + if (requestNumber == 2) { + return http.Response('', 304, headers: {'etag': 'rotated'}); + } + thirdHeaders = request.headers; + return http.Response('{}', 200); + }); + + final requestor = makeRequestor(mock); + await requestor.request(); + await requestor.request(); + await requestor.request(); + + expect(thirdHeaders, containsPair('if-none-match', 'first')); + }); + }); + + group('base URL trailing slash', () { + test('does not produce a double-slash in the merged path', () async { + late Uri capturedUri; + final mock = MockClient((request) async { + capturedUri = request.url; + return http.Response('{}', 200); + }); + + final requestor = FDv2Requestor( + logger: LDLogger(), + endpoints: ServiceEndpoints.custom( + polling: 'https://relay.example.com/prefix/'), + contextEncoded: 'CTX', + contextJson: '{"k":"v"}', + usePost: false, + withReasons: false, + httpProperties: HttpProperties(), + httpClientFactory: (props) => + HttpClient(client: mock, httpProperties: props), + ); + await requestor.request(); + + expect(capturedUri.path, equals('/prefix/sdk/poll/eval/CTX')); + }); + }); - @override - void log(LDLogRecord record) { - messages.add(record.message); - } + group('base URL with duplicate-key query parameters', () { + test('preserves all values for repeated keys', () async { + late Uri capturedUri; + final mock = MockClient((request) async { + capturedUri = request.url; + return http.Response('{}', 200); + }); + + final requestor = FDv2Requestor( + logger: LDLogger(), + endpoints: ServiceEndpoints.custom( + polling: 'https://relay.example.com/?tag=a&tag=b'), + contextEncoded: 'CTX', + contextJson: '{"k":"v"}', + usePost: false, + withReasons: false, + httpProperties: HttpProperties(), + httpClientFactory: (props) => + HttpClient(client: mock, httpProperties: props), + ); + await requestor.request(); + + expect(capturedUri.queryParametersAll['tag'], equals(['a', 'b'])); + }); + }); } diff --git a/packages/common_client/test/data_sources/fdv2/support/capturing_log_adapter.dart b/packages/common_client/test/data_sources/fdv2/support/capturing_log_adapter.dart new file mode 100644 index 0000000..646bc2e --- /dev/null +++ b/packages/common_client/test/data_sources/fdv2/support/capturing_log_adapter.dart @@ -0,0 +1,15 @@ +import 'package:launchdarkly_dart_common/launchdarkly_dart_common.dart'; + +/// An [LDLogAdapter] that captures every log record into a list, for +/// assertions about what does or does not appear in log output. +class CapturingLogAdapter implements LDLogAdapter { + final List records = []; + + /// Convenience: just the message strings. + List get messages => records.map((r) => r.message).toList(); + + @override + void log(LDLogRecord record) { + records.add(record); + } +} From 3fe5f42c8991bf4cbcecf771732f2848a7ac9a0b Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Tue, 28 Apr 2026 15:48:47 -0700 Subject: [PATCH 07/10] refactor: treat x-ld-fd-fallback as an annotation, not a replacement Previously the fallback header short-circuited the response: any fallback signal returned a terminalError and discarded the body. Treat the header as an annotation instead. The body is still parsed, 304 is still treated as no-op, and HTTP errors are still classified by status code; the fdv1Fallback flag is stamped on the resulting FDv2SourceResult. The orchestrator can consume the data and transition to FDv1 in the same step rather than throwing away fresh state. Tests cover fallback alongside: a 200 with a valid payload (still ChangeSetResult, with fallback=true), a 304 (still none-type ChangeSetResult, tagged), a non-recoverable 4xx (terminalError, tagged), a recoverable 5xx (interrupted, tagged), and a malformed body (interrupted, tagged). Case-insensitive header match and 'false' header behavior are unchanged. --- .../src/data_sources/fdv2/polling_base.dart | 49 ++++++---- .../data_sources/fdv2/polling_base_test.dart | 91 +++++++++++++------ 2 files changed, 94 insertions(+), 46 deletions(-) diff --git a/packages/common_client/lib/src/data_sources/fdv2/polling_base.dart b/packages/common_client/lib/src/data_sources/fdv2/polling_base.dart index 2e4ebe8..cacaabc 100644 --- a/packages/common_client/lib/src/data_sources/fdv2/polling_base.dart +++ b/packages/common_client/lib/src/data_sources/fdv2/polling_base.dart @@ -19,11 +19,6 @@ import 'source_result.dart'; /// /// - Network errors --> [SourceState.interrupted] with a sanitized /// message. -/// - `x-ld-fd-fallback: true` header --> terminal error with -/// `fdv1Fallback: true`. This check takes precedence over the body -/// and over the status code: if the server signals fallback, the -/// SDK switches to FDv1 regardless of whether a `200`, `304`, or -/// error response carries the header. /// - HTTP `304 Not Modified` --> an empty change set with /// [PayloadType.none], confirming the cached data is current. /// - Other 4xx/5xx --> interrupted (recoverable) or terminalError @@ -31,6 +26,13 @@ import 'source_result.dart'; /// - `200` --> body is parsed as an [FDv2EventsCollection] and fed /// through an [FDv2ProtocolHandler]. The first emitted action /// determines the result. +/// +/// `x-ld-fd-fallback: true` is treated as an annotation on whatever +/// result the response would otherwise produce: the body is still +/// parsed and used, the 304 is still treated as no-op, errors are +/// still classified by status code, and `fdv1Fallback: true` is +/// stamped on the resulting [FDv2SourceResult]. The orchestrator can +/// consume the data and transition to FDv1 in the same step. final class FDv2PollingBase { final LDLogger _logger; final FDv2Requestor _requestor; @@ -69,14 +71,6 @@ final class FDv2PollingBase { response.headers['x-ld-fd-fallback']?.toLowerCase() == 'true'; final environmentId = response.headers['x-ld-envid']; - if (fdv1Fallback) { - return FDv2SourceResults.terminalError( - statusCode: response.status, - message: 'Server requested FDv1 fallback', - fdv1Fallback: true, - ); - } - // 304 Not Modified means the SDK's cached data is confirmed current. if (response.status == 304) { return ChangeSetResult( @@ -84,6 +78,7 @@ final class FDv2PollingBase { environmentId: environmentId, freshness: _now(), persist: true, + fdv1Fallback: fdv1Fallback, ); } @@ -94,21 +89,28 @@ final class FDv2PollingBase { return FDv2SourceResults.interrupted( statusCode: response.status, message: message, + fdv1Fallback: fdv1Fallback, ); } _logger.error('$message; will not retry'); return FDv2SourceResults.terminalError( statusCode: response.status, message: message, + fdv1Fallback: fdv1Fallback, ); } - return _parseBody(response, environmentId: environmentId); + return _parseBody( + response, + environmentId: environmentId, + fdv1Fallback: fdv1Fallback, + ); } FDv2SourceResult _parseBody( RequestorResponse response, { String? environmentId, + required bool fdv1Fallback, }) { // The whole parse path is wrapped: jsonDecode plus the structural // casts inside FDv2EventsCollection.fromJson and the per-event @@ -120,6 +122,7 @@ final class FDv2PollingBase { return FDv2SourceResults.interrupted( statusCode: response.status, message: 'Polling response was not a JSON object', + fdv1Fallback: fdv1Fallback, ); } @@ -138,13 +141,23 @@ final class FDv2PollingBase { environmentId: environmentId, freshness: _now(), persist: true, + fdv1Fallback: fdv1Fallback, ); case ActionGoodbye(:final reason): - return FDv2SourceResults.goodbyeResult(message: reason); + return FDv2SourceResults.goodbyeResult( + message: reason, + fdv1Fallback: fdv1Fallback, + ); case ActionServerError(:final reason): - return FDv2SourceResults.interrupted(message: reason); + return FDv2SourceResults.interrupted( + message: reason, + fdv1Fallback: fdv1Fallback, + ); case ActionError(:final message): - return FDv2SourceResults.interrupted(message: message); + return FDv2SourceResults.interrupted( + message: message, + fdv1Fallback: fdv1Fallback, + ); case ActionNone(): // Continue accumulating events until a payload-transferred or // terminal action is reached. @@ -158,6 +171,7 @@ final class FDv2PollingBase { return FDv2SourceResults.interrupted( statusCode: response.status, message: 'Polling response did not include a complete payload', + fdv1Fallback: fdv1Fallback, ); } catch (err, stack) { // Log only the type at error level (not the message — `jsonDecode` @@ -169,6 +183,7 @@ final class FDv2PollingBase { return FDv2SourceResults.interrupted( statusCode: response.status, message: 'Polling response body was malformed', + fdv1Fallback: fdv1Fallback, ); } } diff --git a/packages/common_client/test/data_sources/fdv2/polling_base_test.dart b/packages/common_client/test/data_sources/fdv2/polling_base_test.dart index 3c7bcd9..e74f757 100644 --- a/packages/common_client/test/data_sources/fdv2/polling_base_test.dart +++ b/packages/common_client/test/data_sources/fdv2/polling_base_test.dart @@ -147,24 +147,9 @@ void main() { }); }); - group('FDv1 fallback', () { - test( - 'returns terminalError with fdv1Fallback=true when ' - 'x-ld-fd-fallback is true', () async { - final mock = MockClient((request) async { - return http.Response('', 200, headers: {'x-ld-fd-fallback': 'true'}); - }); - - final base = makePollingBase(mock); - final result = await base.pollOnce(); - - expect(result, isA()); - final status = result as StatusResult; - expect(status.state, equals(SourceState.terminalError)); - expect(status.fdv1Fallback, isTrue); - }); - - test('does not engage fallback when header is missing', () async { + group('FDv1 fallback annotation', () { + test('absent header leaves fdv1Fallback false on a successful payload', + () async { final mock = MockClient((request) async { return http.Response(buildXferFullBody(), 200); }); @@ -503,9 +488,10 @@ void main() { }); }); - group('FDv1 fallback precedence', () { - test('fallback header takes precedence over a 200 with valid payload', - () async { + group('FDv1 fallback alongside body and status', () { + test( + 'fallback on a 200 with a valid payload still parses the body and ' + 'tags the result', () async { final mock = MockClient((request) async { return http.Response(buildXferFullBody(), 200, headers: {'x-ld-fd-fallback': 'true'}); @@ -514,13 +500,17 @@ void main() { final base = makePollingBase(mock); final result = await base.pollOnce(); - expect(result, isA()); - final status = result as StatusResult; - expect(status.state, equals(SourceState.terminalError)); - expect(status.fdv1Fallback, isTrue); + // The body is still used: ChangeSetResult, not a status result. + expect(result, isA()); + final cs = result as ChangeSetResult; + expect(cs.fdv1Fallback, isTrue); + expect(cs.payload.type, equals(PayloadType.full)); + expect(cs.payload.updates, hasLength(1)); }); - test('fallback header takes precedence over a 304', () async { + test( + 'fallback on a 304 still produces a none-type ChangeSetResult, ' + 'tagged', () async { final mock = MockClient((request) async { return http.Response('', 304, headers: {'x-ld-fd-fallback': 'true'}); }); @@ -528,23 +518,66 @@ void main() { final base = makePollingBase(mock); final result = await base.pollOnce(); + expect(result, isA()); + final cs = result as ChangeSetResult; + expect(cs.fdv1Fallback, isTrue); + expect(cs.payload.type, equals(PayloadType.none)); + }); + + test('fallback on a non-recoverable 4xx still produces terminalError', + () async { + final mock = MockClient((request) async { + return http.Response('', 401, headers: {'x-ld-fd-fallback': 'true'}); + }); + + final base = makePollingBase(mock); + final result = await base.pollOnce(); + expect((result as StatusResult).state, equals(SourceState.terminalError)); expect(result.fdv1Fallback, isTrue); }); + test('fallback on a recoverable 5xx still produces interrupted', () async { + final mock = MockClient((request) async { + return http.Response('', 503, headers: {'x-ld-fd-fallback': 'true'}); + }); + + final base = makePollingBase(mock); + final result = await base.pollOnce(); + + expect((result as StatusResult).state, equals(SourceState.interrupted)); + expect(result.fdv1Fallback, isTrue); + }); + + test('fallback on a malformed body still produces interrupted, tagged', + () async { + final mock = MockClient((request) async { + return http.Response('not json', 200, + headers: {'x-ld-fd-fallback': 'true'}); + }); + + final base = makePollingBase(mock); + final result = await base.pollOnce(); + + expect((result as StatusResult).state, equals(SourceState.interrupted)); + expect(result.fdv1Fallback, isTrue); + }); + test('fallback header is matched case-insensitively', () async { final mock = MockClient((request) async { - return http.Response('', 200, headers: {'x-ld-fd-fallback': 'TRUE'}); + return http.Response(buildXferFullBody(), 200, + headers: {'x-ld-fd-fallback': 'TRUE'}); }); final base = makePollingBase(mock); final result = await base.pollOnce(); - expect((result as StatusResult).state, equals(SourceState.terminalError)); + expect(result, isA()); expect(result.fdv1Fallback, isTrue); }); - test('fallback header value other than true is ignored', () async { + test('fallback header value other than true does not engage fallback', + () async { final mock = MockClient((request) async { return http.Response(buildXferFullBody(), 200, headers: {'x-ld-fd-fallback': 'false'}); From 86a7a436cdd53585e706abc9e8387fc778e826d7 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Tue, 28 Apr 2026 15:53:08 -0700 Subject: [PATCH 08/10] style: trim x-ld-fd-fallback comment --- .../common_client/lib/src/data_sources/fdv2/polling_base.dart | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/common_client/lib/src/data_sources/fdv2/polling_base.dart b/packages/common_client/lib/src/data_sources/fdv2/polling_base.dart index cacaabc..16cfadb 100644 --- a/packages/common_client/lib/src/data_sources/fdv2/polling_base.dart +++ b/packages/common_client/lib/src/data_sources/fdv2/polling_base.dart @@ -65,8 +65,7 @@ final class FDv2PollingBase { } FDv2SourceResult _processResponse(RequestorResponse response) { - // Match `x-ld-fd-fallback` case-insensitively. Servers shouldn't send - // mixed case, but it costs nothing to be lenient on input. + // Match `x-ld-fd-fallback` case-insensitively. final fdv1Fallback = response.headers['x-ld-fd-fallback']?.toLowerCase() == 'true'; final environmentId = response.headers['x-ld-envid']; From 94bb233b9682920b19fea5c6098e7d15ba792727 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Tue, 28 Apr 2026 15:56:09 -0700 Subject: [PATCH 09/10] test: align log capture with the codebase's mocktail pattern Use Mock implements LDLogAdapter (mocktail) for the two tests that assert log content, matching validating_persistence_test.dart, hook_runner_test.dart, env_context_modifier_test.dart, and operations_test.dart. Drop the bespoke CapturingLogAdapter and the support/ folder it lived in. --- .../data_sources/fdv2/polling_base_test.dart | 22 ++++++++++++++----- .../data_sources/fdv2/requestor_test.dart | 22 ++++++++++++++----- .../fdv2/support/capturing_log_adapter.dart | 15 ------------- 3 files changed, 34 insertions(+), 25 deletions(-) delete mode 100644 packages/common_client/test/data_sources/fdv2/support/capturing_log_adapter.dart diff --git a/packages/common_client/test/data_sources/fdv2/polling_base_test.dart b/packages/common_client/test/data_sources/fdv2/polling_base_test.dart index e74f757..e1ae13a 100644 --- a/packages/common_client/test/data_sources/fdv2/polling_base_test.dart +++ b/packages/common_client/test/data_sources/fdv2/polling_base_test.dart @@ -10,9 +10,10 @@ import 'package:launchdarkly_common_client/src/data_sources/fdv2/requestor.dart' import 'package:launchdarkly_common_client/src/data_sources/fdv2/source_result.dart'; import 'package:launchdarkly_dart_common/launchdarkly_dart_common.dart' hide ServiceEndpoints; +import 'package:mocktail/mocktail.dart'; import 'package:test/test.dart'; -import 'support/capturing_log_adapter.dart'; +class MockLogAdapter extends Mock implements LDLogAdapter {} FDv2PollingBase makePollingBase( MockClient innerClient, { @@ -85,6 +86,14 @@ String buildXferFullBody({ } void main() { + setUpAll(() { + registerFallbackValue(LDLogRecord( + level: LDLogLevel.debug, + message: '', + time: DateTime.now(), + logTag: '')); + }); + group('200 response with valid payload', () { test('produces a ChangeSetResult with the parsed payload', () async { final mock = MockClient((request) async { @@ -613,8 +622,9 @@ void main() { // 'ClientException: , uri='. The full URL embeds the // base64url-encoded context in GET mode. The polling base must // never log the raw exception. - final captured = CapturingLogAdapter(); - final logger = LDLogger(adapter: captured, level: LDLogLevel.debug); + final adapter = MockLogAdapter(); + when(() => adapter.log(any())).thenReturn(null); + final logger = LDLogger(adapter: adapter, level: LDLogLevel.debug); final mock = MockClient((request) async { throw http.ClientException('Connection refused', request.url); }); @@ -633,8 +643,10 @@ void main() { final base = FDv2PollingBase(logger: logger, requestor: requestor); await base.pollOnce(); - for (final message in captured.messages) { - expect(message, isNot(contains('SECRET-ENCODED-CONTEXT'))); + final records = verify(() => adapter.log(captureAny())).captured; + for (final record in records) { + expect((record as LDLogRecord).message, + isNot(contains('SECRET-ENCODED-CONTEXT'))); } }); diff --git a/packages/common_client/test/data_sources/fdv2/requestor_test.dart b/packages/common_client/test/data_sources/fdv2/requestor_test.dart index 3b7418e..05431f9 100644 --- a/packages/common_client/test/data_sources/fdv2/requestor_test.dart +++ b/packages/common_client/test/data_sources/fdv2/requestor_test.dart @@ -5,9 +5,10 @@ import 'package:launchdarkly_common_client/src/data_sources/fdv2/requestor.dart' import 'package:launchdarkly_common_client/src/data_sources/fdv2/selector.dart'; import 'package:launchdarkly_dart_common/launchdarkly_dart_common.dart' hide ServiceEndpoints; +import 'package:mocktail/mocktail.dart'; import 'package:test/test.dart'; -import 'support/capturing_log_adapter.dart'; +class MockLogAdapter extends Mock implements LDLogAdapter {} FDv2Requestor makeRequestor( MockClient innerClient, { @@ -31,6 +32,14 @@ FDv2Requestor makeRequestor( } void main() { + setUpAll(() { + registerFallbackValue(LDLogRecord( + level: LDLogLevel.debug, + message: '', + time: DateTime.now(), + logTag: '')); + }); + group('GET requests', () { test('builds polling GET URL with encoded context in path', () async { late Uri capturedUri; @@ -420,8 +429,9 @@ void main() { group('debug logging does not leak the encoded context', () { test('the context segment of the URL is not logged', () async { - final captured = CapturingLogAdapter(); - final logger = LDLogger(adapter: captured, level: LDLogLevel.debug); + final adapter = MockLogAdapter(); + when(() => adapter.log(any())).thenReturn(null); + final logger = LDLogger(adapter: adapter, level: LDLogLevel.debug); final mock = MockClient((request) async { return http.Response('{}', 200); }); @@ -439,8 +449,10 @@ void main() { ); await requestor.request(); - for (final message in captured.messages) { - expect(message, isNot(contains('SECRET-ENCODED-CONTEXT'))); + final records = verify(() => adapter.log(captureAny())).captured; + for (final record in records) { + expect((record as LDLogRecord).message, + isNot(contains('SECRET-ENCODED-CONTEXT'))); } }); }); diff --git a/packages/common_client/test/data_sources/fdv2/support/capturing_log_adapter.dart b/packages/common_client/test/data_sources/fdv2/support/capturing_log_adapter.dart deleted file mode 100644 index 646bc2e..0000000 --- a/packages/common_client/test/data_sources/fdv2/support/capturing_log_adapter.dart +++ /dev/null @@ -1,15 +0,0 @@ -import 'package:launchdarkly_dart_common/launchdarkly_dart_common.dart'; - -/// An [LDLogAdapter] that captures every log record into a list, for -/// assertions about what does or does not appear in log output. -class CapturingLogAdapter implements LDLogAdapter { - final List records = []; - - /// Convenience: just the message strings. - List get messages => records.map((r) => r.message).toList(); - - @override - void log(LDLogRecord record) { - records.add(record); - } -} From eba8fb081e228964dd57f48c374d7fd2a65880c9 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Tue, 28 Apr 2026 16:16:16 -0700 Subject: [PATCH 10/10] refactor: use Dart 3 case-pattern null narrowing in requestor Replace `if (x != null) { ... x! ... }` with `if (x case final v?) { ... v ... }`. Case-pattern narrowing gives a non-null local without the runtime assertion of the bang operator. For the basis check, the case-pattern combines the null narrowing and the non-empty guard via `when`, replacing the transitive `basis.isNotEmpty && basis.state!.isNotEmpty` pattern with a single `if (basis.state case final state? when state.isNotEmpty)`. --- .../lib/src/data_sources/fdv2/requestor.dart | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/common_client/lib/src/data_sources/fdv2/requestor.dart b/packages/common_client/lib/src/data_sources/fdv2/requestor.dart index 1412cec..3b84518 100644 --- a/packages/common_client/lib/src/data_sources/fdv2/requestor.dart +++ b/packages/common_client/lib/src/data_sources/fdv2/requestor.dart @@ -68,8 +68,8 @@ final class FDv2Requestor { final uri = _buildUri(basis: basis); final method = _usePost ? RequestMethod.post : RequestMethod.get; final additionalHeaders = {}; - if (_lastEtag != null) { - additionalHeaders['if-none-match'] = _lastEtag!; + if (_lastEtag case final etag?) { + additionalHeaders['if-none-match'] = etag; } // Avoid logging the full URI -- in GET mode it embeds the @@ -128,8 +128,8 @@ final class FDv2Requestor { if (_withReasons) { mergedQuery['withReasons'] = 'true'; } - if (basis.isNotEmpty && basis.state!.isNotEmpty) { - mergedQuery['basis'] = basis.state!; + if (basis.state case final state? when state.isNotEmpty) { + mergedQuery['basis'] = state; } return _baseUri.replace(