From 1f9931b9ede2f870ae568f33e9406f506bbfd6a8 Mon Sep 17 00:00:00 2001 From: Neil Deshpande Date: Wed, 17 Aug 2022 16:41:28 -0700 Subject: [PATCH 1/4] Don't default to BadRequestException in case of errors in ServiceInterop --- .../Query/Core/ExceptionToCosmosException.cs | 10 +++-- .../Core/QueryPlan/QueryPlanRetriever.cs | 16 +++----- .../Query/QueryPlanRetrieverTests.cs | 37 +++++++++++++++++-- 3 files changed, 46 insertions(+), 17 deletions(-) diff --git a/Microsoft.Azure.Cosmos/src/Query/Core/ExceptionToCosmosException.cs b/Microsoft.Azure.Cosmos/src/Query/Core/ExceptionToCosmosException.cs index 966c603f5d..c6dcce7a67 100644 --- a/Microsoft.Azure.Cosmos/src/Query/Core/ExceptionToCosmosException.cs +++ b/Microsoft.Azure.Cosmos/src/Query/Core/ExceptionToCosmosException.cs @@ -76,18 +76,20 @@ private static bool TryCreateFromExceptionWithStackTrace( ITrace trace, out CosmosException cosmosException) { + Exception innerException = ExceptionWithStackTraceException.UnWrapMonadExcepion(exceptionWithStackTrace, trace); + // Use the original stack trace from the inner exception. - if (exceptionWithStackTrace.InnerException is Microsoft.Azure.Documents.DocumentClientException - || exceptionWithStackTrace.InnerException is CosmosException) + if (innerException is Microsoft.Azure.Documents.DocumentClientException + || innerException is CosmosException) { return ExceptionToCosmosException.TryCreateFromException( - exceptionWithStackTrace.InnerException, + innerException, trace, out cosmosException); } if (!ExceptionToCosmosException.TryCreateFromException( - exceptionWithStackTrace.InnerException, + innerException, trace, out cosmosException)) { diff --git a/Microsoft.Azure.Cosmos/src/Query/Core/QueryPlan/QueryPlanRetriever.cs b/Microsoft.Azure.Cosmos/src/Query/Core/QueryPlan/QueryPlanRetriever.cs index 32e061dfd8..09ad98f395 100644 --- a/Microsoft.Azure.Cosmos/src/Query/Core/QueryPlan/QueryPlanRetriever.cs +++ b/Microsoft.Azure.Cosmos/src/Query/Core/QueryPlan/QueryPlanRetriever.cs @@ -76,18 +76,14 @@ public static async Task GetQueryPlanWithServiceI if (!tryGetQueryPlan.Succeeded) { - Exception originalException = ExceptionWithStackTraceException.UnWrapMonadExcepion(tryGetQueryPlan.Exception, serviceInteropTrace); - if (originalException is CosmosException) + if (ExceptionToCosmosException.TryCreateFromException(tryGetQueryPlan.Exception, serviceInteropTrace, out CosmosException cosmosException)) { - throw originalException; + throw cosmosException; + } + else + { + throw ExceptionWithStackTraceException.UnWrapMonadExcepion(tryGetQueryPlan.Exception, serviceInteropTrace); } - - throw CosmosExceptionFactory.CreateBadRequestException( - message: originalException.Message, - headers: new Headers(), - stackTrace: tryGetQueryPlan.Exception.StackTrace, - innerException: originalException, - trace: trace); } return tryGetQueryPlan.Result; diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Query/QueryPlanRetrieverTests.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Query/QueryPlanRetrieverTests.cs index 47a681eb1a..e84084a91a 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Query/QueryPlanRetrieverTests.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Query/QueryPlanRetrieverTests.cs @@ -41,7 +41,6 @@ public async Task ServiceInterop_BadRequestContainsInnerException() It.IsAny(), It.IsAny())).ReturnsAsync(TryCatch.FromException(innerException)); - Mock trace = new Mock(); CosmosException cosmosException = await Assert.ThrowsExceptionAsync(() => QueryPlanRetriever.GetQueryPlanWithServiceInteropAsync( queryClient.Object, new SqlQuerySpec("selectttttt * from c"), @@ -50,8 +49,7 @@ public async Task ServiceInterop_BadRequestContainsInnerException() hasLogicalPartitionKey: false, geospatialType: Cosmos.GeospatialType.Geography, useSystemPrefix: false, - trace.Object, - default)); + NoOpTrace.Singleton)); Assert.AreEqual(HttpStatusCode.BadRequest, cosmosException.StatusCode); Assert.AreEqual(innerException, cosmosException.InnerException); @@ -92,5 +90,38 @@ public async Task ServiceInterop_BadRequestContainsOriginalCosmosException() Assert.AreEqual(expectedException, cosmosException); } + + [TestMethod] + public async Task ServiceInterop_E_UNEXPECTED() + { + UnexpectedQueryPartitionProviderException innerException = new UnexpectedQueryPartitionProviderException("E_UNEXPECTED"); + Mock queryClient = new Mock(); + + queryClient.Setup(c => c.TryGetPartitionedQueryExecutionInfoAsync( + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny())).ReturnsAsync(TryCatch.FromException(innerException)); + + CosmosException cosmosException = await Assert.ThrowsExceptionAsync(() => QueryPlanRetriever.GetQueryPlanWithServiceInteropAsync( + queryClient.Object, + new SqlQuerySpec("Super secret query that triggers bug"), + ResourceType.Document, + new Documents.PartitionKeyDefinition() { Paths = new Collection() { "/id" } }, + hasLogicalPartitionKey: false, + useSystemPrefix: false, + NoOpTrace.Singleton)); + + Assert.AreEqual(HttpStatusCode.InternalServerError, cosmosException.StatusCode); + Assert.AreEqual(innerException, cosmosException.InnerException); + Assert.IsNotNull(cosmosException.Trace); + Assert.IsNotNull(cosmosException.Diagnostics); + } } } From 16739245dc5612bd145b4ef1d287be831beec7fe Mon Sep 17 00:00:00 2001 From: Neil Deshpande Date: Tue, 15 Nov 2022 13:02:07 -0800 Subject: [PATCH 2/4] Incorporate code review feedback --- .../src/Query/Core/ExceptionToCosmosException.cs | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/Microsoft.Azure.Cosmos/src/Query/Core/ExceptionToCosmosException.cs b/Microsoft.Azure.Cosmos/src/Query/Core/ExceptionToCosmosException.cs index c6dcce7a67..0bf9497a6f 100644 --- a/Microsoft.Azure.Cosmos/src/Query/Core/ExceptionToCosmosException.cs +++ b/Microsoft.Azure.Cosmos/src/Query/Core/ExceptionToCosmosException.cs @@ -78,16 +78,6 @@ private static bool TryCreateFromExceptionWithStackTrace( { Exception innerException = ExceptionWithStackTraceException.UnWrapMonadExcepion(exceptionWithStackTrace, trace); - // Use the original stack trace from the inner exception. - if (innerException is Microsoft.Azure.Documents.DocumentClientException - || innerException is CosmosException) - { - return ExceptionToCosmosException.TryCreateFromException( - innerException, - trace, - out cosmosException); - } - if (!ExceptionToCosmosException.TryCreateFromException( innerException, trace, From e073e6dc01c377dc9e04d950c5b25e3fc368c9b0 Mon Sep 17 00:00:00 2001 From: Neil Deshpande Date: Tue, 15 Nov 2022 13:44:19 -0800 Subject: [PATCH 3/4] Fix build error --- .../Query/QueryPlanRetrieverTests.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Query/QueryPlanRetrieverTests.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Query/QueryPlanRetrieverTests.cs index e84084a91a..c1dc2fc3ae 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Query/QueryPlanRetrieverTests.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Query/QueryPlanRetrieverTests.cs @@ -107,6 +107,7 @@ public async Task ServiceInterop_E_UNEXPECTED() It.IsAny(), It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny())).ReturnsAsync(TryCatch.FromException(innerException)); CosmosException cosmosException = await Assert.ThrowsExceptionAsync(() => QueryPlanRetriever.GetQueryPlanWithServiceInteropAsync( @@ -115,6 +116,7 @@ public async Task ServiceInterop_E_UNEXPECTED() ResourceType.Document, new Documents.PartitionKeyDefinition() { Paths = new Collection() { "/id" } }, hasLogicalPartitionKey: false, + geospatialType: Cosmos.GeospatialType.Geography, useSystemPrefix: false, NoOpTrace.Singleton)); From 25bdb17482e581f553c09dbdfce8882c235e730f Mon Sep 17 00:00:00 2001 From: Neil Deshpande Date: Wed, 16 Nov 2022 13:33:45 -0800 Subject: [PATCH 4/4] fix up failing test --- .../Query/Core/ExceptionToCosmosException.cs | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/Microsoft.Azure.Cosmos/src/Query/Core/ExceptionToCosmosException.cs b/Microsoft.Azure.Cosmos/src/Query/Core/ExceptionToCosmosException.cs index 0bf9497a6f..02c1882e26 100644 --- a/Microsoft.Azure.Cosmos/src/Query/Core/ExceptionToCosmosException.cs +++ b/Microsoft.Azure.Cosmos/src/Query/Core/ExceptionToCosmosException.cs @@ -86,14 +86,18 @@ private static bool TryCreateFromExceptionWithStackTrace( return false; } - cosmosException = CosmosExceptionFactory.Create( - cosmosException.StatusCode, - cosmosException.Message, - exceptionWithStackTrace.StackTrace, - headers: cosmosException.Headers, - cosmosException.Trace, - cosmosException.Error, - cosmosException.InnerException); + if (innerException is not CosmosException && innerException is not Documents.DocumentClientException) + { + cosmosException = CosmosExceptionFactory.Create( + cosmosException.StatusCode, + cosmosException.Message, + exceptionWithStackTrace.StackTrace, + headers: cosmosException.Headers, + cosmosException.Trace, + cosmosException.Error, + cosmosException.InnerException); + } + return true; }