From fb09e6b896ae3be4181315cd6e95a81fe38fe361 Mon Sep 17 00:00:00 2001 From: Jake Willey Date: Fri, 6 Mar 2020 14:01:16 -0800 Subject: [PATCH 1/6] Retryafter value is now always included in all query responses. Previously the header would not get set in the query stream APIs. --- .../src/Query/v3Query/QueryIterator.cs | 38 +++--- .../CosmosExceptions/CosmosException.cs | 4 +- .../CosmosBasicQueryTests.cs | 116 ++++++++++++++++++ .../Utils/TestCommon.cs | 10 ++ 4 files changed, 147 insertions(+), 21 deletions(-) diff --git a/Microsoft.Azure.Cosmos/src/Query/v3Query/QueryIterator.cs b/Microsoft.Azure.Cosmos/src/Query/v3Query/QueryIterator.cs index 3dfa07c3ab..cd04d67a2a 100644 --- a/Microsoft.Azure.Cosmos/src/Query/v3Query/QueryIterator.cs +++ b/Microsoft.Azure.Cosmos/src/Query/v3Query/QueryIterator.cs @@ -135,10 +135,9 @@ public override async Task ReadNextAsync(CancellationToken canc diagnostics.AddDiagnosticsInternal(queryPage); } - QueryResponse queryResponse; if (responseCore.IsSuccess) { - queryResponse = QueryResponse.CreateSuccess( + return QueryResponse.CreateSuccess( result: responseCore.CosmosElements, count: responseCore.CosmosElements.Count, responseLengthBytes: responseCore.ResponseLengthBytes, @@ -155,26 +154,27 @@ public override async Task ReadNextAsync(CancellationToken canc SubStatusCode = responseCore.SubStatusCode ?? Documents.SubStatusCodes.Unknown }); } - else + + if (responseCore.CosmosException != null) { - queryResponse = QueryResponse.CreateFailure( - statusCode: responseCore.StatusCode, - cosmosException: responseCore.CosmosException, - requestMessage: null, - diagnostics: diagnostics, - responseHeaders: new CosmosQueryResponseMessageHeaders( - responseCore.ContinuationToken, - responseCore.DisallowContinuationTokenMessage, - cosmosQueryContext.ResourceTypeEnum, - cosmosQueryContext.ContainerResourceId) - { - RequestCharge = responseCore.RequestCharge, - ActivityId = responseCore.ActivityId, - SubStatusCode = responseCore.SubStatusCode ?? Documents.SubStatusCodes.Unknown - }); + return responseCore.CosmosException.ToCosmosResponseMessage(null); } - return queryResponse; + return QueryResponse.CreateFailure( + statusCode: responseCore.StatusCode, + cosmosException: responseCore.CosmosException, + requestMessage: null, + diagnostics: diagnostics, + responseHeaders: new CosmosQueryResponseMessageHeaders( + responseCore.ContinuationToken, + responseCore.DisallowContinuationTokenMessage, + cosmosQueryContext.ResourceTypeEnum, + cosmosQueryContext.ContainerResourceId) + { + RequestCharge = responseCore.RequestCharge, + ActivityId = responseCore.ActivityId, + SubStatusCode = responseCore.SubStatusCode ?? Documents.SubStatusCodes.Unknown, + }); } } diff --git a/Microsoft.Azure.Cosmos/src/Resource/CosmosExceptions/CosmosException.cs b/Microsoft.Azure.Cosmos/src/Resource/CosmosExceptions/CosmosException.cs index 0d797f9848..8addda7904 100644 --- a/Microsoft.Azure.Cosmos/src/Resource/CosmosExceptions/CosmosException.cs +++ b/Microsoft.Azure.Cosmos/src/Resource/CosmosExceptions/CosmosException.cs @@ -192,10 +192,10 @@ private static string MergeErrorMessages(string message, Error error) if (string.IsNullOrEmpty(message)) { - return error.Message; + return error.ToString(); } - return $"{message}; Inner Message:{error.Message}"; + return $"{message}; Inner Message:{error.ToString()}"; } private string ToStringHelper(bool includeDiagnostics) diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/CosmosBasicQueryTests.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/CosmosBasicQueryTests.cs index c16e520e23..07776a8fb6 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/CosmosBasicQueryTests.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/CosmosBasicQueryTests.cs @@ -9,11 +9,14 @@ namespace Microsoft.Azure.Cosmos.SDK.EmulatorTests using System.IO; using System.Linq; using System.Net; + using System.Threading; using System.Threading.Tasks; using Cosmos.Scripts; using Microsoft.Azure.Cosmos.Linq; using Microsoft.Azure.Cosmos.Query.Core; + using Microsoft.Azure.Documents.Collections; using Microsoft.VisualStudio.TestTools.UnitTesting; + using Moq; using Newtonsoft.Json; [TestClass] @@ -147,6 +150,119 @@ public async Task ContainerTest(bool directMode) } } + [TestMethod] + [DataRow(false)] + [DataRow(true)] + public async Task QueryRequestRateTest(bool directMode) + { + CosmosClient client = directMode ? DirectCosmosClient : GatewayCosmosClient; + Container container = client.GetContainer(DatabaseId, ContainerId); + List createdIds = new List() + { + "BasicQueryItem" + Guid.NewGuid(), + "BasicQueryItem2"+ Guid.NewGuid(), + "BasicQueryItem3"+ Guid.NewGuid() + }; + + foreach (string id in createdIds) + { + dynamic item = new + { + id = id, + pk = id, + }; + + await container.CreateItemAsync(item: item); + } + + Documents.IStoreModel storeModel = client.ClientContext.DocumentClient.StoreModel; + Mock mockStore = new Mock(); + client.ClientContext.DocumentClient.StoreModel = mockStore.Object; + + // Cause 429 after the first call + int callCount = 0; + string activityId = null; + string errorMessage = "Resource Not Found"; + mockStore.Setup(x => x.ProcessMessageAsync(It.IsAny(), It.IsAny())) + .Returns((dsr, token) => + { + callCount++; + + if (callCount > 1) + { + INameValueCollection headers = new DictionaryNameValueCollection(); + headers.Add(Documents.HttpConstants.HttpHeaders.RetryAfterInMilliseconds, "42"); + activityId = Guid.NewGuid().ToString(); + headers.Add(Documents.HttpConstants.HttpHeaders.ActivityId, activityId); + Documents.DocumentServiceResponse response = new Documents.DocumentServiceResponse( + body: TestCommon.GenerateStreamFromString(@"{""Errors"":[""" + errorMessage + @"""]}"), + headers: headers, + statusCode: (HttpStatusCode)429, + clientSideRequestStatistics: dsr.RequestContext.ClientRequestStatistics); + + return Task.FromResult(response); + } + + return storeModel.ProcessMessageAsync(dsr, token); + }); + + List results = new List(); + try + { + FeedIterator feedIterator = container.GetItemQueryIterator( + "select * from T where STARTSWITH(T.id, \"BasicQueryItem\")", + requestOptions: new QueryRequestOptions() + { + MaxItemCount = 1, + MaxConcurrency = 1 + }); + + while (feedIterator.HasMoreResults) + { + FeedResponse response = await feedIterator.ReadNextAsync(); + Assert.IsTrue(response.Count <= 1); + Assert.IsTrue(response.Resource.Count() <= 1); + + results.AddRange(response); + } + Assert.Fail("Should throw 429 exception after the first page."); + } + catch (CosmosException ce) + { + Assert.IsTrue(ce.RetryAfter.HasValue); + Assert.AreEqual(42, ce.RetryAfter.Value.TotalMilliseconds); + Assert.AreEqual(activityId, ce.ActivityId); + Assert.IsNotNull(ce.DiagnosticsContext); + Assert.IsTrue(ce.Message.Contains(errorMessage)); + } + + callCount = 0; + FeedIterator streamIterator = container.GetItemQueryStreamIterator( + "select * from T where STARTSWITH(T.id, \"BasicQueryItem\")", + requestOptions: new QueryRequestOptions() + { + MaxItemCount = 1, + MaxConcurrency = 1 + }); + + // First request should be a success + using (ResponseMessage response = await streamIterator.ReadNextAsync()) + { + response.EnsureSuccessStatusCode(); + Assert.IsNotNull(response.Content); + } + + // Second page should be a failure + using (ResponseMessage response = await streamIterator.ReadNextAsync()) + { + Assert.AreEqual(429, (int)response.StatusCode); + Assert.AreEqual("42", response.Headers.RetryAfterLiteral); + Assert.AreEqual(activityId, response.Headers.ActivityId); + Assert.IsNotNull(response.DiagnosticsContext); + Assert.IsTrue(response.ErrorMessage.Contains(errorMessage)); + } + } + [TestMethod] [DataRow(false)] [DataRow(true)] diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/Utils/TestCommon.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/Utils/TestCommon.cs index 657add2548..a41e1e80c6 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/Utils/TestCommon.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/Utils/TestCommon.cs @@ -53,6 +53,16 @@ static TestCommon() TestCommon.masterStalenessIntervalInSeconds = int.Parse(ConfigurationManager.AppSettings["MasterStalenessIntervalInSeconds"], CultureInfo.InvariantCulture); } + internal static MemoryStream GenerateStreamFromString(string s) + { + MemoryStream stream = new MemoryStream(); + StreamWriter writer = new StreamWriter(stream); + writer.Write(s); + writer.Flush(); + stream.Position = 0; + return stream; + } + internal static (string endpoint, string authKey) GetAccountInfo() { string authKey = ConfigurationManager.AppSettings["MasterKey"]; From e4b7efb6402a02b1fe73bbfb16d86688ee5a4934 Mon Sep 17 00:00:00 2001 From: Jake Willey Date: Fri, 6 Mar 2020 14:33:41 -0800 Subject: [PATCH 2/6] Updated changelog --- changelog.md | 1 + 1 file changed, 1 insertion(+) diff --git a/changelog.md b/changelog.md index 081537e20d..a3fe8eab95 100644 --- a/changelog.md +++ b/changelog.md @@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - [#1213](https://github.com/Azure/azure-cosmos-dotnet-v3/pull/1213) CosmosException now returns the original stack trace. - [#1213](https://github.com/Azure/azure-cosmos-dotnet-v3/pull/1213) ResponseMessage.ErrorMessage is now always correctly populated. There was bug in some scenarios where the error message was left in the content stream. - [#1242](https://github.com/Azure/azure-cosmos-dotnet-v3/pull/1242) Client encryption - Fix bug in read path without encrypted properties +- [#1263](https://github.com/Azure/azure-cosmos-dotnet-v3/pull/1263) Fix a bug where retry after internval did not get set on query stream responses ## [3.7.0-preview](https://www.nuget.org/packages/Microsoft.Azure.Cosmos/3.7.0-preview) - 2020-02-25 From 1bfd02f7626699dbfa786f5ee2ee4f34e3ecb46d Mon Sep 17 00:00:00 2001 From: Jake Willey Date: Mon, 16 Mar 2020 14:00:08 -0700 Subject: [PATCH 3/6] Removed using to force CI to run again. --- Microsoft.Azure.Cosmos/src/Query/v3Query/QueryIterator.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/Microsoft.Azure.Cosmos/src/Query/v3Query/QueryIterator.cs b/Microsoft.Azure.Cosmos/src/Query/v3Query/QueryIterator.cs index 67354b05ed..dfab88b85f 100644 --- a/Microsoft.Azure.Cosmos/src/Query/v3Query/QueryIterator.cs +++ b/Microsoft.Azure.Cosmos/src/Query/v3Query/QueryIterator.cs @@ -9,7 +9,6 @@ namespace Microsoft.Azure.Cosmos.Query using System.Threading.Tasks; using Microsoft.Azure.Cosmos.CosmosElements; using Microsoft.Azure.Cosmos.Diagnostics; - using Microsoft.Azure.Cosmos.Json; using Microsoft.Azure.Cosmos.Query.Core; using Microsoft.Azure.Cosmos.Query.Core.Exceptions; using Microsoft.Azure.Cosmos.Query.Core.ExecutionContext; From a5dd51b60cc2db6323af42e5086a3272f287e6d3 Mon Sep 17 00:00:00 2001 From: Jake Willey Date: Tue, 17 Mar 2020 06:01:30 -0700 Subject: [PATCH 4/6] Applied formatting to CosmosClient to force CI to run again. --- Microsoft.Azure.Cosmos/src/CosmosClient.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Microsoft.Azure.Cosmos/src/CosmosClient.cs b/Microsoft.Azure.Cosmos/src/CosmosClient.cs index a3c6e9749f..b666fb4995 100644 --- a/Microsoft.Azure.Cosmos/src/CosmosClient.cs +++ b/Microsoft.Azure.Cosmos/src/CosmosClient.cs @@ -831,8 +831,10 @@ private HttpClientHandler CreateHttpClientHandler(CosmosClientOptions clientOpti return null; } - HttpClientHandler httpClientHandler = new HttpClientHandler(); - httpClientHandler.Proxy = clientOptions.WebProxy; + HttpClientHandler httpClientHandler = new HttpClientHandler + { + Proxy = clientOptions.WebProxy + }; return httpClientHandler; } From d46f06d9f337dbedbe7433dbc607c72d9489b860 Mon Sep 17 00:00:00 2001 From: Jake Willey Date: Tue, 17 Mar 2020 07:01:23 -0700 Subject: [PATCH 5/6] Refactored throttle test to prevent conflicts with other tests --- .../CosmosBasicQueryTests.cs | 57 +++++++++++++------ 1 file changed, 39 insertions(+), 18 deletions(-) diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/CosmosBasicQueryTests.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/CosmosBasicQueryTests.cs index 07776a8fb6..3c72cc0c14 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/CosmosBasicQueryTests.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/CosmosBasicQueryTests.cs @@ -155,34 +155,55 @@ public async Task ContainerTest(bool directMode) [DataRow(true)] public async Task QueryRequestRateTest(bool directMode) { - CosmosClient client = directMode ? DirectCosmosClient : GatewayCosmosClient; - Container container = client.GetContainer(DatabaseId, ContainerId); - List createdIds = new List() - { - "BasicQueryItem" + Guid.NewGuid(), - "BasicQueryItem2"+ Guid.NewGuid(), - "BasicQueryItem3"+ Guid.NewGuid() - }; + string firstItemIdAndPk = "BasicQueryItem" + Guid.NewGuid(); - foreach (string id in createdIds) + // Prevent the test from changing the static client { - dynamic item = new + CosmosClient client = directMode ? DirectCosmosClient : GatewayCosmosClient; + Container container = client.GetContainer(DatabaseId, ContainerId); + + List createdIds = new List() { - id = id, - pk = id, + firstItemIdAndPk, + "BasicQueryItem2"+ Guid.NewGuid(), + "BasicQueryItem3"+ Guid.NewGuid() }; - await container.CreateItemAsync(item: item); + foreach (string id in createdIds) + { + dynamic item = new + { + id = id, + pk = id, + }; + + await container.CreateItemAsync(item: item); + } } - Documents.IStoreModel storeModel = client.ClientContext.DocumentClient.StoreModel; + CosmosClient clientWithThrottle; + if (directMode) + { + clientWithThrottle = TestCommon.CreateCosmosClient(); + } + else + { + clientWithThrottle = TestCommon.CreateCosmosClient((builder) => builder.WithConnectionModeGateway()); + } + + Container containerWithThrottle = clientWithThrottle.GetContainer(DatabaseId, ContainerId); + + // Do a read to warm up all the caches to prevent them from getting the throttle errors + using (await containerWithThrottle.ReadItemStreamAsync(firstItemIdAndPk, new PartitionKey(firstItemIdAndPk))) { } + + Documents.IStoreModel storeModel = clientWithThrottle.ClientContext.DocumentClient.StoreModel; Mock mockStore = new Mock(); - client.ClientContext.DocumentClient.StoreModel = mockStore.Object; + clientWithThrottle.ClientContext.DocumentClient.StoreModel = mockStore.Object; // Cause 429 after the first call int callCount = 0; string activityId = null; - string errorMessage = "Resource Not Found"; + string errorMessage = "QueryRequestRateTest Resource Not Found"; mockStore.Setup(x => x.ProcessMessageAsync(It.IsAny(), It.IsAny())) .Returns((dsr, token) => { @@ -209,7 +230,7 @@ public async Task QueryRequestRateTest(bool directMode) List results = new List(); try { - FeedIterator feedIterator = container.GetItemQueryIterator( + FeedIterator feedIterator = containerWithThrottle.GetItemQueryIterator( "select * from T where STARTSWITH(T.id, \"BasicQueryItem\")", requestOptions: new QueryRequestOptions() { @@ -237,7 +258,7 @@ public async Task QueryRequestRateTest(bool directMode) } callCount = 0; - FeedIterator streamIterator = container.GetItemQueryStreamIterator( + FeedIterator streamIterator = containerWithThrottle.GetItemQueryStreamIterator( "select * from T where STARTSWITH(T.id, \"BasicQueryItem\")", requestOptions: new QueryRequestOptions() { From 3a5aa56b8a7d49d46cec20a69876498b885fb62e Mon Sep 17 00:00:00 2001 From: Jake Willey Date: Tue, 17 Mar 2020 12:43:24 -0700 Subject: [PATCH 6/6] Fixed test to use a separate container to avoid conflicts when running tests in parallel. --- .../CosmosBasicQueryTests.cs | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/CosmosBasicQueryTests.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/CosmosBasicQueryTests.cs index 3c72cc0c14..bed29fd411 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/CosmosBasicQueryTests.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/CosmosBasicQueryTests.cs @@ -26,7 +26,7 @@ public class CosmosBasicQueryTests private static CosmosClient DirectCosmosClient; private static CosmosClient GatewayCosmosClient; private const string DatabaseId = "CosmosBasicQueryTests"; - private const string ContainerId = "ContainerBasicQueryTests"; + private static readonly string ContainerId = "ContainerBasicQueryTests" + Guid.NewGuid(); [ClassInitialize] public static async Task TestInit(TestContext textContext) @@ -290,7 +290,9 @@ public async Task QueryRequestRateTest(bool directMode) public async Task ItemTest(bool directMode) { CosmosClient client = directMode ? DirectCosmosClient : GatewayCosmosClient; - Container container = client.GetContainer(DatabaseId, ContainerId); + Database database = client.GetDatabase(DatabaseId); + Container container = await database.CreateContainerAsync(Guid.NewGuid().ToString(), "/pk"); + List createdIds = new List() { "BasicQueryItem", @@ -387,7 +389,9 @@ public async Task ItemTest(bool directMode) public async Task ScriptsStoredProcedureTest(bool directMode) { CosmosClient client = directMode ? DirectCosmosClient : GatewayCosmosClient; - Scripts scripts = client.GetContainer(DatabaseId, ContainerId).Scripts; + Database database = client.GetDatabase(DatabaseId); + Container container = await database.CreateContainerAsync(Guid.NewGuid().ToString(), "/pk"); + Scripts scripts = container.Scripts; List createdIds = new List() { @@ -439,7 +443,9 @@ public async Task ScriptsStoredProcedureTest(bool directMode) public async Task ScriptsUserDefinedFunctionTest(bool directMode) { CosmosClient client = directMode ? DirectCosmosClient : GatewayCosmosClient; - Scripts scripts = client.GetContainer(DatabaseId, ContainerId).Scripts; + Database database = client.GetDatabase(DatabaseId); + Container container = await database.CreateContainerAsync(Guid.NewGuid().ToString(), "/pk"); + Scripts scripts = container.Scripts; List createdIds = new List() { @@ -491,7 +497,9 @@ public async Task ScriptsUserDefinedFunctionTest(bool directMode) public async Task ScriptsTriggerTest(bool directMode) { CosmosClient client = directMode ? DirectCosmosClient : GatewayCosmosClient; - Scripts scripts = client.GetContainer(DatabaseId, ContainerId).Scripts; + Database database = client.GetDatabase(DatabaseId); + Container container = await database.CreateContainerAsync(Guid.NewGuid().ToString(), "/pk"); + Scripts scripts = container.Scripts; List createdIds = new List() {