Handle invalid JSON HTTP response from Gateway for error scenarios

[ealsur]

We have received multiple reports of cases where the SDK throws a JSON serialization exception when processing HTTP responses from the Cosmos DB Gateway, for example:

Exception: Newtonsoft.Json.JsonReaderException: Error reading JObject from JsonReader. Path '', line 0, position 0.   
  at Newtonsoft.Json.Linq.JObject.Load(JsonReader reader, JsonLoadSettings settings)    
  at Microsoft.Azure.Documents.JsonSerializable.LoadFrom(JsonReader reader, JsonSerializerSettings serializerSettings)    
  at Microsoft.Azure.Documents.JsonSerializable.LoadFrom[T](JsonTextReader jsonReader, ITypeResolver`1 typeResolver, JsonSerializerSettings settings)    
  at Microsoft.Azure.Cosmos.GatewayStoreClient.<CreateDocumentClientExceptionAsync>d__11.MoveNext() --- End of stack trace from previous location where exception was thrown ---    
  at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()    
  at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)    
  at System.Runtime.CompilerServices.TaskAwaiter`1.GetResult()    
  at Microsoft.Azure.Cosmos.GatewayStoreClient.<ParseResponseAsync>d__9.MoveNext()

Microsoft.Azure.Cosmos.GatewayStoreClient.<ParseResponseAsync> hints as this being an HTTP response from Gateway.

Microsoft.Azure.Cosmos.GatewayStoreClient.<CreateDocumentClientExceptionAsync> means the statuscode of the HTTP response was a failure (not 200 or any success).

Which means, Gateway returned a failure but the content was invalid.

SDK checks the Content-Type header and if it's application/json then expects the content to be valid JSON:

azure-cosmos-dotnet-v3/Microsoft.Azure.Cosmos/src/GatewayStoreClient.cs

Lines 151 to 154 in 083df87

	if (string.Equals(responseMessage.Content?.Headers?.ContentType?.MediaType, "application/json", StringComparison.OrdinalIgnoreCase))
	{
	Stream readStream = await responseMessage.Content.ReadAsStreamAsync();
	Error error = Documents.Resource.LoadFrom<Error>(readStream);

In all these cases what happened was that the content was either empty or invalid JSON.

To avoid the exception in the most common case (empty), we should check the Content-Size of the response, and if it's 0, either throw an exception saying that the response is invalid because it's empty or avoid attempting the JSON deserialization of the body and just produce the error response using the statuscode as if it had no content.

Expected outcome

• If the Content-Size header is application/json but the content size is 0, skip attempting to deserialize the content
• If the Content-Size header is application/json but the content size > 0, attempt to deserialize the content. If that fails, follow the code path that treats the content as text (non-JSON path).

[sourabh1007]

Doesn't it service responsibility to define a standard format of response in case of error which any SDK can understand and use (or log) the information accordingly.
Right now, it looks like, service sends response in whatever format they want, and SDK is trying to handle all the scenarios.

[ealsur]

Agree that the service should behave correctly. But this change would avoid the conversation be "The SDK is throwing an exception, it's probably a problem in the SDK" which is what happened in all cases this was reported. We then found out that it was never the SDK problem. Making this change would surface the actional information that is required to course the investigation correctly.

[philipthomas-MSFT]

I am able to repro this in branch. Will work through suggested solutions.

[philipthomas-MSFT]

@ealsur

Clarification:

• What HttpStatus codes are normally associate with CreateDocumentClientExceptionAsync? I understand that we think that there will never be a 200 with a CreateDocumentClientExceptionAsync. How can we verify this?
• When you say "Content-Size" you really mean ContentLength, correct? Or is there a header key called "Content-Size" that has a different behavior and function as ContentLength?

  • azure-cosmos-dotnet-v3/Microsoft.Azure.Cosmos/src/Headers/Headers.cs

    Line 120 in 5a096d1

    public virtual string ContentLength

• Not seeing a custom exception for invalid responses, unless we reuse DocumentClientException. I would like to create a new one, but want to make sure I am not duplicating efforts.
  • When throwing this new exception or using DocumentClientException, we should inherit the HttpStatus code from the failed response, but will it cause any side effects (ie. retry policies, timeouts, etc.)?
• Should we return the failed response ResourceAddress, RequestStatistics, all other Header information, etc.? Why and what is the impact?
• What about scenarios when the Content has whitespaces? The ContentLength will not be 0. So checking ContentLength alone will not bullet proof this.

Testing

Testing for empty and whitespaces.

[philipthomas-MSFT]

Blocked while waiting on approvals and feedback.