Hotchocolate: Serialization exception on remote schema error

[nigel-sampson]

Describe the bug
As part of introducing test seams into our application we've added a delegating handler to the http clients used to communicate with remote schemas. This allows us to simulate them being unavailable. The core code for this handler looks like the following:

if (IsServiceUnavailable(request)) {

    return new HttpResponseMessage {
        StatusCode = HttpStatusCode.ServiceUnavailable,
        Content = new StringContent(""),
        ReasonPhrase = $"Service '{_schemaName}' unavailable due to test configuration"
    };
} else {
    return await base.SendAsync(request, cancellationToken);
}

When the unavailable response is triggered we see the initiating request to the the stitched schema fail with a 500 error.

The stack trace in the logs includes the following:

Newtonsoft.Json.JsonSerializationException: Self referencing loop detected for property 'ManifestModule' with type 'System.Reflection.RuntimeModule'. Path 'errors[0].extensions.remote.Exception.TargetSite.Module.Assembly'.   
at bool Newtonsoft.Json.Serialization.JsonSerializerInternalWriter.CheckForCircularReference(JsonWriter writer, object value, JsonProperty property, JsonContract contract, JsonContainerContract containerContract, JsonProperty containerProperty)
at bool Newtonsoft.Json.Serialization.JsonSerializerInternalWriter.CalculatePropertyValues(JsonWriter writer, object value, JsonContainerContract contract, JsonProperty member, JsonProperty property, out JsonContract memberContract, out object memberValue)  
at void Newtonsoft.Json.Serialization.JsonSerializerInternalWriter.SerializeObject(JsonWriter writer, object value, JsonObjectContract contract, JsonProperty member, JsonContainerContract collectionContract, JsonProperty containerProperty)  
at void Newtonsoft.Json.Serialization.JsonSerializerInternalWriter.SerializeObject(JsonWriter writer, object value, JsonObjectContract contract, JsonProperty member, JsonContainerContract collectionContract, JsonProperty containerProperty)
at void Newtonsoft.Json.Serialization.JsonSerializerInternalWriter.SerializeObject(JsonWriter writer, object value, JsonObjectContract contract, JsonProperty member, JsonContainerContract collectionContract, JsonProperty containerProperty)
at void Newtonsoft.Json.Serialization.JsonSerializerInternalWriter.SerializeObject(JsonWriter writer, object value, JsonObjectContract contract, JsonProperty member, JsonContainerContract collectionContract, JsonProperty containerProperty)
at void Newtonsoft.Json.Serialization.JsonSerializerInternalWriter.SerializeObject(JsonWriter writer, object value, JsonObjectContract contract, JsonProperty member, JsonContainerContract collectionContract, JsonProperty containerProperty)
at void Newtonsoft.Json.Serialization.JsonSerializerInternalWriter.SerializeDictionary(JsonWriter writer, IDictionary values, JsonDictionaryContract contract, JsonProperty member, JsonContainerContract collectionContract, JsonProperty containerProperty)
at void Newtonsoft.Json.Serialization.JsonSerializerInternalWriter.SerializeDictionary(JsonWriter writer, IDictionary values, JsonDictionaryContract contract, JsonProperty member, JsonContainerContract collectionContract, JsonProperty containerProperty) 
at void Newtonsoft.Json.Serialization.JsonSerializerInternalWriter.SerializeList(JsonWriter writer, IEnumerable values, JsonArrayContract contract, JsonProperty member, JsonContainerContract collectionContract, JsonProperty containerProperty)
at void Newtonsoft.Json.Serialization.JsonSerializerInternalWriter.SerializeDictionary(JsonWriter writer, IDictionary values, JsonDictionaryContract contract, JsonProperty member, JsonContainerContract collectionContract, JsonProperty containerProperty)
at void Newtonsoft.Json.Serialization.JsonSerializerInternalWriter.Serialize(JsonWriter jsonWriter, object value, Type objectType)  at void Newtonsoft.Json.JsonSerializer.SerializeInternal(JsonWriter jsonWriter, object value, Type objectType)
at string Newtonsoft.Json.JsonConvert.SerializeObjectInternal(object value, Type type, JsonSerializer jsonSerializer)
at async Task HotChocolate.Execution.JsonQueryResultSerializer.SerializeAsync(IReadOnlyQueryResult result, Stream stream) in C:/hc/src/Core/Core/Execution/JsonQueryResultSerializer.cs:line 38 
at async Task HotChocolate.AspNetCore.QueryMiddlewareBase.WriteResponseAsync(HttpResponse response, IExecutionResult executionResult) in C:/hc/src/Server/AspNetCore/QueryMiddlewareBase.cs:line 221
at async Task HotChocolate.AspNetCore.QueryMiddlewareBase.HandleRequestAsync(HttpContext context, IQueryExecutor queryExecutor) in C:/hc/src/Server/AspNetCore/QueryMiddlewareBase.cs:line 208
at async Task HotChocolate.AspNetCore.QueryMiddlewareBase.InvokeAsync(HttpContext context) in C:/hc/src/Server/AspNetCore/QueryMiddlewareBase.cs:line 117

