From 39417bd022487e634fb3ae75c155967d972ab357 Mon Sep 17 00:00:00 2001 From: Debdatta Kunda Date: Mon, 8 Jan 2024 16:41:14 -0800 Subject: [PATCH 1/3] Code changes to add continuation token for account properties task. --- .../src/Routing/GlobalEndpointManager.cs | 23 ++++++++++++++----- 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/Microsoft.Azure.Cosmos/src/Routing/GlobalEndpointManager.cs b/Microsoft.Azure.Cosmos/src/Routing/GlobalEndpointManager.cs index 64156f729e..1f5862fa8e 100644 --- a/Microsoft.Azure.Cosmos/src/Routing/GlobalEndpointManager.cs +++ b/Microsoft.Azure.Cosmos/src/Routing/GlobalEndpointManager.cs @@ -574,12 +574,23 @@ internal async Task GetDatabaseAccountAsync(bool forceRefresh #nullable disable // Needed because AsyncCache does not have nullable enabled return await this.databaseAccountCache.GetAsync( key: string.Empty, - obsoleteValue: null, - singleValueInitFunc: () => GlobalEndpointManager.GetDatabaseAccountFromAnyLocationsAsync( - this.defaultEndpoint, - this.connectionPolicy.PreferredLocations, - this.GetDatabaseAccountAsync, - this.cancellationTokenSource.Token), + obsoleteValue: null, + singleValueInitFunc: () => + { + Task accountPropertiesTask = GlobalEndpointManager.GetDatabaseAccountFromAnyLocationsAsync( + this.defaultEndpoint, + this.connectionPolicy.PreferredLocations, + this.GetDatabaseAccountAsync, + this.cancellationTokenSource.Token); + + Task continuationTask = accountPropertiesTask.ContinueWith( + task => DefaultTrace.TraceVerbose("Failed to fetch database account from any locations with exception: {0}. Activity Id: '{1}'", + task.Exception, + System.Diagnostics.Trace.CorrelationManager.ActivityId), + TaskContinuationOptions.OnlyOnFaulted); + + return accountPropertiesTask; + }, cancellationToken: this.cancellationTokenSource.Token, forceRefresh: forceRefresh); #nullable enable From b92d314653111b092046aa9e30f10ef808c05793 Mon Sep 17 00:00:00 2001 From: Debdatta Kunda Date: Tue, 30 Jan 2024 14:53:30 -0800 Subject: [PATCH 2/3] Code changes to fix unobserved exception from getting thrown. --- .../src/Routing/GlobalEndpointManager.cs | 36 +++++++---------- .../GlobalEndpointManagerTest.cs | 39 +++++++++++++++++++ 2 files changed, 54 insertions(+), 21 deletions(-) diff --git a/Microsoft.Azure.Cosmos/src/Routing/GlobalEndpointManager.cs b/Microsoft.Azure.Cosmos/src/Routing/GlobalEndpointManager.cs index 1f5862fa8e..b82b9813c6 100644 --- a/Microsoft.Azure.Cosmos/src/Routing/GlobalEndpointManager.cs +++ b/Microsoft.Azure.Cosmos/src/Routing/GlobalEndpointManager.cs @@ -559,7 +559,12 @@ private async Task RefreshDatabaseAccountInternalAsync(bool forceRefresh) { this.LastBackgroundRefreshUtc = DateTime.UtcNow; this.locationCache.OnDatabaseAccountRead(await this.GetDatabaseAccountAsync(true)); - + } + catch (Exception ex) + { + DefaultTrace.TraceWarning("Failed to refresh database account with exception: {0}. Activity Id: '{1}'", + ex, + System.Diagnostics.Trace.CorrelationManager.ActivityId); } finally { @@ -571,27 +576,16 @@ private async Task RefreshDatabaseAccountInternalAsync(bool forceRefresh) } internal async Task GetDatabaseAccountAsync(bool forceRefresh = false) { -#nullable disable // Needed because AsyncCache does not have nullable enabled - return await this.databaseAccountCache.GetAsync( - key: string.Empty, +#nullable disable // Needed because AsyncCache does not have nullable enabled + return await this.databaseAccountCache.GetAsync( + key: string.Empty, obsoleteValue: null, - singleValueInitFunc: () => - { - Task accountPropertiesTask = GlobalEndpointManager.GetDatabaseAccountFromAnyLocationsAsync( - this.defaultEndpoint, - this.connectionPolicy.PreferredLocations, - this.GetDatabaseAccountAsync, - this.cancellationTokenSource.Token); - - Task continuationTask = accountPropertiesTask.ContinueWith( - task => DefaultTrace.TraceVerbose("Failed to fetch database account from any locations with exception: {0}. Activity Id: '{1}'", - task.Exception, - System.Diagnostics.Trace.CorrelationManager.ActivityId), - TaskContinuationOptions.OnlyOnFaulted); - - return accountPropertiesTask; - }, - cancellationToken: this.cancellationTokenSource.Token, + singleValueInitFunc: () => GlobalEndpointManager.GetDatabaseAccountFromAnyLocationsAsync( + this.defaultEndpoint, + this.connectionPolicy.PreferredLocations, + this.GetDatabaseAccountAsync, + this.cancellationTokenSource.Token), + cancellationToken: this.cancellationTokenSource.Token, forceRefresh: forceRefresh); #nullable enable } diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/GlobalEndpointManagerTest.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/GlobalEndpointManagerTest.cs index 28e24d7e55..30c0d4e457 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/GlobalEndpointManagerTest.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/GlobalEndpointManagerTest.cs @@ -407,6 +407,45 @@ public async Task GetDatabaseAccountFromAnyLocationsMockTestAsync() } } + /// + /// Test to validate that when an exception is thrown during a RefreshLocationAsync call + /// the exception should not be bubbled up and remain unobserved. The exception should be + /// handled gracefully and logged as a warning trace event. + /// + [TestMethod] + public async Task RefreshLocationAsync_WhenGetDatabaseThrowsException_ShouldNotBubbleUpAsUnobservedException() + { + // Arrange. + Mock mockOwner = new Mock(); + mockOwner.Setup(owner => owner.ServiceEndpoint).Returns(new Uri("https://defaultendpoint.net/")); + mockOwner.Setup(owner => owner.GetDatabaseAccountInternalAsync(It.IsAny(), It.IsAny())).ThrowsAsync(new TaskCanceledException()); + + //Create connection policy and populate preferred locations + ConnectionPolicy connectionPolicy = new ConnectionPolicy(); + connectionPolicy.PreferredLocations.Add("ReadLocation1"); + connectionPolicy.PreferredLocations.Add("ReadLocation2"); + + bool isExceptionLogged = false; + void TraceHandler(string message) + { + if (message.Contains("Failed to refresh database account with exception:")) + { + isExceptionLogged = true; + } + } + + DefaultTrace.TraceSource.Listeners.Add(new TestTraceListener { Callback = TraceHandler }); + DefaultTrace.InitEventListener(); + + using GlobalEndpointManager globalEndpointManager = new (mockOwner.Object, connectionPolicy); + + // Act. + await globalEndpointManager.RefreshLocationAsync(forceRefresh: false); + + // Assert. + Assert.IsTrue(isExceptionLogged, "The exception was logged as a warning trace event."); + } + private sealed class GetAccountRequestInjector { public Func ShouldFailRequest { get; set; } From 567ee5be9e1e28bff3737b1dc96f53d642ff411d Mon Sep 17 00:00:00 2001 From: Debdatta Kunda Date: Tue, 30 Jan 2024 15:06:48 -0800 Subject: [PATCH 3/3] Code changes to sync up with master and remvoe unnecessary changes. --- .../src/Routing/GlobalEndpointManager.cs | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/Microsoft.Azure.Cosmos/src/Routing/GlobalEndpointManager.cs b/Microsoft.Azure.Cosmos/src/Routing/GlobalEndpointManager.cs index 30ddaf3887..db1a01cd61 100644 --- a/Microsoft.Azure.Cosmos/src/Routing/GlobalEndpointManager.cs +++ b/Microsoft.Azure.Cosmos/src/Routing/GlobalEndpointManager.cs @@ -601,16 +601,16 @@ private async Task RefreshDatabaseAccountInternalAsync(bool forceRefresh) } internal async Task GetDatabaseAccountAsync(bool forceRefresh = false) { -#nullable disable // Needed because AsyncCache does not have nullable enabled - return await this.databaseAccountCache.GetAsync( - key: string.Empty, - obsoleteValue: null, - singleValueInitFunc: () => GlobalEndpointManager.GetDatabaseAccountFromAnyLocationsAsync( - this.defaultEndpoint, - this.connectionPolicy.PreferredLocations, - this.GetDatabaseAccountAsync, - this.cancellationTokenSource.Token), - cancellationToken: this.cancellationTokenSource.Token, +#nullable disable // Needed because AsyncCache does not have nullable enabled + return await this.databaseAccountCache.GetAsync( + key: string.Empty, + obsoleteValue: null, + singleValueInitFunc: () => GlobalEndpointManager.GetDatabaseAccountFromAnyLocationsAsync( + this.defaultEndpoint, + this.connectionPolicy.PreferredLocations, + this.GetDatabaseAccountAsync, + this.cancellationTokenSource.Token), + cancellationToken: this.cancellationTokenSource.Token, forceRefresh: forceRefresh); #nullable enable }