Trying to parse non-json responses - 'U' is an invalid start of a value. LineNumber: 0

[mikocot]

When using persisted queries the server is expected to return BadRequest whenever hash isn't correct or wasn't registered yet.
Such responses are part of the process and come as plain text (html/text). This means they obviously can't be deserialized to any response type using neither System.Text.Json nor Newtonsoft, not even to an object.

At the same time the default IsValidResponseToDeserialize seems to be perfectly fine with passing such responses to the serializer.
I can understand originally this behavior was introduced to allow for parsing validation errors (#419), but I don't think it was meant to be as permissive (disregard content type).

A workaround is obviously to provide an alternative implementation, but I simply don't see a reasony why one would ever pass something that is not json-like to a json serializer, while the method is very precisely called: IsValidResponseToDeserialize. BadRequest html/text is neither valid nor possible to deserialize.

Current implementation:

    public Func<HttpResponseMessage, bool> IsValidResponseToDeserialize { get; set; } = r =>
        // Why not application/json? See https://github.com/graphql/graphql-over-http/blob/main/spec/GraphQLOverHTTP.md#processing-the-response
        r.IsSuccessStatusCode || r.StatusCode == HttpStatusCode.BadRequest || r.Content.Headers.ContentType?.MediaType == "application/graphql+json";

NOTE: this is just one of the cases, but such errors can be returned in many other situations.

[sungam3r]

Such responses are part of the process and come as plain text (html/text).

Could you please provide a link to specification describing that response format?

[mikocot]

Well, even the docs linked in the code indicate that:
https://github.com/graphql/graphql-over-http/blob/main/spec/GraphQLOverHTTP.md#processing-the-response

In our apollo server we get most of the errors as well formatter json and errors field, but there are some, e.g. related to incorrect query hash that come back as plain text. Possibly they are coming from some earlier layers?

[sungam3r]

The docs you provided indicate that status code should be 400. I do not see requirement that response should be plain text (html/text). On the contrary, in apollo docs I see

but I agree that Apollo docs do not say explicitly what HTTP code and content-type should be returned. In GraphQL.NET+Server APQ responses with error are retruned with HTTP code 400 (following spec requirement for operation that was not executed - ExecutionResult.Executed property) and one of application/graphql-response+json, application/graphql+json or application/json content types (no text/plain).

[mikocot]

yeah, they are not very specific, and as well the library is not restricted to use those particular and then it's even worse if you consider future changes to server behaviors, or clear bugs.

There is IMO just no reason to let serializer deserialize anything that isn't understandable by it. Especially the logic is already there to handle such errors. Now, the problem is that it is potentially a bit of a change to current users if for some reason they get responses that are not officially json but do parse correctly

[sungam3r]

So... I still do not understant what do you want. Make a suggestion.

[mikocot]

Well, it's simple, currently we check the result of IsValidResponseToDeserialize , if its true, we pass the response to serializer.
But IsValidResponseToDeserialize is true regardless of the ContentType in majority of cases, meaing its passing response to serializer even if its 100% sure the serializer is going to fail at parsing it

[sungam3r]

Ok, understand.