Expected behavior
I would expect the stitched schema to return a 200 response with the details of the unavailable remote schema included in the errors collection and the results of any other remote schema to be returned.

Additional context
This is with Hot Chocolate 0.8.1.

[nigel-sampson]

I haven't been able to recreate the issue in your unit tests yet but I've found that adding an error filter that clears the extensions works around this.

.AddExecutionConfiguration(excution =>
{
    excution.AddErrorFilter(error => {
        return error.Exception is Exception ex
            ? ErrorBuilder.FromError(error).ClearExtensions().Build()
            : error;
    });
})

On a related note, it seems that we're potentially leaking exception information in the extension.remote.Exception. path of this exception such as stack trace.

[nigel-sampson]

To expand on the last point, I've been messing around with the ErrorBehaviour.ConnectionLost, if I remove the error filter then the snapshot looks the following which is concerning.

{
  "Data": {
    "createCustomer": null
  },
  "Extensions": {},
  "Errors": [
    {
      "Message": "Unexpected Execution Error",
      "Code": null,
      "Path": null,
      "Locations": [],
      "Exception": {
        "Message": "No connection could be made because the target machine actively refused it",
        "Data": {},
        "InnerException": {
          "ClassName": "System.Net.Sockets.SocketException",
          "Message": "No connection could be made because the target machine actively refused it",
          "Data": null,
          "InnerException": null,
          "HelpURL": null,
          "StackTraceString": "   at System.Net.Http.ConnectHelper.ConnectAsync(String host, Int32 port, CancellationToken cancellationToken)",
          "RemoteStackTraceString": null,
          "RemoteStackIndex": 0,
          "ExceptionMethod": null,
          "HResult": -2147467259,
          "Source": "System.Private.CoreLib",
          "WatsonBuckets": null,
          "NativeErrorCode": 10061
        },
        "StackTrace": "   at System.Net.Http.ConnectHelper.ConnectAsync(String host, Int32 port, CancellationToken cancellationToken)\n   at System.Threading.Tasks.ValueTask`1.get_Result()\n   at System.Net.Http.HttpConnectionPool.CreateConnectionAsync(HttpRequestMessage request, CancellationToken cancellationToken)\n   at System.Threading.Tasks.ValueTask`1.get_Result()\n   at System.Net.Http.HttpConnectionPool.WaitForCreatedConnectionAsync(ValueTask`1 creationTask)\n   at System.Threading.Tasks.ValueTask`1.get_Result()\n   at System.Net.Http.HttpConnectionPool.SendWithRetryAsync(HttpRequestMessage request, Boolean doRequestAuth, CancellationToken cancellationToken)\n   at System.Net.Http.RedirectHandler.SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)\n   at System.Net.Http.HttpClient.FinishSendAsyncBuffered(Task`1 sendTask, HttpRequestMessage request, CancellationTokenSource cts, Boolean disposeCts)\n   at HotChocolate.Stitching.Utilities.HttpQueryClient.FetchStringInternalAsync(HttpQueryRequest request, HttpClient httpClient) in C:\\Dev\\GitHub\\hotchocolate\\src\\Stitching\\Stitching\\Utilities\\HttpQueryClient.cs:line 64\n   at HotChocolate.Stitching.Utilities.HttpQueryClient.FetchAsync(HttpQueryRequest request, HttpClient httpClient) in C:\\Dev\\GitHub\\hotchocolate\\src\\Stitching\\Stitching\\Utilities\\HttpQueryClient.cs:line 31\n   at HotChocolate.Stitching.Delegation.RemoteQueryMiddleware.InvokeAsync(IQueryContext context) in C:\\Dev\\GitHub\\hotchocolate\\src\\Stitching\\Stitching\\Delegation\\RemoteQueryMiddleware.cs:line 35\n   at HotChocolate.Execution.ExceptionMiddleware.InvokeAsync(IQueryContext context) in C:\\Dev\\GitHub\\hotchocolate\\src\\Core\\Core\\Execution\\Middleware\\ExceptionMiddleware.cs:line 26",
        "HelpLink": null,
        "Source": "System.Net.Http",
        "HResult": -2147467259
      },
      "Extensions": {
        "remote": {
          "Message": "Unexpected Execution Error",
          "Code": null,
          "Path": null,
          "Locations": [],
          "Exception": {
            "Message": "No connection could be made because the target machine actively refused it",
            "Data": {},
            "InnerException": {
              "ClassName": "System.Net.Sockets.SocketException",
              "Message": "No connection could be made because the target machine actively refused it",
              "Data": null,
              "InnerException": null,
              "HelpURL": null,
              "StackTraceString": "   at System.Net.Http.ConnectHelper.ConnectAsync(String host, Int32 port, CancellationToken cancellationToken)",
              "RemoteStackTraceString": null,
              "RemoteStackIndex": 0,
              "ExceptionMethod": null,
              "HResult": -2147467259,
              "Source": "System.Private.CoreLib",
              "WatsonBuckets": null,
              "NativeErrorCode": 10061
            },
            "StackTrace": "   at System.Net.Http.ConnectHelper.ConnectAsync(String host, Int32 port, CancellationToken cancellationToken)\n   at System.Threading.Tasks.ValueTask`1.get_Result()\n   at System.Net.Http.HttpConnectionPool.CreateConnectionAsync(HttpRequestMessage request, CancellationToken cancellationToken)\n   at System.Threading.Tasks.ValueTask`1.get_Result()\n   at System.Net.Http.HttpConnectionPool.WaitForCreatedConnectionAsync(ValueTask`1 creationTask)\n   at System.Threading.Tasks.ValueTask`1.get_Result()\n   at System.Net.Http.HttpConnectionPool.SendWithRetryAsync(HttpRequestMessage request, Boolean doRequestAuth, CancellationToken cancellationToken)\n   at System.Net.Http.RedirectHandler.SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)\n   at System.Net.Http.HttpClient.FinishSendAsyncBuffered(Task`1 sendTask, HttpRequestMessage request, CancellationTokenSource cts, Boolean disposeCts)\n   at HotChocolate.Stitching.Utilities.HttpQueryClient.FetchStringInternalAsync(HttpQueryRequest request, HttpClient httpClient) in C:\\Dev\\GitHub\\hotchocolate\\src\\Stitching\\Stitching\\Utilities\\HttpQueryClient.cs:line 64\n   at HotChocolate.Stitching.Utilities.HttpQueryClient.FetchAsync(HttpQueryRequest request, HttpClient httpClient) in C:\\Dev\\GitHub\\hotchocolate\\src\\Stitching\\Stitching\\Utilities\\HttpQueryClient.cs:line 31\n   at HotChocolate.Stitching.Delegation.RemoteQueryMiddleware.InvokeAsync(IQueryContext context) in C:\\Dev\\GitHub\\hotchocolate\\src\\Stitching\\Stitching\\Delegation\\RemoteQueryMiddleware.cs:line 35\n   at HotChocolate.Execution.ExceptionMiddleware.InvokeAsync(IQueryContext context) in C:\\Dev\\GitHub\\hotchocolate\\src\\Core\\Core\\Execution\\Middleware\\ExceptionMiddleware.cs:line 26",
            "HelpLink": null,
            "Source": "System.Net.Http",
            "HResult": -2147467259
          },
          "Extensions": {}
        }
      }
    }
  ]
}

[michaelstaib]

That looks kind of ok. The exception property is only serialized in the snapshot but not on the result serialization.

The thing that should be different are those guys

"Code": null,
"Path": null,
"Locations": [],

The should be set to the field that is causing this issue.

If you look at the result:

"Data": {
"createCustomer": null
}

It seems kind of ok here since we could not reach the backend remote services we will set the result to null. This behavior is spaced.

Also we will raise an "Unexpected Execution Error". We do not want to expose the backend exception to the outside world so the actual exception is removed when the result is serialized. I also have a test for version 9 now and I have working most of it correctly now.

When I have some time I will port it to the version 8 branch and make a build. We are I think still two weeks out of releasing version 9.

[michaelstaib]

After looking at it ... I see the error now. The problem is that we are not removing the error from the error in the extensions. That is why your clear extension fix remedies this issue.

[michaelstaib]

this one is now fixed with #983