From 241634c3b8e62c2b407f46c0069485a3da6e6b53 Mon Sep 17 00:00:00 2001 From: adams85 <31276480+adams85@users.noreply.github.com> Date: Mon, 29 Apr 2024 13:47:58 +0200 Subject: [PATCH] Error reporting improvements (#86) * Define error codes for evaluation errors (EvaluationErrorCode) and expose them to the user (EvaluationDetails.ErrorCode) * Define error codes for config data refresh errors (RefreshErrorCode) and expose them to the user (RefreshResult.ErrorCode) * Introduce ConfigFetched hook * Make subscription to hooks thread-safe * Adjust tests --- .../ConfigCatClientTests.cs | 99 +++++++++++++- .../ConfigServiceTests.cs | 87 +++++++++++- .../ConfigV2EvaluationTests.cs | 2 +- .../EvaluationTestsBase.cs | 2 +- .../HttpConfigFetcherTests.cs | 4 +- src/ConfigCat.Client.Tests/OverrideTests.cs | 62 ++++++++- src/ConfigCatClient/ConfigCatClient.cs | 23 +++- .../ConfigService/AutoPollConfigService.cs | 8 +- .../ConfigService/ConfigServiceBase.cs | 29 ++-- .../ConfigService/LazyLoadConfigService.cs | 4 +- .../ConfigService/NullConfigService.cs | 6 +- .../Configuration/ConfigCatClientOptions.cs | 7 + .../Evaluation/EvaluationDetails.cs | 10 +- .../Evaluation/EvaluationErrorCode.cs | 32 +++++ .../Evaluation/EvaluationErrorException.cs | 13 ++ .../Evaluation/RolloutEvaluator.cs | 30 ++--- .../Evaluation/RolloutEvaluatorExtensions.cs | 19 ++- src/ConfigCatClient/FetchResult.cs | 12 +- .../Hooks/ConfigFetchedEventArgs.cs | 25 ++++ src/ConfigCatClient/Hooks/Hooks.cs | 126 +++++++++++------- src/ConfigCatClient/Hooks/IProvidesHooks.cs | 7 +- src/ConfigCatClient/Hooks/NullHooks.cs | 2 +- src/ConfigCatClient/Hooks/SafeHooksWrapper.cs | 16 ++- src/ConfigCatClient/HttpConfigFetcher.cs | 14 +- .../Models/ConditionContainer.cs | 2 +- .../Models/InvalidConfigModelException.cs | 8 ++ .../Models/PrerequisiteFlagCondition.cs | 2 +- src/ConfigCatClient/Models/Segment.cs | 2 +- .../Models/SegmentCondition.cs | 2 +- src/ConfigCatClient/Models/SettingValue.cs | 6 +- src/ConfigCatClient/Models/UserCondition.cs | 4 +- src/ConfigCatClient/RefreshErrorCode.cs | 49 +++++++ src/ConfigCatClient/RefreshResult.cs | 19 ++- 33 files changed, 593 insertions(+), 140 deletions(-) create mode 100644 src/ConfigCatClient/Evaluation/EvaluationErrorCode.cs create mode 100644 src/ConfigCatClient/Evaluation/EvaluationErrorException.cs create mode 100644 src/ConfigCatClient/Hooks/ConfigFetchedEventArgs.cs create mode 100644 src/ConfigCatClient/Models/InvalidConfigModelException.cs create mode 100644 src/ConfigCatClient/RefreshErrorCode.cs diff --git a/src/ConfigCat.Client.Tests/ConfigCatClientTests.cs b/src/ConfigCat.Client.Tests/ConfigCatClientTests.cs index ce6c1fa6..ecec60c2 100644 --- a/src/ConfigCat.Client.Tests/ConfigCatClientTests.cs +++ b/src/ConfigCat.Client.Tests/ConfigCatClientTests.cs @@ -249,7 +249,7 @@ public async Task GetValueAsync_EvaluateServiceThrowException_ShouldReturnDefaul [DataRow(false)] [DataRow(true)] [DataTestMethod] - public async Task GetValueDetails_ShouldReturnCorrectEvaluationDetails_SettingIsNotAvailable(bool isAsync) + public async Task GetValueDetails_ShouldReturnCorrectEvaluationDetails_ConfigJsonIsNotAvailable(bool isAsync) { // Arrange @@ -284,6 +284,62 @@ public async Task GetValueDetails_ShouldReturnCorrectEvaluationDetails_SettingIs Assert.IsNull(actual.VariationId); Assert.AreEqual(DateTime.MinValue, actual.FetchTime); Assert.AreSame(user, actual.User); + Assert.AreEqual(EvaluationErrorCode.ConfigJsonNotAvailable, actual.ErrorCode); + Assert.IsNotNull(actual.ErrorMessage); + Assert.IsNull(actual.ErrorException); + Assert.IsNull(actual.MatchedTargetingRule); + Assert.IsNull(actual.MatchedPercentageOption); + } + + [DataRow(false)] + [DataRow(true)] + [DataTestMethod] + public async Task GetValueDetails_ShouldReturnCorrectEvaluationDetails_SettingIsMissing(bool isAsync) + { + // Arrange + + const string key = "does-not-exist"; + const bool defaultValue = false; + + const string cacheKey = "123"; + var configJsonFilePath = Path.Combine("data", "sample_variationid_v5.json"); + var timeStamp = ProjectConfig.GenerateTimeStamp(); + + var client = CreateClientWithMockedFetcher(cacheKey, this.loggerMock, this.fetcherMock, + onFetch: _ => FetchResult.Success(ConfigHelper.FromFile(configJsonFilePath, httpETag: "12345", timeStamp)), + configServiceFactory: (fetcher, cacheParams, loggerWrapper) => + { + return new ManualPollConfigService(this.fetcherMock.Object, cacheParams, loggerWrapper); + }, + out var configService, out _); + + if (isAsync) + { + await client.ForceRefreshAsync(); + } + else + { + client.ForceRefresh(); + } + + var user = new User("a@configcat.com") { Email = "a@configcat.com" }; + + // Act + + var actual = isAsync + ? await client.GetValueDetailsAsync(key, defaultValue, user) + : client.GetValueDetails(key, defaultValue, user); + + // Assert + + Assert.IsNotNull(actual); + Assert.AreEqual(key, actual.Key); + Assert.AreEqual(defaultValue, actual.Value); + Assert.IsTrue(actual.IsDefaultValue); + Assert.IsNull(actual.VariationId); + Assert.AreEqual(timeStamp, actual.FetchTime); + Assert.AreSame(user, actual.User); + Assert.AreEqual(EvaluationErrorCode.SettingKeyMissing, actual.ErrorCode); Assert.IsNotNull(actual.ErrorMessage); Assert.IsNull(actual.ErrorException); Assert.IsNull(actual.MatchedTargetingRule); @@ -336,6 +392,7 @@ public async Task GetValueDetails_ShouldReturnCorrectEvaluationDetails_SettingIs Assert.AreEqual("a0e56eda", actual.VariationId); Assert.AreEqual(timeStamp, actual.FetchTime); Assert.IsNull(actual.User); + Assert.AreEqual(EvaluationErrorCode.None, actual.ErrorCode); Assert.IsNull(actual.ErrorMessage); Assert.IsNull(actual.ErrorException); Assert.IsNull(actual.MatchedTargetingRule); @@ -390,6 +447,7 @@ public async Task GetValueDetails_ShouldReturnCorrectEvaluationDetails_SettingIs Assert.AreEqual("67787ae4", actual.VariationId); Assert.AreEqual(timeStamp, actual.FetchTime); Assert.AreSame(user, actual.User); + Assert.AreEqual(EvaluationErrorCode.None, actual.ErrorCode); Assert.IsNull(actual.ErrorMessage); Assert.IsNull(actual.ErrorException); Assert.IsNotNull(actual.MatchedTargetingRule); @@ -444,6 +502,7 @@ public async Task GetValueDetails_ShouldReturnCorrectEvaluationDetails_SettingIs Assert.AreEqual("67787ae4", actual.VariationId); Assert.AreEqual(timeStamp, actual.FetchTime); Assert.AreSame(user, actual.User); + Assert.AreEqual(EvaluationErrorCode.None, actual.ErrorCode); Assert.IsNull(actual.ErrorMessage); Assert.IsNull(actual.ErrorException); Assert.IsNull(actual.MatchedTargetingRule); @@ -487,6 +546,7 @@ public async Task GetValueDetails_ConfigServiceThrowException_ShouldReturnDefaul Assert.IsNull(actual.VariationId); Assert.AreEqual(DateTime.MinValue, actual.FetchTime); Assert.IsNull(actual.User); + Assert.AreEqual(EvaluationErrorCode.UnexpectedError, actual.ErrorCode); Assert.AreEqual(errorMessage, actual.ErrorMessage); Assert.IsInstanceOfType(actual.ErrorException, typeof(ApplicationException)); Assert.IsNull(actual.MatchedTargetingRule); @@ -550,6 +610,7 @@ public async Task GetValueDetails_EvaluateServiceThrowException_ShouldReturnDefa Assert.IsNull(actual.VariationId); Assert.AreEqual(timeStamp, actual.FetchTime); Assert.AreSame(user, actual.User); + Assert.AreEqual(EvaluationErrorCode.UnexpectedError, actual.ErrorCode); Assert.AreEqual(errorMessage, actual.ErrorMessage); Assert.IsInstanceOfType(actual.ErrorException, typeof(ApplicationException)); Assert.IsNull(actual.MatchedTargetingRule); @@ -619,6 +680,7 @@ public async Task GetAllValueDetails_ShouldReturnCorrectEvaluationDetails(bool i Assert.AreEqual(expectedItem.VariationId, actualDetails.VariationId); Assert.AreEqual(timeStamp, actualDetails.FetchTime); Assert.AreSame(user, actualDetails.User); + Assert.AreEqual(EvaluationErrorCode.None, actualDetails.ErrorCode); Assert.IsNull(actualDetails.ErrorMessage); Assert.IsNull(actualDetails.ErrorException); Assert.IsNotNull(actualDetails.MatchedTargetingRule); @@ -755,6 +817,7 @@ public async Task GetAllValueDetails_EvaluateServiceThrowException_ShouldReturnD Assert.IsNull(actualDetails.VariationId); Assert.AreEqual(timeStamp, actualDetails.FetchTime); Assert.AreSame(user, actualDetails.User); + Assert.AreEqual(EvaluationErrorCode.UnexpectedError, actualDetails.ErrorCode); Assert.AreEqual(errorMessage, actualDetails.ErrorMessage); Assert.IsInstanceOfType(actualDetails.ErrorException, typeof(ApplicationException)); Assert.IsNull(actualDetails.MatchedTargetingRule); @@ -977,6 +1040,7 @@ public async Task ForceRefresh_ShouldInvokeConfigServiceRefreshConfigAsync() this.configServiceMock.Verify(m => m.RefreshConfigAsync(It.IsAny()), Times.Once); Assert.IsTrue(result.IsSuccess); + Assert.AreEqual(RefreshErrorCode.None, result.ErrorCode); Assert.IsNull(result.ErrorMessage); Assert.IsNull(result.ErrorException); } @@ -1002,6 +1066,7 @@ public void ForceRefresh_ShouldInvokeConfigServiceRefreshConfig() this.configServiceMock.Verify(m => m.RefreshConfig(), Times.Once); Assert.IsTrue(result.IsSuccess); + Assert.AreEqual(RefreshErrorCode.None, result.ErrorCode); Assert.IsNull(result.ErrorMessage); Assert.IsNull(result.ErrorException); } @@ -1027,6 +1092,7 @@ public async Task ForceRefreshAsync_ShouldInvokeConfigServiceRefreshConfigAsync( this.configServiceMock.Verify(m => m.RefreshConfigAsync(It.IsAny()), Times.Once); Assert.IsTrue(result.IsSuccess); + Assert.AreEqual(RefreshErrorCode.None, result.ErrorCode); Assert.IsNull(result.ErrorMessage); Assert.IsNull(result.ErrorException); } @@ -1054,6 +1120,7 @@ public async Task ForceRefreshAsync_ConfigServiceThrowException_ShouldNotReThrow this.loggerMock.Verify(m => m.Log(LogLevel.Error, It.IsAny(), ref It.Ref.IsAny, It.IsAny()), Times.Once); Assert.IsFalse(result.IsSuccess); + Assert.AreEqual(RefreshErrorCode.UnexpectedError, result.ErrorCode); Assert.AreEqual(exception.Message, result.ErrorMessage); Assert.AreSame(exception, result.ErrorException); } @@ -1081,6 +1148,7 @@ public void ForceRefresh_ConfigServiceThrowException_ShouldNotReThrowTheExceptio this.loggerMock.Verify(m => m.Log(LogLevel.Error, It.IsAny(), ref It.Ref.IsAny, It.IsAny()), Times.Once); Assert.IsFalse(result.IsSuccess); + Assert.AreEqual(RefreshErrorCode.UnexpectedError, result.ErrorCode); Assert.AreEqual(exception.Message, result.ErrorMessage); Assert.AreSame(exception, result.ErrorException); } @@ -1536,6 +1604,7 @@ public async Task OfflineMode_OfflineToOnlineTransition(string pollingMode) Assert.AreEqual(etag2, ParseETagAsInt32((await configService.GetConfigAsync()).HttpETag)); Assert.IsTrue(refreshResult.IsSuccess); + Assert.AreEqual(RefreshErrorCode.None, refreshResult.ErrorCode); Assert.IsNull(refreshResult.ErrorMessage); Assert.IsNull(refreshResult.ErrorException); @@ -1552,6 +1621,7 @@ public async Task OfflineMode_OfflineToOnlineTransition(string pollingMode) Assert.AreEqual(etag3, ParseETagAsInt32((await configService.GetConfigAsync()).HttpETag)); Assert.IsTrue(refreshResult.IsSuccess); + Assert.AreEqual(RefreshErrorCode.None, refreshResult.ErrorCode); Assert.IsNull(refreshResult.ErrorMessage); Assert.IsNull(refreshResult.ErrorException); } @@ -1666,6 +1736,7 @@ public async Task OfflineMode_OnlineToOfflineTransition(string pollingMode) Assert.AreEqual(etag1, ParseETagAsInt32((await configService.GetConfigAsync()).HttpETag)); Assert.IsFalse(refreshResult.IsSuccess); + Assert.AreEqual(RefreshErrorCode.OfflineClient, refreshResult.ErrorCode); StringAssert.Contains(refreshResult.ErrorMessage, "offline mode"); Assert.IsNull(refreshResult.ErrorException); @@ -1680,6 +1751,7 @@ public async Task OfflineMode_OnlineToOfflineTransition(string pollingMode) Assert.AreEqual(etag1, ParseETagAsInt32((await configService.GetConfigAsync()).HttpETag)); Assert.IsFalse(refreshResult.IsSuccess); + Assert.AreEqual(RefreshErrorCode.OfflineClient, refreshResult.ErrorCode); StringAssert.Contains(refreshResult.ErrorMessage, "offline mode"); Assert.IsNull(refreshResult.ErrorException); } @@ -1696,12 +1768,14 @@ public async Task Hooks_MockedClientRaisesEvents() var configJsonFilePath = Path.Combine("data", "sample_variationid_v5.json"); var clientReadyEventCount = 0; + var configFetchedEvents = new List(); var configChangedEvents = new List(); var flagEvaluatedEvents = new List(); var errorEvents = new List(); var hooks = new Hooks(); hooks.ClientReady += (s, e) => clientReadyEventCount++; + hooks.ConfigFetched += (s, e) => configFetchedEvents.Add(e); hooks.ConfigChanged += (s, e) => configChangedEvents.Add(e); hooks.FlagEvaluated += (s, e) => flagEvaluatedEvents.Add(e); hooks.Error += (s, e) => errorEvents.Add(e); @@ -1713,7 +1787,7 @@ public async Task Hooks_MockedClientRaisesEvents() var onFetch = (ProjectConfig latestConfig, CancellationToken _) => { var logMessage = loggerWrapper.FetchFailedDueToUnexpectedError(errorException); - return FetchResult.Failure(latestConfig, errorMessage: logMessage.InvariantFormattedMessage, errorException: errorException); + return FetchResult.Failure(latestConfig, RefreshErrorCode.HttpRequestFailure, errorMessage: logMessage.InvariantFormattedMessage, errorException: errorException); }; this.fetcherMock.Setup(m => m.FetchAsync(It.IsAny(), It.IsAny())).ReturnsAsync(onFetch); @@ -1727,6 +1801,7 @@ public async Task Hooks_MockedClientRaisesEvents() var client = new ConfigCatClient(configService, this.loggerMock.Object, new RolloutEvaluator(loggerWrapper), hooks); Assert.AreEqual(1, clientReadyEventCount); + Assert.AreEqual(0, configFetchedEvents.Count); Assert.AreEqual(0, configChangedEvents.Count); Assert.AreEqual(0, flagEvaluatedEvents.Count); Assert.AreEqual(0, errorEvents.Count); @@ -1734,6 +1809,10 @@ public async Task Hooks_MockedClientRaisesEvents() // 2. Fetch fails await client.ForceRefreshAsync(); + Assert.AreEqual(1, configFetchedEvents.Count); + Assert.IsTrue(configFetchedEvents[0].IsInitiatedByUser); + Assert.IsFalse(configFetchedEvents[0].Result.IsSuccess); + Assert.AreEqual(RefreshErrorCode.HttpRequestFailure, configFetchedEvents[0].Result.ErrorCode); Assert.AreEqual(0, configChangedEvents.Count); Assert.AreEqual(1, errorEvents.Count); Assert.IsNotNull(errorEvents[0].Message); @@ -1748,6 +1827,10 @@ public async Task Hooks_MockedClientRaisesEvents() await client.ForceRefreshAsync(); + Assert.AreEqual(2, configFetchedEvents.Count); + Assert.IsTrue(configFetchedEvents[1].IsInitiatedByUser); + Assert.IsTrue(configFetchedEvents[1].Result.IsSuccess); + Assert.AreEqual(RefreshErrorCode.None, configFetchedEvents[1].Result.ErrorCode); Assert.AreEqual(1, configChangedEvents.Count); Assert.AreSame(config.Config, configChangedEvents[0].NewConfig); @@ -1766,6 +1849,7 @@ public async Task Hooks_MockedClientRaisesEvents() client.Dispose(); Assert.AreEqual(1, clientReadyEventCount); + Assert.AreEqual(2, configFetchedEvents.Count); Assert.AreEqual(1, configChangedEvents.Count); Assert.AreEqual(evaluationDetails.Count, flagEvaluatedEvents.Count); Assert.AreEqual(1, errorEvents.Count); @@ -1778,11 +1862,13 @@ public async Task Hooks_MockedClientRaisesEvents() public async Task Hooks_RealClientRaisesEvents(bool subscribeViaOptions) { var clientReadyCallCount = 0; + var configFetchedEvents = new List(); var configChangedEvents = new List(); var flagEvaluatedEvents = new List(); var errorEvents = new List(); EventHandler handleClientReady = (s, e) => clientReadyCallCount++; + EventHandler handleConfigFetched = (s, e) => configFetchedEvents.Add(e); EventHandler handleConfigChanged = (s, e) => configChangedEvents.Add(e); EventHandler handleFlagEvaluated = (s, e) => flagEvaluatedEvents.Add(e); EventHandler handleError = (s, e) => errorEvents.Add(e); @@ -1790,6 +1876,7 @@ public async Task Hooks_RealClientRaisesEvents(bool subscribeViaOptions) void Subscribe(IProvidesHooks hooks) { hooks.ClientReady += handleClientReady; + hooks.ConfigFetched += handleConfigFetched; hooks.ConfigChanged += handleConfigChanged; hooks.FlagEvaluated += handleFlagEvaluated; hooks.Error += handleError; @@ -1798,6 +1885,7 @@ void Subscribe(IProvidesHooks hooks) void Unsubscribe(IProvidesHooks hooks) { hooks.ClientReady -= handleClientReady; + hooks.ConfigFetched -= handleConfigFetched; hooks.ConfigChanged -= handleConfigChanged; hooks.FlagEvaluated -= handleFlagEvaluated; hooks.Error -= handleError; @@ -1826,6 +1914,7 @@ void Unsubscribe(IProvidesHooks hooks) } Assert.AreEqual(subscribeViaOptions ? 2 : 0, clientReadyCallCount); + Assert.AreEqual(0, configFetchedEvents.Count); Assert.AreEqual(0, configChangedEvents.Count); Assert.AreEqual(0, flagEvaluatedEvents.Count); Assert.AreEqual(0, errorEvents.Count); @@ -1833,6 +1922,11 @@ void Unsubscribe(IProvidesHooks hooks) // 2. Fetch succeeds await client.ForceRefreshAsync(); + Assert.AreEqual(2, configFetchedEvents.Count); + Assert.AreSame(configFetchedEvents[0], configFetchedEvents[1]); + Assert.IsTrue(configFetchedEvents[0].IsInitiatedByUser); + Assert.IsTrue(configFetchedEvents[0].Result.IsSuccess); + Assert.AreEqual(RefreshErrorCode.None, configFetchedEvents[1].Result.ErrorCode); Assert.AreEqual(2, configChangedEvents.Count); Assert.IsTrue(configChangedEvents[0].NewConfig.Settings.Any()); Assert.AreSame(configChangedEvents[0], configChangedEvents[1]); @@ -1873,6 +1967,7 @@ void Unsubscribe(IProvidesHooks hooks) client.Dispose(); Assert.AreEqual(subscribeViaOptions ? 2 : 0, clientReadyCallCount); + Assert.AreEqual(2, configFetchedEvents.Count); Assert.AreEqual(2, configChangedEvents.Count); Assert.AreEqual(evaluationDetails.Count * 2, flagEvaluatedEvents.Count); Assert.AreEqual(2, errorEvents.Count); diff --git a/src/ConfigCat.Client.Tests/ConfigServiceTests.cs b/src/ConfigCat.Client.Tests/ConfigServiceTests.cs index 693cdc9b..0b545cd6 100644 --- a/src/ConfigCat.Client.Tests/ConfigServiceTests.cs +++ b/src/ConfigCat.Client.Tests/ConfigServiceTests.cs @@ -168,6 +168,9 @@ public async Task LazyLoadConfigService_RefreshConfigAsync_ConfigChanged_ShouldR var configChangedEvents = new ConcurrentQueue(); hooks.ConfigChanged += (s, e) => configChangedEvents.Enqueue(e); + var configFetchedEvents = new ConcurrentQueue(); + hooks.ConfigFetched += (s, e) => configFetchedEvents.Enqueue(e); + this.cacheMock .Setup(m => m.GetAsync(It.IsAny(), It.IsAny())) .ReturnsAsync(cachedPc); @@ -196,6 +199,12 @@ public async Task LazyLoadConfigService_RefreshConfigAsync_ConfigChanged_ShouldR Assert.IsTrue(configChangedEvents.TryDequeue(out var configChangedEvent)); Assert.AreSame(fetchedPc.Config, configChangedEvent.NewConfig); Assert.AreEqual(0, configChangedEvents.Count); + + Assert.AreEqual(1, configFetchedEvents.Count); + Assert.IsTrue(configFetchedEvents.TryDequeue(out var configFetchedEvent)); + Assert.IsTrue(configFetchedEvent.IsInitiatedByUser); + Assert.IsTrue(configFetchedEvent.Result.IsSuccess); + Assert.AreEqual(RefreshErrorCode.None, configFetchedEvent.Result.ErrorCode); } [TestMethod] @@ -416,6 +425,9 @@ public async Task ManualPollConfigService_GetConfigAsync_ShouldInvokeCacheGet() var clientReadyEventCount = 0; hooks.ClientReady += (s, e) => Interlocked.Increment(ref clientReadyEventCount); + var configFetchedEventCount = 0; + hooks.ConfigFetched += (s, e) => Interlocked.Increment(ref configFetchedEventCount); + this.cacheMock .Setup(m => m.GetAsync(It.IsAny(), It.IsAny())) .ReturnsAsync(cachedPc); @@ -439,6 +451,8 @@ public async Task ManualPollConfigService_GetConfigAsync_ShouldInvokeCacheGet() this.cacheMock.Verify(m => m.SetAsync(It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); Assert.AreEqual(1, Volatile.Read(ref clientReadyEventCount)); + + Assert.AreEqual(0, Volatile.Read(ref configFetchedEventCount)); } [TestMethod] @@ -455,6 +469,9 @@ public async Task ManualPollConfigService_RefreshConfigAsync_ShouldInvokeCacheGe var clientReadyEventCount = 0; hooks.ClientReady += (s, e) => Interlocked.Increment(ref clientReadyEventCount); + var configFetchedEvents = new ConcurrentQueue(); + hooks.ConfigFetched += (s, e) => configFetchedEvents.Enqueue(e); + byte callOrder = 1; this.cacheMock @@ -488,6 +505,12 @@ public async Task ManualPollConfigService_RefreshConfigAsync_ShouldInvokeCacheGe this.cacheMock.Verify(m => m.SetAsync(It.IsAny(), It.IsAny(), It.IsAny()), Times.Once); Assert.AreEqual(1, Volatile.Read(ref clientReadyEventCount)); + + Assert.AreEqual(1, configFetchedEvents.Count); + Assert.IsTrue(configFetchedEvents.TryDequeue(out var configFetchedEvent)); + Assert.IsTrue(configFetchedEvent.IsInitiatedByUser); + Assert.IsTrue(configFetchedEvent.Result.IsSuccess); + Assert.AreEqual(RefreshErrorCode.None, configFetchedEvent.Result.ErrorCode); } [TestMethod] @@ -504,6 +527,9 @@ public async Task ManualPollConfigService_RefreshConfigAsync_ConfigChanged_Shoul var configChangedEvents = new ConcurrentQueue(); hooks.ConfigChanged += (s, e) => configChangedEvents.Enqueue(e); + var configFetchedEvents = new ConcurrentQueue(); + hooks.ConfigFetched += (s, e) => configFetchedEvents.Enqueue(e); + this.cacheMock .Setup(m => m.GetAsync(It.IsAny(), It.IsAny())) .ReturnsAsync(cachedPc); @@ -531,6 +557,12 @@ public async Task ManualPollConfigService_RefreshConfigAsync_ConfigChanged_Shoul Assert.IsTrue(configChangedEvents.TryDequeue(out var configChangedEvent)); Assert.AreSame(fetchedPc.Config, configChangedEvent.NewConfig); Assert.AreEqual(0, configChangedEvents.Count); + + Assert.AreEqual(1, configFetchedEvents.Count); + Assert.IsTrue(configFetchedEvents.TryDequeue(out var configFetchedEvent)); + Assert.IsTrue(configFetchedEvent.IsInitiatedByUser); + Assert.IsTrue(configFetchedEvent.Result.IsSuccess); + Assert.AreEqual(RefreshErrorCode.None, configFetchedEvent.Result.ErrorCode); } [TestMethod] @@ -624,6 +656,9 @@ public async Task AutoPollConfigService_GetConfig_ReturnsCachedConfigWhenCachedC var clientReadyTcs = new TaskCompletionSource(); hooks.ClientReady += (s, e) => clientReadyTcs.TrySetResult(default); + var configFetchedEventCount = 0; + hooks.ConfigFetched += (s, e) => Interlocked.Increment(ref configFetchedEventCount); + var cache = new InMemoryConfigCache(); cache.Set(null!, cachedPc); @@ -667,6 +702,8 @@ public async Task AutoPollConfigService_GetConfig_ReturnsCachedConfigWhenCachedC { Assert.IsTrue(clientReadyCalled); } + + Assert.AreEqual(0, Volatile.Read(ref configFetchedEventCount)); } [DataRow(false)] @@ -687,6 +724,9 @@ public async Task AutoPollConfigService_GetConfig_FetchesConfigWhenCachedConfigI var clientReadyTcs = new TaskCompletionSource(); hooks.ClientReady += (s, e) => clientReadyTcs.TrySetResult(default); + var configFetchedEvents = new ConcurrentQueue(); + hooks.ConfigFetched += (s, e) => configFetchedEvents.Enqueue(e); + var cache = new InMemoryConfigCache(); cache.Set(null!, cachedPc); @@ -710,9 +750,13 @@ public async Task AutoPollConfigService_GetConfig_FetchesConfigWhenCachedConfigI // Allow some time for other initalization callbacks to execute. using var cts = new CancellationTokenSource(); - var task = await Task.WhenAny(clientReadyTcs.Task, Task.Delay(maxInitWaitTime, cts.Token)); + var clientReadyTask = Task.Run(async () => await clientReadyTcs.Task); + var task = await Task.WhenAny(clientReadyTask, Task.Delay(maxInitWaitTime, cts.Token)); cts.Cancel(); - clientReadyCalled = task == clientReadyTcs.Task && task.Status == TaskStatus.RanToCompletion; + clientReadyCalled = task == clientReadyTask && task.Status == TaskStatus.RanToCompletion; + + // Wait for the hook event handlers to execute (as that might not happen if the service got disposed immediately). + SpinWait.SpinUntil(() => configFetchedEvents.TryPeek(out _), TimeSpan.FromSeconds(1)); } // Assert @@ -722,6 +766,12 @@ public async Task AutoPollConfigService_GetConfig_FetchesConfigWhenCachedConfigI this.fetcherMock.Verify(m => m.FetchAsync(cachedPc, It.IsAny()), Times.Once); Assert.IsTrue(clientReadyCalled); + + Assert.AreEqual(1, configFetchedEvents.Count); + Assert.IsTrue(configFetchedEvents.TryDequeue(out var configFetchedEvent)); + Assert.IsFalse(configFetchedEvent.IsInitiatedByUser); + Assert.IsTrue(configFetchedEvent.Result.IsSuccess); + Assert.AreEqual(RefreshErrorCode.None, configFetchedEvent.Result.ErrorCode); } [DataRow(false, false, true)] @@ -746,11 +796,14 @@ public async Task AutoPollConfigService_GetConfig_ReturnsExpiredConfigWhenCantRe var clientReadyTcs = new TaskCompletionSource(); hooks.ClientReady += (s, e) => clientReadyTcs.TrySetResult(default); + var configFetchedEvents = new ConcurrentQueue(); + hooks.ConfigFetched += (s, e) => configFetchedEvents.Enqueue(e); + var cache = new InMemoryConfigCache(); cache.Set(null!, cachedPc); this.fetcherMock.Setup(m => m.FetchAsync(cachedPc, It.IsAny())).ReturnsAsync( - failure ? FetchResult.Failure(fetchedPc, "network error") : FetchResult.NotModified(fetchedPc)); + failure ? FetchResult.Failure(fetchedPc, RefreshErrorCode.HttpRequestFailure, "network error") : FetchResult.NotModified(fetchedPc)); var config = PollingModes.AutoPoll(pollInterval, maxInitWaitTime); var service = new AutoPollConfigService(config, @@ -770,9 +823,13 @@ public async Task AutoPollConfigService_GetConfig_ReturnsExpiredConfigWhenCantRe // Allow some time for other initalization callbacks to execute. using var cts = new CancellationTokenSource(); - var task = await Task.WhenAny(clientReadyTcs.Task, Task.Delay(maxInitWaitTime, cts.Token)); + var clientReadyTask = Task.Run(async () => await clientReadyTcs.Task); + var task = await Task.WhenAny(clientReadyTask, Task.Delay(maxInitWaitTime, cts.Token)); cts.Cancel(); - clientReadyCalled = task == clientReadyTcs.Task && task.Status == TaskStatus.RanToCompletion; + clientReadyCalled = task == clientReadyTask && task.Status == TaskStatus.RanToCompletion; + + // Wait for the hook event handlers to execute (as that might not happen if the service got disposed immediately). + SpinWait.SpinUntil(() => configFetchedEvents.TryPeek(out _), TimeSpan.FromSeconds(1)); } // Assert @@ -782,6 +839,12 @@ public async Task AutoPollConfigService_GetConfig_ReturnsExpiredConfigWhenCantRe this.fetcherMock.Verify(m => m.FetchAsync(cachedPc, It.IsAny()), Times.Once); Assert.IsTrue(clientReadyCalled); + + Assert.IsTrue(configFetchedEvents.Count > 0); + Assert.IsTrue(configFetchedEvents.TryDequeue(out var configFetchedEvent)); + Assert.IsFalse(configFetchedEvent.IsInitiatedByUser); + Assert.AreEqual(failure, !configFetchedEvent.Result.IsSuccess); + Assert.AreEqual(failure ? RefreshErrorCode.HttpRequestFailure : RefreshErrorCode.None, configFetchedEvent.Result.ErrorCode); } [DataRow(false)] @@ -801,6 +864,9 @@ public async Task LazyLoadConfigService_GetConfig_ReturnsCachedConfigWhenCachedC var clientReadyEventCount = 0; hooks.ClientReady += (s, e) => Interlocked.Increment(ref clientReadyEventCount); + var configFetchedEventCount = 0; + hooks.ConfigFetched += (s, e) => Interlocked.Increment(ref configFetchedEventCount); + var cache = new InMemoryConfigCache(); cache.Set(null!, cachedPc); @@ -841,6 +907,8 @@ public async Task LazyLoadConfigService_GetConfig_ReturnsCachedConfigWhenCachedC } Assert.AreEqual(1, Volatile.Read(ref clientReadyEventCount)); + + Assert.AreEqual(0, Volatile.Read(ref configFetchedEventCount)); } [DataRow(false)] @@ -860,6 +928,9 @@ public async Task LazyLoadConfigService_GetConfig_FetchesConfigWhenCachedConfigI var clientReadyEventCount = 0; hooks.ClientReady += (s, e) => Interlocked.Increment(ref clientReadyEventCount); + var configFetchedEvents = new ConcurrentQueue(); + hooks.ConfigFetched += (s, e) => configFetchedEvents.Enqueue(e); + var cache = new InMemoryConfigCache(); cache.Set(null!, cachedPc); @@ -900,5 +971,11 @@ public async Task LazyLoadConfigService_GetConfig_FetchesConfigWhenCachedConfigI } Assert.AreEqual(1, Volatile.Read(ref clientReadyEventCount)); + + Assert.IsTrue(configFetchedEvents.Count > 0); + Assert.IsTrue(configFetchedEvents.TryDequeue(out var configFetchedEvent)); + Assert.IsFalse(configFetchedEvent.IsInitiatedByUser); + Assert.IsTrue(configFetchedEvent.Result.IsSuccess); + Assert.AreEqual(RefreshErrorCode.None, configFetchedEvent.Result.ErrorCode); } } diff --git a/src/ConfigCat.Client.Tests/ConfigV2EvaluationTests.cs b/src/ConfigCat.Client.Tests/ConfigV2EvaluationTests.cs index 272c1d34..1a25aa45 100644 --- a/src/ConfigCat.Client.Tests/ConfigV2EvaluationTests.cs +++ b/src/ConfigCat.Client.Tests/ConfigV2EvaluationTests.cs @@ -233,7 +233,7 @@ public void PrerequisiteFlagCircularDependencyTest(string key, string dependency var logger = new Mock().Object.AsWrapper(); var evaluator = new RolloutEvaluator(logger); - var ex = Assert.ThrowsException(() => evaluator.Evaluate(config!.Settings, key, defaultValue: null, user: null, remoteConfig: null, logger)); + var ex = Assert.ThrowsException(() => evaluator.Evaluate(config!.Settings, key, defaultValue: null, user: null, remoteConfig: null, logger)); StringAssert.Contains(ex.Message, "Circular dependency detected"); StringAssert.Contains(ex.Message, dependencyCycle); diff --git a/src/ConfigCat.Client.Tests/EvaluationTestsBase.cs b/src/ConfigCat.Client.Tests/EvaluationTestsBase.cs index ee67108b..da9db788 100644 --- a/src/ConfigCat.Client.Tests/EvaluationTestsBase.cs +++ b/src/ConfigCat.Client.Tests/EvaluationTestsBase.cs @@ -112,7 +112,7 @@ public void GetValue_WithIncompatibleDefaultValueType_ShouldThrowWithImprovedErr this.logger, }; - var ex = Assert.ThrowsException(() => + var ex = Assert.ThrowsException(() => { try { EvaluateMethodDefinition.MakeGenericMethod(settingClrType).Invoke(null, args); } catch (TargetInvocationException ex) { throw ex.InnerException!; } diff --git a/src/ConfigCat.Client.Tests/HttpConfigFetcherTests.cs b/src/ConfigCat.Client.Tests/HttpConfigFetcherTests.cs index d303947b..984b7344 100644 --- a/src/ConfigCat.Client.Tests/HttpConfigFetcherTests.cs +++ b/src/ConfigCat.Client.Tests/HttpConfigFetcherTests.cs @@ -50,7 +50,7 @@ public void HttpConfigFetcher_WithCustomHttpClientHandler_HandlersDisposeShouldN } [TestMethod] - public async Task HttpConfigFetcher_ResponseHttpCodeIsUnexpected_ShouldReturnsPassedConfig() + public async Task HttpConfigFetcher_ResponseHttpCodeIsUnexpected_ShouldReturnPassedConfig() { // Arrange @@ -68,6 +68,7 @@ public async Task HttpConfigFetcher_ResponseHttpCodeIsUnexpected_ShouldReturnsPa // Assert Assert.IsTrue(actual.IsFailure); + Assert.AreEqual(RefreshErrorCode.InvalidSdkKey, actual.ErrorCode); Assert.IsNotNull(actual.ErrorMessage); Assert.IsNull(actual.ErrorException); Assert.AreNotSame(lastConfig, actual.Config); @@ -96,6 +97,7 @@ public async Task HttpConfigFetcher_ThrowAnException_ShouldReturnPassedConfig() // Assert Assert.IsTrue(actual.IsFailure); + Assert.AreEqual(RefreshErrorCode.HttpRequestFailure, actual.ErrorCode); Assert.IsNotNull(actual.ErrorMessage); Assert.AreSame(exception, actual.ErrorException); Assert.AreEqual(lastConfig, actual.Config); diff --git a/src/ConfigCat.Client.Tests/OverrideTests.cs b/src/ConfigCat.Client.Tests/OverrideTests.cs index 1ba1f9c1..0dbd4369 100644 --- a/src/ConfigCat.Client.Tests/OverrideTests.cs +++ b/src/ConfigCat.Client.Tests/OverrideTests.cs @@ -4,8 +4,8 @@ using System.IO; using System.Linq; using System.Threading.Tasks; +using ConfigCat.Client.Evaluation; using Microsoft.VisualStudio.TestTools.UnitTesting; -using Moq; #if USE_NEWTONSOFT_JSON using JsonValue = Newtonsoft.Json.Linq.JValue; @@ -266,6 +266,7 @@ public void LocalOnly() Assert.IsTrue(client.GetValue("nonexisting", false)); Assert.IsFalse(refreshResult.IsSuccess); + Assert.AreEqual(RefreshErrorCode.LocalOnlyClient, refreshResult.ErrorCode); StringAssert.Contains(refreshResult.ErrorMessage, nameof(OverrideBehaviour.LocalOnly)); Assert.IsNull(refreshResult.ErrorException); } @@ -333,6 +334,7 @@ public async Task LocalOnly_Async() Assert.IsTrue(client.GetValue("nonexisting", false)); Assert.IsFalse(refreshResult.IsSuccess); + Assert.AreEqual(RefreshErrorCode.LocalOnlyClient, refreshResult.ErrorCode); StringAssert.Contains(refreshResult.ErrorMessage, nameof(OverrideBehaviour.LocalOnly)); Assert.IsNull(refreshResult.ErrorException); } @@ -576,14 +578,37 @@ public void OverrideValueTypeMismatchShouldBeHandledCorrectly_Dictionary(object options.FlagOverrides = FlagOverrides.LocalDictionary(dictionary, OverrideBehaviour.LocalOnly); }); - var method = typeof(IConfigCatClient).GetMethod(nameof(IConfigCatClient.GetValue))! + var method = typeof(IConfigCatClient).GetMethod(nameof(IConfigCatClient.GetValueDetails))! .GetGenericMethodDefinition() .MakeGenericMethod(defaultValue.GetType()); - var actualEvaluatedValue = method.Invoke(client, new[] { key, defaultValue, null }); + var actualEvaluatedValueDetails = (EvaluationDetails)method.Invoke(client, new[] { key, defaultValue, null })!; + var actualEvaluatedValue = actualEvaluatedValueDetails.Value; var actualEvaluatedValues = client.GetAllValues(user: null); Assert.AreEqual(expectedEvaluatedValue, actualEvaluatedValue); + if (!defaultValue.Equals(expectedEvaluatedValue)) + { + Assert.IsFalse(actualEvaluatedValueDetails.IsDefaultValue); + Assert.AreEqual(EvaluationErrorCode.None, actualEvaluatedValueDetails.ErrorCode); + Assert.IsNull(actualEvaluatedValueDetails.ErrorMessage); + Assert.IsNull(actualEvaluatedValueDetails.ErrorException); + } + else + { + Assert.IsTrue(actualEvaluatedValueDetails.IsDefaultValue); + Assert.IsNotNull(actualEvaluatedValueDetails.ErrorMessage); + if (overrideValue.ToSettingValue(out _).HasUnsupportedValue) + { + Assert.AreEqual(EvaluationErrorCode.InvalidConfigModel, actualEvaluatedValueDetails.ErrorCode); + Assert.IsInstanceOfType(actualEvaluatedValueDetails.ErrorException, typeof(InvalidConfigModelException)); + } + else + { + Assert.AreEqual(EvaluationErrorCode.SettingValueTypeMismatch, actualEvaluatedValueDetails.ErrorCode); + Assert.IsInstanceOfType(actualEvaluatedValueDetails.ErrorException, typeof(EvaluationErrorException)); + } + } overrideValue.ToSettingValue(out var overrideValueSettingType); var expectedEvaluatedValues = new KeyValuePair[] @@ -638,14 +663,41 @@ public void OverrideValueTypeMismatchShouldBeHandledCorrectly_SimplifiedConfig(s options.FlagOverrides = FlagOverrides.LocalFile(filePath, autoReload: false, OverrideBehaviour.LocalOnly); }); - var method = typeof(IConfigCatClient).GetMethod(nameof(IConfigCatClient.GetValue))! + var method = typeof(IConfigCatClient).GetMethod(nameof(IConfigCatClient.GetValueDetails))! .GetGenericMethodDefinition() .MakeGenericMethod(defaultValue.GetType()); - var actualEvaluatedValue = method.Invoke(client, new[] { key, defaultValue, null }); + var actualEvaluatedValueDetails = (EvaluationDetails)method.Invoke(client, new[] { key, defaultValue, null })!; + var actualEvaluatedValue = actualEvaluatedValueDetails.Value; var actualEvaluatedValues = client.GetAllValues(user: null); Assert.AreEqual(expectedEvaluatedValue, actualEvaluatedValue); + if (!defaultValue.Equals(expectedEvaluatedValue)) + { + Assert.IsFalse(actualEvaluatedValueDetails.IsDefaultValue); + Assert.AreEqual(EvaluationErrorCode.None, actualEvaluatedValueDetails.ErrorCode); + Assert.IsNull(actualEvaluatedValueDetails.ErrorMessage); + Assert.IsNull(actualEvaluatedValueDetails.ErrorException); + } + else + { + Assert.IsTrue(actualEvaluatedValueDetails.IsDefaultValue); + Assert.IsNotNull(actualEvaluatedValueDetails.ErrorMessage); +#if USE_NEWTONSOFT_JSON + if (overrideValue is not JsonValue overrideJsonValue || overrideJsonValue.ToSettingValue(out _).HasUnsupportedValue) +#else + if (overrideValue.ToSettingValue(out _).HasUnsupportedValue) +#endif + { + Assert.AreEqual(EvaluationErrorCode.InvalidConfigModel, actualEvaluatedValueDetails.ErrorCode); + Assert.IsInstanceOfType(actualEvaluatedValueDetails.ErrorException, typeof(InvalidConfigModelException)); + } + else + { + Assert.AreEqual(EvaluationErrorCode.SettingValueTypeMismatch, actualEvaluatedValueDetails.ErrorCode); + Assert.IsInstanceOfType(actualEvaluatedValueDetails.ErrorException, typeof(EvaluationErrorException)); + } + } var unwrappedOverrideValue = overrideValue is JsonValue jsonValue ? jsonValue.ToSettingValue(out var overrideValueSettingType) diff --git a/src/ConfigCatClient/ConfigCatClient.cs b/src/ConfigCatClient/ConfigCatClient.cs index 0974ebc0..d27563c0 100644 --- a/src/ConfigCatClient/ConfigCatClient.cs +++ b/src/ConfigCatClient/ConfigCatClient.cs @@ -282,7 +282,8 @@ public T GetValue(string key, T defaultValue, User? user = null) catch (Exception ex) { this.logger.SettingEvaluationError(nameof(GetValue), key, nameof(defaultValue), defaultValue, ex); - evaluationDetails = EvaluationDetails.FromDefaultValue(key, defaultValue, fetchTime: settings.RemoteConfig?.TimeStamp, user, ex.Message, ex); + evaluationDetails = EvaluationDetails.FromDefaultValue(key, defaultValue, fetchTime: settings.RemoteConfig?.TimeStamp, user, + ex.Message, ex, RolloutEvaluatorExtensions.GetErrorCode(ex)); value = defaultValue; } @@ -322,7 +323,8 @@ public async Task GetValueAsync(string key, T defaultValue, User? user = n catch (Exception ex) { this.logger.SettingEvaluationError(nameof(GetValueAsync), key, nameof(defaultValue), defaultValue, ex); - evaluationDetails = EvaluationDetails.FromDefaultValue(key, defaultValue, fetchTime: settings.RemoteConfig?.TimeStamp, user, ex.Message, ex); + evaluationDetails = EvaluationDetails.FromDefaultValue(key, defaultValue, fetchTime: settings.RemoteConfig?.TimeStamp, user, + ex.Message, ex, RolloutEvaluatorExtensions.GetErrorCode(ex)); value = defaultValue; } @@ -356,7 +358,8 @@ public EvaluationDetails GetValueDetails(string key, T defaultValue, User? catch (Exception ex) { this.logger.SettingEvaluationError(nameof(GetValueDetails), key, nameof(defaultValue), defaultValue, ex); - evaluationDetails = EvaluationDetails.FromDefaultValue(key, defaultValue, fetchTime: settings.RemoteConfig?.TimeStamp, user, ex.Message, ex); + evaluationDetails = EvaluationDetails.FromDefaultValue(key, defaultValue, fetchTime: settings.RemoteConfig?.TimeStamp, user, + ex.Message, ex, RolloutEvaluatorExtensions.GetErrorCode(ex)); } this.hooks.RaiseFlagEvaluated(evaluationDetails); @@ -393,7 +396,8 @@ public async Task> GetValueDetailsAsync(string key, T de catch (Exception ex) { this.logger.SettingEvaluationError(nameof(GetValueDetailsAsync), key, nameof(defaultValue), defaultValue, ex); - evaluationDetails = EvaluationDetails.FromDefaultValue(key, defaultValue, fetchTime: settings.RemoteConfig?.TimeStamp, user, ex.Message, ex); + evaluationDetails = EvaluationDetails.FromDefaultValue(key, defaultValue, fetchTime: settings.RemoteConfig?.TimeStamp, user, + ex.Message, ex, RolloutEvaluatorExtensions.GetErrorCode(ex)); } this.hooks.RaiseFlagEvaluated(evaluationDetails); @@ -590,7 +594,7 @@ public RefreshResult ForceRefresh() catch (Exception ex) { this.logger.ForceRefreshError(nameof(ForceRefresh), ex); - return RefreshResult.Failure(ex.Message, ex); + return RefreshResult.Failure(RefreshErrorCode.UnexpectedError, ex.Message, ex); } } @@ -608,7 +612,7 @@ public async Task ForceRefreshAsync(CancellationToken cancellatio catch (Exception ex) { this.logger.ForceRefreshError(nameof(ForceRefreshAsync), ex); - return RefreshResult.Failure(ex.Message, ex); + return RefreshResult.Failure(RefreshErrorCode.UnexpectedError, ex.Message, ex); } } @@ -740,6 +744,13 @@ public event EventHandler? FlagEvaluated remove { this.hooks.FlagEvaluated -= value; } } + /// + public event EventHandler? ConfigFetched + { + add { this.hooks.ConfigFetched += value; } + remove { this.hooks.ConfigFetched -= value; } + } + /// public event EventHandler? ConfigChanged { diff --git a/src/ConfigCatClient/ConfigService/AutoPollConfigService.cs b/src/ConfigCatClient/ConfigService/AutoPollConfigService.cs index 899fe22e..0214a3f6 100644 --- a/src/ConfigCatClient/ConfigService/AutoPollConfigService.cs +++ b/src/ConfigCatClient/ConfigService/AutoPollConfigService.cs @@ -146,10 +146,10 @@ public async ValueTask GetConfigAsync(CancellationToken cancellat return await this.ConfigCache.GetAsync(base.CacheKey, cancellationToken).ConfigureAwait(false); } - protected override void OnConfigFetched(ProjectConfig newConfig) + protected override void OnConfigFetched(in FetchResult fetchResult, bool isInitiatedByUser) { - base.OnConfigFetched(newConfig); SignalInitialization(); + base.OnConfigFetched(fetchResult, isInitiatedByUser); } protected override void SetOnlineCoreSynchronized() @@ -214,7 +214,7 @@ private async ValueTask PollCoreAsync(bool isFirstIteration, CancellationToken c { if (!IsOffline) { - await RefreshConfigCoreAsync(latestConfig, cancellationToken).ConfigureAwait(false); + await RefreshConfigCoreAsync(latestConfig, isInitiatedByUser: false, cancellationToken).ConfigureAwait(false); } } else @@ -227,7 +227,7 @@ private async ValueTask PollCoreAsync(bool isFirstIteration, CancellationToken c if (!IsOffline) { var latestConfig = await this.ConfigCache.GetAsync(base.CacheKey, cancellationToken).ConfigureAwait(false); - await RefreshConfigCoreAsync(latestConfig, cancellationToken).ConfigureAwait(false); + await RefreshConfigCoreAsync(latestConfig, isInitiatedByUser: false, cancellationToken).ConfigureAwait(false); } } } diff --git a/src/ConfigCatClient/ConfigService/ConfigServiceBase.cs b/src/ConfigCatClient/ConfigService/ConfigServiceBase.cs index 0aa90926..7afb130b 100644 --- a/src/ConfigCatClient/ConfigService/ConfigServiceBase.cs +++ b/src/ConfigCatClient/ConfigService/ConfigServiceBase.cs @@ -74,17 +74,17 @@ public virtual RefreshResult RefreshConfig() if (!IsOffline) { var latestConfig = this.ConfigCache.Get(this.CacheKey); - var configWithFetchResult = RefreshConfigCore(latestConfig); + var configWithFetchResult = RefreshConfigCore(latestConfig, isInitiatedByUser: true); return RefreshResult.From(configWithFetchResult.Item2); } else { var logMessage = this.Logger.ConfigServiceCannotInitiateHttpCalls(); - return RefreshResult.Failure(logMessage.InvariantFormattedMessage); + return RefreshResult.Failure(RefreshErrorCode.OfflineClient, logMessage.InvariantFormattedMessage); } } - protected ConfigWithFetchResult RefreshConfigCore(ProjectConfig latestConfig) + protected ConfigWithFetchResult RefreshConfigCore(ProjectConfig latestConfig, bool isInitiatedByUser) { var fetchResult = this.ConfigFetcher.Fetch(latestConfig); @@ -96,11 +96,11 @@ protected ConfigWithFetchResult RefreshConfigCore(ProjectConfig latestConfig) latestConfig = fetchResult.Config; } - OnConfigFetched(fetchResult.Config); + OnConfigFetched(fetchResult, isInitiatedByUser); if (fetchResult.IsSuccess) { - OnConfigChanged(fetchResult.Config); + OnConfigChanged(fetchResult); } return new ConfigWithFetchResult(latestConfig, fetchResult); @@ -111,17 +111,17 @@ public virtual async ValueTask RefreshConfigAsync(CancellationTok if (!IsOffline) { var latestConfig = await this.ConfigCache.GetAsync(this.CacheKey, cancellationToken).ConfigureAwait(false); - var configWithFetchResult = await RefreshConfigCoreAsync(latestConfig, cancellationToken).ConfigureAwait(false); + var configWithFetchResult = await RefreshConfigCoreAsync(latestConfig, isInitiatedByUser: true, cancellationToken).ConfigureAwait(false); return RefreshResult.From(configWithFetchResult.Item2); } else { var logMessage = this.Logger.ConfigServiceCannotInitiateHttpCalls(); - return RefreshResult.Failure(logMessage.InvariantFormattedMessage); + return RefreshResult.Failure(RefreshErrorCode.OfflineClient, logMessage.InvariantFormattedMessage); } } - protected async Task RefreshConfigCoreAsync(ProjectConfig latestConfig, CancellationToken cancellationToken) + protected async Task RefreshConfigCoreAsync(ProjectConfig latestConfig, bool isInitiatedByUser, CancellationToken cancellationToken) { var fetchResult = await this.ConfigFetcher.FetchAsync(latestConfig, cancellationToken).ConfigureAwait(false); @@ -133,23 +133,26 @@ protected async Task RefreshConfigCoreAsync(ProjectConfig latestConfig = fetchResult.Config; } - OnConfigFetched(fetchResult.Config); + OnConfigFetched(fetchResult, isInitiatedByUser); if (fetchResult.IsSuccess) { - OnConfigChanged(fetchResult.Config); + OnConfigChanged(fetchResult); } return new ConfigWithFetchResult(latestConfig, fetchResult); } - protected virtual void OnConfigFetched(ProjectConfig newConfig) { } + protected virtual void OnConfigFetched(in FetchResult fetchResult, bool isInitiatedByUser) + { + this.Hooks.RaiseConfigFetched(RefreshResult.From(fetchResult), isInitiatedByUser); + } - protected virtual void OnConfigChanged(ProjectConfig newConfig) + protected virtual void OnConfigChanged(in FetchResult fetchResult) { this.Logger.Debug("config changed"); - this.Hooks.RaiseConfigChanged(newConfig.Config ?? new Config()); + this.Hooks.RaiseConfigChanged(fetchResult.Config.Config ?? new Config()); } public bool IsOffline diff --git a/src/ConfigCatClient/ConfigService/LazyLoadConfigService.cs b/src/ConfigCatClient/ConfigService/LazyLoadConfigService.cs index 2ea94583..b07fba05 100644 --- a/src/ConfigCatClient/ConfigService/LazyLoadConfigService.cs +++ b/src/ConfigCatClient/ConfigService/LazyLoadConfigService.cs @@ -30,7 +30,7 @@ public ProjectConfig GetConfig() if (!IsOffline) { - var configWithFetchResult = RefreshConfigCore(cachedConfig); + var configWithFetchResult = RefreshConfigCore(cachedConfig, isInitiatedByUser: false); return configWithFetchResult.Item1; } } @@ -51,7 +51,7 @@ public async ValueTask GetConfigAsync(CancellationToken cancellat if (!IsOffline) { - var configWithFetchResult = await RefreshConfigCoreAsync(cachedConfig, cancellationToken).ConfigureAwait(false); + var configWithFetchResult = await RefreshConfigCoreAsync(cachedConfig, isInitiatedByUser: false, cancellationToken).ConfigureAwait(false); return configWithFetchResult.Item1; } } diff --git a/src/ConfigCatClient/ConfigService/NullConfigService.cs b/src/ConfigCatClient/ConfigService/NullConfigService.cs index 97c83327..4ae232b0 100644 --- a/src/ConfigCatClient/ConfigService/NullConfigService.cs +++ b/src/ConfigCatClient/ConfigService/NullConfigService.cs @@ -18,7 +18,11 @@ public NullConfigService(LoggerWrapper logger, SafeHooksWrapper hooks = default) public ValueTask GetConfigAsync(CancellationToken cancellationToken = default) => new ValueTask(ProjectConfig.Empty); - public RefreshResult RefreshConfig() { return RefreshResult.Failure($"Client is configured to use the {nameof(OverrideBehaviour.LocalOnly)} override behavior, which prevents making HTTP requests."); } + public RefreshResult RefreshConfig() + { + return RefreshResult.Failure(RefreshErrorCode.LocalOnlyClient, + $"Client is configured to use the {nameof(OverrideBehaviour.LocalOnly)} override behavior, which prevents making HTTP requests."); + } public ValueTask RefreshConfigAsync(CancellationToken cancellationToken = default) => new ValueTask(RefreshConfig()); diff --git a/src/ConfigCatClient/Configuration/ConfigCatClientOptions.cs b/src/ConfigCatClient/Configuration/ConfigCatClientOptions.cs index 84471b1c..51b17edb 100644 --- a/src/ConfigCatClient/Configuration/ConfigCatClientOptions.cs +++ b/src/ConfigCatClient/Configuration/ConfigCatClientOptions.cs @@ -127,6 +127,13 @@ public event EventHandler? FlagEvaluated remove { this.hooks.FlagEvaluated -= value; } } + /// + public event EventHandler? ConfigFetched + { + add { this.hooks.ConfigFetched += value; } + remove { this.hooks.ConfigFetched -= value; } + } + /// public event EventHandler? ConfigChanged { diff --git a/src/ConfigCatClient/Evaluation/EvaluationDetails.cs b/src/ConfigCatClient/Evaluation/EvaluationDetails.cs index 4ee38bf0..cda4c0b3 100644 --- a/src/ConfigCatClient/Evaluation/EvaluationDetails.cs +++ b/src/ConfigCatClient/Evaluation/EvaluationDetails.cs @@ -28,14 +28,15 @@ internal static EvaluationDetails FromEvaluateResult(string key, } internal static EvaluationDetails FromDefaultValue(string key, TValue defaultValue, DateTime? fetchTime, User? user, - string? errorMessage = null, Exception? errorException = null) + string errorMessage, Exception? errorException = null, EvaluationErrorCode errorCode = EvaluationErrorCode.UnexpectedError) { var instance = new EvaluationDetails(key, defaultValue) { User = user, IsDefaultValue = true, ErrorMessage = errorMessage, - ErrorException = errorException + ErrorException = errorException, + ErrorCode = errorCode, }; if (fetchTime is not null) @@ -84,6 +85,11 @@ private protected EvaluationDetails(string key) /// public bool IsDefaultValue { get; set; } + /// + /// The code identifying the reason for the error in case evaluation failed. + /// + public EvaluationErrorCode ErrorCode { get; set; } + /// /// Error message in case evaluation failed. /// diff --git a/src/ConfigCatClient/Evaluation/EvaluationErrorCode.cs b/src/ConfigCatClient/Evaluation/EvaluationErrorCode.cs new file mode 100644 index 00000000..0e95cad0 --- /dev/null +++ b/src/ConfigCatClient/Evaluation/EvaluationErrorCode.cs @@ -0,0 +1,32 @@ +namespace ConfigCat.Client; + +/// +/// Specifies the possible evaluation error codes. +/// +public enum EvaluationErrorCode +{ + /// + /// An unexpected error occurred during the evaluation. + /// + UnexpectedError = -1, + /// + /// No error occurred (the evaluation was successful). + /// + None = 0, + /// + /// The evaluation failed because of an error in the config model. (Most likely, invalid data was passed to the SDK via flag overrides.) + /// + InvalidConfigModel = 1, + /// + /// The evaluation failed because of a type mismatch between the evaluated setting value and the specified default value. + /// + SettingValueTypeMismatch = 2, + /// + /// The evaluation failed because the config JSON was not available locally. + /// + ConfigJsonNotAvailable = 1000, + /// + /// The evaluation failed because the key of the evaluated setting was not found in the config JSON. + /// + SettingKeyMissing = 1001, +} diff --git a/src/ConfigCatClient/Evaluation/EvaluationErrorException.cs b/src/ConfigCatClient/Evaluation/EvaluationErrorException.cs new file mode 100644 index 00000000..30336b91 --- /dev/null +++ b/src/ConfigCatClient/Evaluation/EvaluationErrorException.cs @@ -0,0 +1,13 @@ +using System; + +namespace ConfigCat.Client.Evaluation; + +internal sealed class EvaluationErrorException : InvalidOperationException +{ + public EvaluationErrorException(EvaluationErrorCode errorCode, string message) : base(message) + { + ErrorCode = errorCode; + } + + public EvaluationErrorCode ErrorCode { get; } +} diff --git a/src/ConfigCatClient/Evaluation/RolloutEvaluator.cs b/src/ConfigCatClient/Evaluation/RolloutEvaluator.cs index 121c2f56..7e610e99 100644 --- a/src/ConfigCatClient/Evaluation/RolloutEvaluator.cs +++ b/src/ConfigCatClient/Evaluation/RolloutEvaluator.cs @@ -60,7 +60,7 @@ public EvaluateResult Evaluate(T defaultValue, ref EvaluateContext context, [ // The latter case is handled by SettingValue.GetValue below. if (context.Setting.SettingType != Setting.UnknownType && context.Setting.SettingType != expectedSettingType) { - throw new InvalidOperationException( + throw new EvaluationErrorException(EvaluationErrorCode.SettingValueTypeMismatch, "The type of a setting must match the type of the specified default value. " + $"Setting's type was {context.Setting.SettingType} but the default value's type was {typeof(T)}. " + $"Please use a default value which corresponds to the setting type {context.Setting.SettingType}. " @@ -153,7 +153,7 @@ private bool TryEvaluateTargetingRules(TargetingRule[] targetingRules, ref Evalu var percentageOptions = targetingRule.PercentageOptions; if (percentageOptions is not { Length: > 0 }) { - throw new InvalidOperationException("Targeting rule THEN part is missing or invalid."); + throw new InvalidConfigModelException("Targeting rule THEN part is missing or invalid."); } logBuilder?.IncreaseIndent(); @@ -254,7 +254,7 @@ private bool TryEvaluatePercentageOptions(PercentageOption[] percentageOptions, return true; } - throw new InvalidOperationException("Sum of percentage option percentages is less than 100."); + throw new InvalidConfigModelException("Sum of percentage option percentages is less than 100."); } private bool EvaluateConditions(TCondition[] conditions, TargetingRule? targetingRule, string contextSalt, ref EvaluateContext context, out string? error) @@ -359,7 +359,7 @@ private bool EvaluateUserCondition(UserCondition condition, string contextSalt, return false; } - var userAttributeName = condition.ComparisonAttribute ?? throw new InvalidOperationException("Comparison attribute name is missing."); + var userAttributeName = condition.ComparisonAttribute ?? throw new InvalidConfigModelException("Comparison attribute name is missing."); var userAttributeValue = context.User.GetAttribute(userAttributeName); if (userAttributeValue is null || userAttributeValue is string { Length: 0 }) @@ -459,7 +459,7 @@ private bool EvaluateUserCondition(UserCondition condition, string contextSalt, EnsureConfigJsonSalt(context.Setting.ConfigJsonSalt), contextSalt, negate: comparator == UserComparator.SensitiveArrayNotContainsAnyOf); default: - throw new InvalidOperationException("Comparison operator is invalid."); + throw new InvalidConfigModelException("Comparison operator is invalid."); } } @@ -717,11 +717,11 @@ private bool EvaluatePrerequisiteFlagCondition(PrerequisiteFlagCondition conditi var prerequisiteFlagKey = condition.PrerequisiteFlagKey; if (prerequisiteFlagKey is null) { - throw new InvalidOperationException("Prerequisite flag key is missing."); + throw new InvalidConfigModelException("Prerequisite flag key is missing."); } else if (!context.Settings.TryGetValue(prerequisiteFlagKey, out prerequisiteFlag)) { - throw new InvalidOperationException("Prerequisite flag is missing."); + throw new InvalidConfigModelException("Prerequisite flag is missing."); } var comparisonValue = EnsureComparisonValue(condition.ComparisonValue.GetValue(throwIfInvalid: false)); @@ -729,7 +729,7 @@ private bool EvaluatePrerequisiteFlagCondition(PrerequisiteFlagCondition conditi var expectedSettingType = comparisonValue.GetType().ToSettingType(); if (prerequisiteFlag.SettingType != Setting.UnknownType && prerequisiteFlag.SettingType != expectedSettingType) { - throw new InvalidOperationException($"Type mismatch between comparison value '{comparisonValue}' and prerequisite flag '{prerequisiteFlagKey}'."); + throw new InvalidConfigModelException($"Type mismatch between comparison value '{comparisonValue}' and prerequisite flag '{prerequisiteFlagKey}'."); } context.VisitedFlags.Add(context.Key); @@ -737,7 +737,7 @@ private bool EvaluatePrerequisiteFlagCondition(PrerequisiteFlagCondition conditi { context.VisitedFlags.Add(prerequisiteFlagKey!); var dependencyCycle = new StringListFormatter(context.VisitedFlags).ToString("a", CultureInfo.InvariantCulture); - throw new InvalidOperationException($"Circular dependency detected between the following depending flags: {dependencyCycle}."); + throw new InvalidConfigModelException($"Circular dependency detected between the following depending flags: {dependencyCycle}."); } var prerequisiteFlagContext = new EvaluateContext(prerequisiteFlagKey!, prerequisiteFlag!, context); @@ -758,7 +758,7 @@ private bool EvaluatePrerequisiteFlagCondition(PrerequisiteFlagCondition conditi { PrerequisiteFlagComparator.Equals => prerequisiteFlagValue.Equals(comparisonValue), PrerequisiteFlagComparator.NotEquals => !prerequisiteFlagValue.Equals(comparisonValue), - _ => throw new InvalidOperationException("Comparison operator is invalid.") + _ => throw new InvalidConfigModelException("Comparison operator is invalid.") }; logBuilder? @@ -791,11 +791,11 @@ private bool EvaluateSegmentCondition(SegmentCondition condition, ref EvaluateCo return false; } - var segment = condition.Segment ?? throw new InvalidOperationException("Segment reference is invalid."); + var segment = condition.Segment ?? throw new InvalidConfigModelException("Segment reference is invalid."); if (segment.Name is not { Length: > 0 }) { - throw new InvalidOperationException("Segment name is missing."); + throw new InvalidConfigModelException("Segment name is missing."); } logBuilder? @@ -810,7 +810,7 @@ private bool EvaluateSegmentCondition(SegmentCondition condition, ref EvaluateCo { SegmentComparator.IsIn => segmentResult, SegmentComparator.IsNotIn => !segmentResult, - _ => throw new InvalidOperationException("Comparison operator is invalid.") + _ => throw new InvalidConfigModelException("Comparison operator is invalid.") }; if (logBuilder is not null) @@ -865,13 +865,13 @@ private static byte[] HashComparisonValue(ReadOnlySpan valueUtf8, string c private static string EnsureConfigJsonSalt([NotNull] string? value) { - return value ?? throw new InvalidOperationException("Config JSON salt is missing."); + return value ?? throw new InvalidConfigModelException("Config JSON salt is missing."); } [return: NotNull] private static T EnsureComparisonValue([NotNull] T? value) { - return value ?? throw new InvalidOperationException("Comparison value is missing or invalid."); + return value ?? throw new InvalidConfigModelException("Comparison value is missing or invalid."); } private static string UserAttributeValueToString(object attributeValue) diff --git a/src/ConfigCatClient/Evaluation/RolloutEvaluatorExtensions.cs b/src/ConfigCatClient/Evaluation/RolloutEvaluatorExtensions.cs index f0a8b52e..12428daa 100644 --- a/src/ConfigCatClient/Evaluation/RolloutEvaluatorExtensions.cs +++ b/src/ConfigCatClient/Evaluation/RolloutEvaluatorExtensions.cs @@ -15,14 +15,16 @@ public static EvaluationDetails Evaluate(this IRolloutEvaluator evaluator, if (settings is null) { logMessage = logger.ConfigJsonIsNotPresent(key, nameof(defaultValue), defaultValue); - return EvaluationDetails.FromDefaultValue(key, defaultValue, fetchTime: remoteConfig?.TimeStamp, user, logMessage.InvariantFormattedMessage); + return EvaluationDetails.FromDefaultValue(key, defaultValue, fetchTime: remoteConfig?.TimeStamp, user, + logMessage.InvariantFormattedMessage, errorCode: EvaluationErrorCode.ConfigJsonNotAvailable); } if (!settings.TryGetValue(key, out var setting)) { var availableKeys = new StringListFormatter(settings.Keys).ToString(); logMessage = logger.SettingEvaluationFailedDueToMissingKey(key, nameof(defaultValue), defaultValue, availableKeys); - return EvaluationDetails.FromDefaultValue(key, defaultValue, fetchTime: remoteConfig?.TimeStamp, user, logMessage.InvariantFormattedMessage); + return EvaluationDetails.FromDefaultValue(key, defaultValue, fetchTime: remoteConfig?.TimeStamp, user, + logMessage.InvariantFormattedMessage, errorCode: EvaluationErrorCode.SettingKeyMissing); } var evaluateContext = new EvaluateContext(key, setting, user, settings); @@ -65,7 +67,8 @@ public static EvaluationDetails[] EvaluateAll(this IRolloutEvaluator evaluator, { exceptionList ??= new List(); exceptionList.Add(ex); - evaluationDetails = EvaluationDetails.FromDefaultValue(kvp.Key, defaultValue: null, fetchTime: remoteConfig?.TimeStamp, user, ex.Message, ex); + evaluationDetails = EvaluationDetails.FromDefaultValue(kvp.Key, defaultValue: null, fetchTime: remoteConfig?.TimeStamp, user, + ex.Message, ex, GetErrorCode(ex)); } evaluationDetailsArray[index++] = evaluationDetails; @@ -85,4 +88,14 @@ internal static bool CheckSettingsAvailable([NotNullWhen(true)] Dictionary evaluationErrorException.ErrorCode, + InvalidConfigModelException => EvaluationErrorCode.InvalidConfigModel, + _ => EvaluationErrorCode.UnexpectedError, + }; + } } diff --git a/src/ConfigCatClient/FetchResult.cs b/src/ConfigCatClient/FetchResult.cs index 4c92aff3..deeaeda0 100644 --- a/src/ConfigCatClient/FetchResult.cs +++ b/src/ConfigCatClient/FetchResult.cs @@ -9,25 +9,26 @@ internal readonly struct FetchResult public static FetchResult Success(ProjectConfig config) { - return new FetchResult(config, errorMessageOrToken: null); + return new FetchResult(config, RefreshErrorCode.None, errorMessageOrToken: null); } public static FetchResult NotModified(ProjectConfig config) { - return new FetchResult(config, NotModifiedToken); + return new FetchResult(config, RefreshErrorCode.None, NotModifiedToken); } - public static FetchResult Failure(ProjectConfig config, string errorMessage, Exception? errorException = null) + public static FetchResult Failure(ProjectConfig config, RefreshErrorCode errorCode, string errorMessage, Exception? errorException = null) { - return new FetchResult(config, errorMessage, errorException); + return new FetchResult(config, errorCode, errorMessage, errorException); } private readonly object? errorMessageOrToken; - private FetchResult(ProjectConfig config, object? errorMessageOrToken, Exception? errorException = null) + private FetchResult(ProjectConfig config, RefreshErrorCode errorCode, object? errorMessageOrToken, Exception? errorException = null) { Config = config; this.errorMessageOrToken = errorMessageOrToken; + ErrorCode = errorCode; ErrorException = errorException; } @@ -37,6 +38,7 @@ private FetchResult(ProjectConfig config, object? errorMessageOrToken, Exception public bool IsFailure => this.errorMessageOrToken is string; public ProjectConfig Config { get; } + public RefreshErrorCode ErrorCode { get; } public string? ErrorMessage => this.errorMessageOrToken as string; public Exception? ErrorException { get; } } diff --git a/src/ConfigCatClient/Hooks/ConfigFetchedEventArgs.cs b/src/ConfigCatClient/Hooks/ConfigFetchedEventArgs.cs new file mode 100644 index 00000000..413ab724 --- /dev/null +++ b/src/ConfigCatClient/Hooks/ConfigFetchedEventArgs.cs @@ -0,0 +1,25 @@ +using System; + +namespace ConfigCat.Client; + +/// +/// Provides data for the event. +/// +public class ConfigFetchedEventArgs : EventArgs +{ + internal ConfigFetchedEventArgs(RefreshResult result, bool isInitiatedByUser) + { + Result = result; + IsInitiatedByUser = isInitiatedByUser; + } + + /// + /// The result of the operation. + /// + public RefreshResult Result { get; } + + /// + /// Indicates whether the operation was initiated by the user or by the SDK. + /// + public bool IsInitiatedByUser { get; } +} diff --git a/src/ConfigCatClient/Hooks/Hooks.cs b/src/ConfigCatClient/Hooks/Hooks.cs index f8f835e6..51fae7e2 100644 --- a/src/ConfigCatClient/Hooks/Hooks.cs +++ b/src/ConfigCatClient/Hooks/Hooks.cs @@ -5,27 +5,27 @@ namespace ConfigCat.Client; internal class Hooks : IProvidesHooks { - private static readonly EventHandlers DisconnectedEventHandlers = new(); + private static readonly Events DisconnectedEvents = new(); - private volatile EventHandlers eventHandlers; + private volatile Events events; private IConfigCatClient? client; // should be null only in case of testing - protected Hooks(EventHandlers eventHandlers) + protected Hooks(Events events) { - this.eventHandlers = eventHandlers; + this.events = events; } - public Hooks() : this(new ActualEventHandlers()) { } + public Hooks() : this(new RealEvents()) { } public virtual bool TryDisconnect() { - // Replacing the current EventHandlers object (eventHandlers) with a special instance of EventHandlers (DisconnectedEventHandlers) achieves multiple things: + // Replacing the current Events object (this.events) with a special instance of Events (DisconnectedEvents) achieves multiple things: // 1. determines whether the hooks instance has already been disconnected or not, // 2. removes implicit references to subscriber objects (so this instance won't keep them alive under any circumstances), // 3. makes sure that future subscriptions are ignored from this point on. - var originalEventHandlers = Interlocked.Exchange(ref this.eventHandlers, DisconnectedEventHandlers); + var originalEvents = Interlocked.Exchange(ref this.events, DisconnectedEvents); - return !ReferenceEquals(originalEventHandlers, DisconnectedEventHandlers); + return !ReferenceEquals(originalEvents, DisconnectedEvents); } public virtual void SetSender(IConfigCatClient client) @@ -33,69 +33,97 @@ public virtual void SetSender(IConfigCatClient client) this.client = client; } - /// - public event EventHandler? ClientReady - { - add { this.eventHandlers.ClientReady += value; } - remove { this.eventHandlers.ClientReady -= value; } - } + public void RaiseClientReady() + => this.events.RaiseClientReady(this.client); + + public void RaiseFlagEvaluated(EvaluationDetails evaluationDetails) + => this.events.RaiseFlagEvaluated(this.client, evaluationDetails); - internal void RaiseClientReady() + public void RaiseConfigFetched(RefreshResult result, bool isInitiatedByUser) + => this.events.RaiseConfigFetched(this.client, result, isInitiatedByUser); + + public void RaiseConfigChanged(IConfig newConfig) + => this.events.RaiseConfigChanged(this.client, newConfig); + + public void RaiseError(string message, Exception? exception) + => this.events.RaiseError(this.client, message, exception); + + public event EventHandler? ClientReady { - this.eventHandlers.ClientReady?.Invoke(this.client, EventArgs.Empty); + add { this.events.ClientReady += value; } + remove { this.events.ClientReady -= value; } } - /// public event EventHandler? FlagEvaluated { - add { this.eventHandlers.FlagEvaluated += value; } - remove { this.eventHandlers.FlagEvaluated -= value; } + add { this.events.FlagEvaluated += value; } + remove { this.events.FlagEvaluated -= value; } } - internal void RaiseFlagEvaluated(EvaluationDetails evaluationDetails) + public event EventHandler? ConfigFetched { - this.eventHandlers.FlagEvaluated?.Invoke(this.client, new FlagEvaluatedEventArgs(evaluationDetails)); + add { this.events.ConfigFetched += value; } + remove { this.events.ConfigFetched -= value; } } - /// public event EventHandler? ConfigChanged { - add { this.eventHandlers.ConfigChanged += value; } - remove { this.eventHandlers.ConfigChanged -= value; } + add { this.events.ConfigChanged += value; } + remove { this.events.ConfigChanged -= value; } } - internal void RaiseConfigChanged(IConfig newConfig) - { - this.eventHandlers.ConfigChanged?.Invoke(this.client, new ConfigChangedEventArgs(newConfig)); - } - - /// public event EventHandler? Error { - add { this.eventHandlers.Error += value; } - remove { this.eventHandlers.Error -= value; } + add { this.events.Error += value; } + remove { this.events.Error -= value; } } - internal void RaiseError(string message, Exception? exception) + public class Events : IProvidesHooks { - this.eventHandlers.Error?.Invoke(this.client, new ConfigCatClientErrorEventArgs(message, exception)); - } - - protected class EventHandlers - { - private static void Noop(Delegate? _) { /* This method is for keeping SonarQube happy. */ } - - public virtual EventHandler? ClientReady { get => null; set => Noop(value); } - public virtual EventHandler? FlagEvaluated { get => null; set => Noop(value); } - public virtual EventHandler? ConfigChanged { get => null; set => Noop(value); } - public virtual EventHandler? Error { get => null; set => Noop(value); } + public virtual void RaiseClientReady(IConfigCatClient? client) { /* intentional no-op */ } + public virtual void RaiseFlagEvaluated(IConfigCatClient? client, EvaluationDetails evaluationDetails) { /* intentional no-op */ } + public virtual void RaiseConfigFetched(IConfigCatClient? client, RefreshResult result, bool isInitiatedByUser) { /* intentional no-op */ } + public virtual void RaiseConfigChanged(IConfigCatClient? client, IConfig newConfig) { /* intentional no-op */ } + public virtual void RaiseError(IConfigCatClient? client, string message, Exception? exception) { /* intentional no-op */ } + + public virtual event EventHandler? ClientReady { add { /* intentional no-op */ } remove { /* intentional no-op */ } } + public virtual event EventHandler? FlagEvaluated { add { /* intentional no-op */ } remove { /* intentional no-op */ } } + public virtual event EventHandler? ConfigFetched { add { /* intentional no-op */ } remove { /* intentional no-op */ } } + public virtual event EventHandler? ConfigChanged { add { /* intentional no-op */ } remove { /* intentional no-op */ } } + public virtual event EventHandler? Error { add { /* intentional no-op */ } remove { /* intentional no-op */ } } } - private sealed class ActualEventHandlers : EventHandlers + private sealed class RealEvents : Events { - public override EventHandler? ClientReady { get; set; } - public override EventHandler? FlagEvaluated { get; set; } - public override EventHandler? ConfigChanged { get; set; } - public override EventHandler? Error { get; set; } + public override void RaiseClientReady(IConfigCatClient? client) + { + ClientReady?.Invoke(client, EventArgs.Empty); + } + + public override void RaiseFlagEvaluated(IConfigCatClient? client, EvaluationDetails evaluationDetails) + { + FlagEvaluated?.Invoke(client, new FlagEvaluatedEventArgs(evaluationDetails)); + } + + public override void RaiseConfigFetched(IConfigCatClient? client, RefreshResult result, bool isInitiatedByUser) + { + ConfigFetched?.Invoke(client, new ConfigFetchedEventArgs(result, isInitiatedByUser)); + } + + public override void RaiseConfigChanged(IConfigCatClient? client, IConfig newConfig) + { + ConfigChanged?.Invoke(client, new ConfigChangedEventArgs(newConfig)); + } + + public override void RaiseError(IConfigCatClient? client, string message, Exception? exception) + { + Error?.Invoke(client, new ConfigCatClientErrorEventArgs(message, exception)); + } + + public override event EventHandler? ClientReady; + public override event EventHandler? FlagEvaluated; + public override event EventHandler? ConfigFetched; + public override event EventHandler? ConfigChanged; + public override event EventHandler? Error; } } diff --git a/src/ConfigCatClient/Hooks/IProvidesHooks.cs b/src/ConfigCatClient/Hooks/IProvidesHooks.cs index f58d4690..2f81b859 100644 --- a/src/ConfigCatClient/Hooks/IProvidesHooks.cs +++ b/src/ConfigCatClient/Hooks/IProvidesHooks.cs @@ -18,7 +18,12 @@ public interface IProvidesHooks event EventHandler? FlagEvaluated; /// - /// Occurs after the locally cached config has been updated. + /// Occurs after attempting to refresh the locally cached config by fetching the latest version from the remote server. + /// + event EventHandler? ConfigFetched; + + /// + /// Occurs after the locally cached config has been updated to a newer version. /// event EventHandler? ConfigChanged; diff --git a/src/ConfigCatClient/Hooks/NullHooks.cs b/src/ConfigCatClient/Hooks/NullHooks.cs index 909e9cb9..26ade3f7 100644 --- a/src/ConfigCatClient/Hooks/NullHooks.cs +++ b/src/ConfigCatClient/Hooks/NullHooks.cs @@ -4,7 +4,7 @@ internal sealed class NullHooks : Hooks { public static readonly NullHooks Instance = new(); - private NullHooks() : base(new EventHandlers()) { } + private NullHooks() : base(new Events()) { } public override bool TryDisconnect() { diff --git a/src/ConfigCatClient/Hooks/SafeHooksWrapper.cs b/src/ConfigCatClient/Hooks/SafeHooksWrapper.cs index bb763323..b53366ea 100644 --- a/src/ConfigCatClient/Hooks/SafeHooksWrapper.cs +++ b/src/ConfigCatClient/Hooks/SafeHooksWrapper.cs @@ -21,16 +21,24 @@ public SafeHooksWrapper(Hooks hooks) } [MethodImpl(MethodImplOptions.NoInlining)] - public void RaiseClientReady() => Hooks.RaiseClientReady(); + public void RaiseClientReady() + => Hooks.RaiseClientReady(); [MethodImpl(MethodImplOptions.NoInlining)] - public void RaiseFlagEvaluated(EvaluationDetails evaluationDetails) => Hooks.RaiseFlagEvaluated(evaluationDetails); + public void RaiseFlagEvaluated(EvaluationDetails evaluationDetails) + => Hooks.RaiseFlagEvaluated(evaluationDetails); [MethodImpl(MethodImplOptions.NoInlining)] - public void RaiseConfigChanged(IConfig newConfig) => Hooks.RaiseConfigChanged(newConfig); + public void RaiseConfigFetched(RefreshResult result, bool isInitiatedByUser) + => Hooks.RaiseConfigFetched(result, isInitiatedByUser); [MethodImpl(MethodImplOptions.NoInlining)] - public void RaiseError(string message, Exception? exception) => Hooks.RaiseError(message, exception); + public void RaiseConfigChanged(IConfig newConfig) + => Hooks.RaiseConfigChanged(newConfig); + + [MethodImpl(MethodImplOptions.NoInlining)] + public void RaiseError(string message, Exception? exception) + => Hooks.RaiseError(message, exception); public static implicit operator SafeHooksWrapper(Hooks? hooks) => hooks is not null ? new SafeHooksWrapper(hooks) : default; } diff --git a/src/ConfigCatClient/HttpConfigFetcher.cs b/src/ConfigCatClient/HttpConfigFetcher.cs index 92f6a0fb..ede9daa8 100644 --- a/src/ConfigCatClient/HttpConfigFetcher.cs +++ b/src/ConfigCatClient/HttpConfigFetcher.cs @@ -74,6 +74,7 @@ private Task BeginFetchOrJoinPending(ProjectConfig lastConfig) private async ValueTask FetchInternalAsync(ProjectConfig lastConfig) { FormattableLogMessage logMessage; + RefreshErrorCode errorCode; Exception errorException; try { @@ -89,7 +90,7 @@ private async ValueTask FetchInternalAsync(ProjectConfig lastConfig { var exception = responseWithBody.Exception; logMessage = this.logger.FetchReceived200WithInvalidBody(exception); - return FetchResult.Failure(lastConfig, logMessage.InvariantFormattedMessage, exception); + return FetchResult.Failure(lastConfig, RefreshErrorCode.InvalidHttpResponseContent, logMessage.InvariantFormattedMessage, exception); } return FetchResult.Success(new ProjectConfig @@ -104,7 +105,7 @@ private async ValueTask FetchInternalAsync(ProjectConfig lastConfig if (lastConfig.IsEmpty) { logMessage = this.logger.FetchReceived304WhenLocalCacheIsEmpty((int)response.StatusCode, response.ReasonPhrase); - return FetchResult.Failure(lastConfig, logMessage.InvariantFormattedMessage); + return FetchResult.Failure(lastConfig, RefreshErrorCode.InvalidHttpResponseWhenLocalCacheIsEmpty, logMessage.InvariantFormattedMessage); } return FetchResult.NotModified(lastConfig.With(ProjectConfig.GenerateTimeStamp())); @@ -114,13 +115,13 @@ private async ValueTask FetchInternalAsync(ProjectConfig lastConfig logMessage = this.logger.FetchFailedDueToInvalidSdkKey(); // We update the timestamp for extra protection against flooding. - return FetchResult.Failure(lastConfig.With(ProjectConfig.GenerateTimeStamp()), logMessage.InvariantFormattedMessage); + return FetchResult.Failure(lastConfig.With(ProjectConfig.GenerateTimeStamp()), RefreshErrorCode.InvalidSdkKey, logMessage.InvariantFormattedMessage); default: logMessage = this.logger.FetchFailedDueToUnexpectedHttpResponse((int)response.StatusCode, response.ReasonPhrase); ReInitializeHttpClient(); - return FetchResult.Failure(lastConfig, logMessage.InvariantFormattedMessage); + return FetchResult.Failure(lastConfig, RefreshErrorCode.UnexpectedHttpResponse, logMessage.InvariantFormattedMessage); } } catch (OperationCanceledException) when (this.cancellationTokenSource.IsCancellationRequested) @@ -135,21 +136,24 @@ private async ValueTask FetchInternalAsync(ProjectConfig lastConfig catch (OperationCanceledException ex) { logMessage = this.logger.FetchFailedDueToRequestTimeout(this.timeout, ex); + errorCode = RefreshErrorCode.HttpRequestTimeout; errorException = ex; } catch (HttpRequestException ex) when (ex.InnerException is WebException { Status: WebExceptionStatus.SecureChannelFailure }) { logMessage = this.logger.EstablishingSecureConnectionFailed(ex); + errorCode = RefreshErrorCode.HttpRequestFailure; errorException = ex; } catch (Exception ex) { logMessage = this.logger.FetchFailedDueToUnexpectedError(ex); + errorCode = RefreshErrorCode.HttpRequestFailure; errorException = ex; } ReInitializeHttpClient(); - return FetchResult.Failure(lastConfig, logMessage.InvariantFormattedMessage, errorException); + return FetchResult.Failure(lastConfig, errorCode, logMessage.InvariantFormattedMessage, errorException); } private async ValueTask FetchRequestAsync(string? httpETag, Uri requestUri, sbyte maxExecutionCount = 3) diff --git a/src/ConfigCatClient/Models/ConditionContainer.cs b/src/ConfigCatClient/Models/ConditionContainer.cs index 6913cc29..4a368024 100644 --- a/src/ConfigCatClient/Models/ConditionContainer.cs +++ b/src/ConfigCatClient/Models/ConditionContainer.cs @@ -49,6 +49,6 @@ public PrerequisiteFlagCondition? PrerequisiteFlagCondition public readonly Condition? GetCondition(bool throwIfInvalid = true) { return this.condition as Condition - ?? (!throwIfInvalid ? null : throw new InvalidOperationException("Condition is missing or invalid.")); + ?? (!throwIfInvalid ? null : throw new InvalidConfigModelException("Condition is missing or invalid.")); } } diff --git a/src/ConfigCatClient/Models/InvalidConfigModelException.cs b/src/ConfigCatClient/Models/InvalidConfigModelException.cs new file mode 100644 index 00000000..b01956b4 --- /dev/null +++ b/src/ConfigCatClient/Models/InvalidConfigModelException.cs @@ -0,0 +1,8 @@ +using System; + +namespace ConfigCat.Client; + +internal sealed class InvalidConfigModelException : InvalidOperationException +{ + public InvalidConfigModelException(string message) : base(message) { } +} diff --git a/src/ConfigCatClient/Models/PrerequisiteFlagCondition.cs b/src/ConfigCatClient/Models/PrerequisiteFlagCondition.cs index 059cf721..eb9223c2 100644 --- a/src/ConfigCatClient/Models/PrerequisiteFlagCondition.cs +++ b/src/ConfigCatClient/Models/PrerequisiteFlagCondition.cs @@ -43,7 +43,7 @@ internal sealed class PrerequisiteFlagCondition : Condition, IPrerequisiteFlagCo #endif public string? PrerequisiteFlagKey { get; set; } - string IPrerequisiteFlagCondition.PrerequisiteFlagKey => PrerequisiteFlagKey ?? throw new InvalidOperationException("Prerequisite flag key is missing."); + string IPrerequisiteFlagCondition.PrerequisiteFlagKey => PrerequisiteFlagKey ?? throw new InvalidConfigModelException("Prerequisite flag key is missing."); private PrerequisiteFlagComparator comparator = UnknownComparator; diff --git a/src/ConfigCatClient/Models/Segment.cs b/src/ConfigCatClient/Models/Segment.cs index 081c62e0..876b8540 100644 --- a/src/ConfigCatClient/Models/Segment.cs +++ b/src/ConfigCatClient/Models/Segment.cs @@ -38,7 +38,7 @@ internal sealed class Segment : ISegment #endif public string? Name { get; set; } - string ISegment.Name => Name ?? throw new InvalidOperationException("Segment name is missing."); + string ISegment.Name => Name ?? throw new InvalidConfigModelException("Segment name is missing."); private UserCondition[]? conditions; diff --git a/src/ConfigCatClient/Models/SegmentCondition.cs b/src/ConfigCatClient/Models/SegmentCondition.cs index 95ab69e3..f4fd7b4a 100644 --- a/src/ConfigCatClient/Models/SegmentCondition.cs +++ b/src/ConfigCatClient/Models/SegmentCondition.cs @@ -40,7 +40,7 @@ internal sealed class SegmentCondition : Condition, ISegmentCondition [JsonIgnore] public Segment? Segment { get; private set; } - ISegment ISegmentCondition.Segment => Segment ?? throw new InvalidOperationException("Segment reference is invalid."); + ISegment ISegmentCondition.Segment => Segment ?? throw new InvalidConfigModelException("Segment reference is invalid."); private SegmentComparator comparator = UnknownComparator; diff --git a/src/ConfigCatClient/Models/SettingValue.cs b/src/ConfigCatClient/Models/SettingValue.cs index 15b5300c..326198c5 100644 --- a/src/ConfigCatClient/Models/SettingValue.cs +++ b/src/ConfigCatClient/Models/SettingValue.cs @@ -83,14 +83,14 @@ public object? UnsupportedValue if (HasUnsupportedValue) { var unsupportedValue = UnsupportedValue; - throw new InvalidOperationException(unsupportedValue is not null + throw new InvalidConfigModelException(unsupportedValue is not null ? $"Setting value '{unsupportedValue}' is of an unsupported type ({unsupportedValue.GetType()})." : "Setting value is null."); } // Value is missing or multiple values specified in the config JSON? else { - throw new InvalidOperationException("Setting value is missing or invalid."); + throw new InvalidConfigModelException("Setting value is missing or invalid."); } } @@ -103,7 +103,7 @@ public object? UnsupportedValue if (value is null || value.GetType().ToSettingType() != settingType) { - return !throwIfInvalid ? null : throw new InvalidOperationException($"Setting value is not of the expected type {settingType}."); + return !throwIfInvalid ? null : throw new InvalidConfigModelException($"Setting value is not of the expected type {settingType}."); } return value; diff --git a/src/ConfigCatClient/Models/UserCondition.cs b/src/ConfigCatClient/Models/UserCondition.cs index 654176ae..a4d44e8e 100644 --- a/src/ConfigCatClient/Models/UserCondition.cs +++ b/src/ConfigCatClient/Models/UserCondition.cs @@ -45,7 +45,7 @@ internal sealed class UserCondition : Condition, IUserCondition #endif public string? ComparisonAttribute { get; set; } - string IUserCondition.ComparisonAttribute => ComparisonAttribute ?? throw new InvalidOperationException("Comparison attribute name is missing."); + string IUserCondition.ComparisonAttribute => ComparisonAttribute ?? throw new InvalidConfigModelException("Comparison attribute name is missing."); private UserComparator comparator = UnknownComparator; @@ -105,7 +105,7 @@ public string[]? StringListValue { return ModelHelper.IsValidOneOf(this.comparisonValue) ? this.comparisonValue - : (!throwIfInvalid ? null : throw new InvalidOperationException("Comparison value is missing or invalid.")); + : (!throwIfInvalid ? null : throw new InvalidConfigModelException("Comparison value is missing or invalid.")); } public override string ToString() diff --git a/src/ConfigCatClient/RefreshErrorCode.cs b/src/ConfigCatClient/RefreshErrorCode.cs new file mode 100644 index 00000000..1ffb825c --- /dev/null +++ b/src/ConfigCatClient/RefreshErrorCode.cs @@ -0,0 +1,49 @@ +namespace ConfigCat.Client; + +/// +/// Specifies the possible config data refresh error codes. +/// +public enum RefreshErrorCode +{ + /// + /// An unexpected error occurred during the refresh operation. + /// + UnexpectedError = -1, + /// + /// No error occurred (the refresh operation was successful). + /// + None = 0, + /// + /// The refresh operation failed because the client is configured to use the override behavior, + /// which prevents making HTTP requests. + /// + LocalOnlyClient = 1, + /// + /// The refresh operation failed because the client is in offline mode, it cannot initiate HTTP requests. + /// + OfflineClient = 3200, + /// + /// The refresh operation failed because a HTTP response indicating an invalid SDK Key was received (403 Forbidden or 404 Not Found). + /// + InvalidSdkKey = 1100, + /// + /// The refresh operation failed because an invalid HTTP response was received (unexpected HTTP status code). + /// + UnexpectedHttpResponse = 1101, + /// + /// The refresh operation failed because the HTTP request timed out. + /// + HttpRequestTimeout = 1102, + /// + /// The refresh operation failed because the HTTP request failed (most likely, due to a local network issue). + /// + HttpRequestFailure = 1103, + /// + /// The refresh operation failed because an invalid HTTP response was received (200 OK with an invalid content). + /// + InvalidHttpResponseContent = 1105, + /// + /// The refresh operation failed because an invalid HTTP response was received (304 Not Modified when no config JSON was cached locally). + /// + InvalidHttpResponseWhenLocalCacheIsEmpty = 1106, +} diff --git a/src/ConfigCatClient/RefreshResult.cs b/src/ConfigCatClient/RefreshResult.cs index 4d0bcd73..840574ca 100644 --- a/src/ConfigCatClient/RefreshResult.cs +++ b/src/ConfigCatClient/RefreshResult.cs @@ -21,20 +21,24 @@ public static RefreshResult Success() /// Creates an instance of the struct which indicates that the operation failed. /// /// The new instance. - public static RefreshResult Failure(string errorMessage, Exception? errorException = null) + public static RefreshResult Failure(RefreshErrorCode errorCode, string errorMessage, Exception? errorException = null) { - return new RefreshResult(errorMessage ?? throw new ArgumentNullException(nameof(errorMessage)), errorException); + return new RefreshResult( + errorCode != RefreshErrorCode.None ? errorCode : throw new ArgumentOutOfRangeException(nameof(errorCode), errorCode, null), + errorMessage ?? throw new ArgumentNullException(nameof(errorMessage)), + errorException); } - internal static RefreshResult From(FetchResult fetchResult) + internal static RefreshResult From(in FetchResult fetchResult) { return !fetchResult.IsFailure ? Success() - : Failure(fetchResult.ErrorMessage, fetchResult.ErrorException); + : Failure(fetchResult.ErrorCode, fetchResult.ErrorMessage, fetchResult.ErrorException); } - private RefreshResult(string? errorMessage, Exception? errorException) + private RefreshResult(RefreshErrorCode errorCode, string? errorMessage, Exception? errorException) { + ErrorCode = errorCode; ErrorMessage = errorMessage; ErrorException = errorException; } @@ -45,6 +49,11 @@ private RefreshResult(string? errorMessage, Exception? errorException) [MemberNotNullWhen(false, nameof(ErrorMessage))] public bool IsSuccess => ErrorMessage is null; + /// + /// The code identifying the reason for the error in case the operation failed. + /// + public RefreshErrorCode ErrorCode { get; } + /// /// Error message in case the operation failed, otherwise . ///