fix: improves handling of invalid contexts and adds SSE Client logging. (#207)

**Requirements**

- [x] I have added test coverage for new or changed functionality

- [x] I have followed the repository's [pull request submission
guidelines](../blob/main/CONTRIBUTING.md#submitting-pull-requests)
- [x] I have validated my changes against all supported platform
versions

**Related issues**

SDK-1413

**Describe the solution you've provided**

Adds context validity checks to start and identify that prevents state
change if invalid context is provided.
Adds logging to SSE client and necessary adapter to use LDLogger.

Author: tanderson-ld

packages/common/test/ld_context_test.dart

@@ -63,6 +63,22 @@ void main() {
     expect(LDContextBuilder().build().canonicalKey, '');
   });
 
+  test('can modify an invalid context to become valid', () {
+    // Start with an invalid context (no kind specified)
+    final invalidContext = LDContextBuilder().build();
+    expect(invalidContext.valid, false);
+    expect(invalidContext.canonicalKey, '');
+
+    // Modify it to become valid by adding a valid kind
+    final validContext = LDContextBuilder.fromContext(invalidContext)
+        .kind('user', 'user-key')
+        .build();
+
+    expect(validContext.valid, true);
+    expect(validContext.canonicalKey, 'user-key');
+    expect(validContext.keys, <String, String>{'user': 'user-key'});
+  });
+
   test('can change the key of a context during build', () {
     final context = LDContextBuilder()
         .kind('user', 'user-key')

packages/common_client/lib/src/context_modifiers/anonymous_context_modifier.dart

@@ -8,17 +8,20 @@ const _anonContextKeyNamespace = 'LaunchDarkly_AnonContextKey';
 
 final class AnonymousContextModifier implements ContextModifier {
   final Persistence _persistence;
+  final LDLogger _logger;
 
-  AnonymousContextModifier(Persistence persistence)
-      : _persistence = persistence;
+  AnonymousContextModifier(Persistence persistence, LDLogger logger)
+      : _persistence = persistence,
+        _logger = logger;
 
   /// For any anonymous contexts, which do not have keys specified, generate
   /// or read a persisted key for the anonymous kinds present. If persistence
   /// is available, then the key will be stable.
   @override
   Future<LDContext> decorate(LDContext context) async {
     if (!context.valid) {
-      return context;
+      _logger.warn(
+          'AnonymousContextModifier was asked to modify an invalid context and will attempt to do so. This is expected if starting with an empty context.');
     }
     // Before we make a builder we should check if any anonymous contexts
     // without keys exist.

packages/common_client/lib/src/context_modifiers/env_context_modifier.dart

@@ -42,6 +42,11 @@ final class AutoEnvContextModifier implements ContextModifier {
 
   @override
   Future<LDContext> decorate(LDContext context) async {
+    if (!context.valid) {
+      _logger.warn(
+          'AutoEnvContextModifier was asked to modify an invalid context and will attempt to do so. This is expected if starting with an empty context.');
+    }
+
     final builder = LDContextBuilder.fromContext(context);
 
     for (final recipe in _recipes) {

packages/common_client/lib/src/data_sources/streaming_data_source.dart

@@ -11,15 +11,20 @@ import 'data_source_status.dart';
 
 typedef MessageHandler = void Function(MessageEvent);
 typedef ErrorHandler = void Function(dynamic);
-typedef SseClientFactory = SSEClient Function(Uri uri,
-    HttpProperties httpProperties, String? body, SseHttpMethod? method);
+typedef SseClientFactory = SSEClient Function(
+    Uri uri,
+    HttpProperties httpProperties,
+    String? body,
+    SseHttpMethod? method,
+    EventSourceLogger? logger);
 
 SSEClient _defaultClientFactory(Uri uri, HttpProperties httpProperties,
-    String? body, SseHttpMethod? method) {
+    String? body, SseHttpMethod? method, EventSourceLogger? logger) {
   return SSEClient(uri, {'put', 'patch', 'delete'},
       headers: httpProperties.baseHeaders,
       body: body,
-      httpMethod: method ?? SseHttpMethod.get);
+      httpMethod: method ?? SseHttpMethod.get,
+      logger: logger);
 }
 
 final class StreamingDataSource implements DataSource {
@@ -109,7 +114,8 @@ final class StreamingDataSource implements DataSource {
         _uri,
         _httpProperties,
         _useReport ? _contextString : null,
-        _useReport ? SseHttpMethod.report : SseHttpMethod.get);
+        _useReport ? SseHttpMethod.report : SseHttpMethod.get,
+        LDLoggerToEventSourceAdapter(_logger));
 
     _subscription = _client!.stream.listen((event) async {
       if (_stopped) {
@@ -147,3 +153,22 @@ final class StreamingDataSource implements DataSource {
     _dataController.close();
   }
 }
+
+/// Adapter to convert LDLogger to EventSourceLogger
+class LDLoggerToEventSourceAdapter implements EventSourceLogger {
+  final LDLogger _logger;
+
+  LDLoggerToEventSourceAdapter(this._logger);
+
+  @override
+  void debug(String message) => _logger.debug(message);
+
+  @override
+  void info(String message) => _logger.info(message);
+
+  @override
+  void warn(String message) => _logger.warn(message);
+
+  @override
+  void error(String message) => _logger.error(message);
+}

packages/common_client/lib/src/ld_common_client.dart

@@ -308,7 +308,7 @@ final class LDCommonClient {
     _envReport = await _makeEnvironmentReport();
 
     // set up context modifiers, adding the auto env modifier if turned on
-    _modifiers = [AnonymousContextModifier(_persistence)];
+    _modifiers = [AnonymousContextModifier(_persistence, _logger)];
     if (_config.autoEnvAttributes == AutoEnvAttributes.enabled) {
       _modifiers.add(
           AutoEnvContextModifier(_envReport, _persistence, _config.logger));
@@ -440,6 +440,13 @@ final class LDCommonClient {
 
   Future<void> _identifyInternal(LDContext context,
       {bool waitForNetworkResults = false}) async {
+    if (!context.valid) {
+      const message =
+          'LDClient was provided an invalid context. The context will be ignored. Existing flags will be used for evaluations until identify is called with a valid context.';
+      _logger.warn(message);
+      throw Exception(message);
+    }
+
     await _setAndDecorateContext(context);
     final completer = Completer<void>();
     _eventProcessor?.processIdentifyEvent(IdentifyEvent(context: _context));

packages/common_client/test/context_decorators/anonymous_context_modifier_test.dart

@@ -1,11 +1,22 @@
 import 'package:launchdarkly_common_client/launchdarkly_common_client.dart';
 import 'package:launchdarkly_common_client/src/context_modifiers/anonymous_context_modifier.dart';
 import 'package:launchdarkly_common_client/src/persistence/persistence.dart';
+import 'package:launchdarkly_dart_common/launchdarkly_dart_common.dart';
 import 'package:test/test.dart';
+import 'package:mocktail/mocktail.dart';
 
 import '../mock_persistence.dart';
 
+class MockAdapter extends Mock implements LDLogAdapter {}
+
 void main() {
+  setUpAll(() {
+    registerFallbackValue(LDLogRecord(
+        level: LDLogLevel.debug,
+        message: '',
+        time: DateTime.now(),
+        logTag: ''));
+  });
   group('without persistence', () {
     test('it populates keys for anonymous contexts that lack them', () async {
       final context = LDContextBuilder()
@@ -16,7 +27,8 @@ void main() {
           .anonymous(true)
           .build();
 
-      final decorator = AnonymousContextModifier(InMemoryPersistence());
+      final decorator =
+          AnonymousContextModifier(InMemoryPersistence(), LDLogger());
       final decoratedContext = await decorator.decorate(context);
 
       expect(decoratedContext.attributesByKind['user']!.key, isNotEmpty);
@@ -33,7 +45,8 @@ void main() {
           .anonymous(true)
           .build();
 
-      final decorator = AnonymousContextModifier(InMemoryPersistence());
+      final decorator =
+          AnonymousContextModifier(InMemoryPersistence(), LDLogger());
       final decoratedContext = await decorator.decorate(context);
 
       expect(decoratedContext.attributesByKind['user']!.key,
@@ -49,7 +62,8 @@ void main() {
           .anonymous(true)
           .build();
 
-      final decorator = AnonymousContextModifier(InMemoryPersistence());
+      final decorator =
+          AnonymousContextModifier(InMemoryPersistence(), LDLogger());
       final decoratedContext = await decorator.decorate(context);
       final decoratedContext2 = await decorator.decorate(context);
 
@@ -74,7 +88,7 @@ void main() {
         encodePersistenceKey('user'): 'the-user-key',
         encodePersistenceKey('company'): 'the-company-key',
       };
-      final decorator = AnonymousContextModifier(mockPersistence);
+      final decorator = AnonymousContextModifier(mockPersistence, LDLogger());
 
       final decoratedContext = await decorator.decorate(context);
 
@@ -92,7 +106,7 @@ void main() {
           .build();
 
       final mockPersistence = MockPersistence();
-      final decorator = AnonymousContextModifier(mockPersistence);
+      final decorator = AnonymousContextModifier(mockPersistence, LDLogger());
 
       final decoratedContext = await decorator.decorate(context);
 
@@ -106,4 +120,25 @@ void main() {
               encodePersistenceKey('company')]);
     });
   });
+
+  group('invalid context handling', () {
+    test('it logs a info log when asked to modify an invalid context',
+        () async {
+      final invalidContext =
+          LDContextBuilder().build(); // This creates an invalid context
+      final mockAdapter = MockAdapter();
+      final logger = LDLogger(adapter: mockAdapter, level: LDLogLevel.info);
+      final decorator = AnonymousContextModifier(InMemoryPersistence(), logger);
+
+      final result = await decorator.decorate(invalidContext);
+
+      expect(result.valid, false);
+
+      final logRecord = verify(() => mockAdapter.log(captureAny())).captured[0]
+          as LDLogRecord;
+      expect(logRecord.level, LDLogLevel.warn);
+      expect(logRecord.message,
+          'AnonymousContextModifier was asked to modify an invalid context and will attempt to do so. This is expected if starting with an empty context.');
+    });
+  });
 }

packages/common_client/test/context_decorators/env_context_modifier_test.dart

@@ -2,8 +2,19 @@ import 'package:launchdarkly_common_client/src/context_modifiers/env_context_mod
 import 'package:launchdarkly_common_client/src/persistence/persistence.dart';
 import 'package:launchdarkly_dart_common/launchdarkly_dart_common.dart';
 import 'package:test/test.dart';
+import 'package:mocktail/mocktail.dart';
+
+class MockAdapter extends Mock implements LDLogAdapter {}
 
 void main() {
+  setUpAll(() {
+    registerFallbackValue(LDLogRecord(
+        level: LDLogLevel.debug,
+        message: '',
+        time: DateTime.now(),
+        logTag: ''));
+  });
+
   group('env reporter with various configurations', () {
     test('reporter has all attributes', () async {
       final mockPersistence = InMemoryPersistence();
@@ -449,4 +460,66 @@ void main() {
       expect(key1, key2);
     });
   });
+
+  group('invalid context handling', () {
+    test('it logs an info log when asked to modify an invalid context',
+        () async {
+      final invalidContext =
+          LDContextBuilder().build(); // This creates an invalid context
+      final mockPersistence = InMemoryPersistence();
+      final mockAdapter = MockAdapter();
+      final logger = LDLogger(adapter: mockAdapter, level: LDLogLevel.info);
+      final envReporter = ConcreteEnvReporter(
+          applicationInfo: Future.value(null),
+          osInfo: Future.value(null),
+          deviceInfo: Future.value(null),
+          locale: Future.value(null));
+
+      final report = await PrioritizedEnvReportBuilder()
+          .setConfigLayer(envReporter)
+          .build();
+
+      final decorator = AutoEnvContextModifier(report, mockPersistence, logger);
+
+      final result = await decorator.decorate(invalidContext);
+
+      expect(result.valid, false);
+
+      final logRecord = verify(() => mockAdapter.log(captureAny())).captured[0]
+          as LDLogRecord;
+      expect(logRecord.level, LDLogLevel.warn);
+      expect(logRecord.message,
+          'AutoEnvContextModifier was asked to modify an invalid context and will attempt to do so. This is expected if starting with an empty context.');
+    });
+
+    test('it makes an invalid context valid by adding environment attributes',
+        () async {
+      final invalidContext = LDContextBuilder().build();
+      final mockPersistence = InMemoryPersistence();
+      final logger = LDLogger();
+      final envReporter = ConcreteEnvReporter(
+          applicationInfo: Future.value(ApplicationInfo(
+              applicationId: 'mockID',
+              applicationName: 'mockName',
+              applicationVersion: 'mockVersion',
+              applicationVersionName: 'mockVersionName')),
+          osInfo: Future.value(OsInfo(
+              family: 'mockFamily',
+              name: 'mockOsName',
+              version: 'mockOsVersion')),
+          deviceInfo: Future.value(
+              DeviceInfo(model: 'mockModel', manufacturer: 'mockManufacturer')),
+          locale: Future.value('mockLocale'));
+
+      final report = await PrioritizedEnvReportBuilder()
+          .setConfigLayer(envReporter)
+          .build();
+
+      final decorator = AutoEnvContextModifier(report, mockPersistence, logger);
+
+      final result = await decorator.decorate(invalidContext);
+
+      expect(result.valid, true);
+    });
+  });
 }

packages/common_client/test/data_sources/streaming_data_source_test.dart

@@ -60,7 +60,7 @@ class MockSseClient implements SSEClient {
           withReasons: withReasons, useReport: useReport),
       httpProperties: httpProperties,
       clientFactory: (Uri uri, HttpProperties properties, String? body,
-          SseHttpMethod? method) {
+          SseHttpMethod? method, EventSourceLogger? logger) {
         factoryCallback?.call(uri, properties, body, method);
         return client;
       });

packages/common_client/test/ld_dart_client_test.dart

@@ -286,6 +286,45 @@ void main() {
     });
   });
 
+  group('given an invalid context', () {
+    test('start returns false with invalid context', () async {
+      final invalidClient = LDCommonClient(
+          TestConfig('', AutoEnvAttributes.disabled, offline: true),
+          CommonPlatform(),
+          LDContextBuilder()
+              .kind('invalid#kind', '') // invalid kind and invalid key
+              .build(),
+          DiagnosticSdkData(name: '', version: ''));
+
+      expect(await invalidClient.start(), false);
+    });
+
+    test('identify returns IdentifyError with invalid context', () async {
+      final invalidClient = LDCommonClient(
+          TestConfig('', AutoEnvAttributes.disabled, offline: true),
+          CommonPlatform(),
+          LDContextBuilder()
+              .kind('user', 'bob')
+              .build(), // Valid initial context
+          DiagnosticSdkData(name: '', version: ''));
+
+      await invalidClient.start();
+
+      final invalidContext = LDContextBuilder()
+          .kind('invalid#kind', '') // invalid kind and invalid key
+          .build();
+
+      expect(
+          await invalidClient.identify(invalidContext), isA<IdentifyError>());
+
+      // check subsequent identify with valid context is accepted
+      final validContext = LDContextBuilder().kind('user', 'alice').build();
+
+      final identifyResult = await invalidClient.identify(validContext);
+      expect(identifyResult, isA<IdentifyComplete>());
+    });
+  });
+
   group('given mock flag data with prerequisites', () {
     late LDCommonClient client;
     late MockPersistence mockPersistence;