GetJsonAsync() returns null when the response content is not JSON

[matteocontrini]

It appears that when the content of a web page response is not a properly deserializable JSON, the GetJsonAsync() call returns null.

This line reproduces the issue:

var res = await "http://random.org".GetJsonAsync();

I expect the call to throw an exception in this case. The GetJsonAsync call should succeed only if the response content is actually a JSON payload.

I believe this happens because Newtonsoft.Json is returning null when the input is not parsable, but that's a bit weird because it should throw instead of failing silently...

[tmenier]

If the shape of a successful (200) response is unknown ahead of time, I would suggest using GetStringAsync or GetStreamAsync and do the parsing yourself. Even the non-generic GetJsonAsync won't be of much help here because dynamics are hard to work with when you don't know the shape.

If the expected shape differs only in error conditions (vast majority of cases where I've seen this come up), the try/catch pattern works well for this.

[matteocontrini]

What do you mean with "do the parsing yourself"? The problem is that a "manual" call to JsonConvert.DeserializeObject with a not properly formatted JSON string would also return null. I'm not sure why this happens (sometimes when the string is not JSON an exception is thrown, sometimes not)...

So what I'm saying is that when the deserialization returns null, the request should be considered as failed and an exception should be thrown by Flurl, because the response doesn't match the expected schema.

Currently the problem is that after calling GetStringAsync<T> I cannot rely on the fact that the result is a non-null T instance. It's not a big problem, as a null check doesn't complicate things particular, but it's not really ideal...

Is there something I'm missing that doesn't make this a possible solution? 😊

[tmenier]

The problem is that a "manual" call to JsonConvert.DeserializeObject with a not properly formatted JSON string would also return null.

Can you provide an example of this? I've never seen that.

[matteocontrini]

I've found that DeserializeObject() returns null when the input string is an empty string. And for some reason an HTTP request to http://random.org through Flurl returns an empty string. That's why the first example code above reproduces the issue.

I'm also pretty sure that there are other cases when deserialization returns null, but I can't reproduce anything now... I know that I've added a check for null in my application some days ago, because I was getting null sometimes.

Also, if you search for "DeserializeObject returns null" there are other people talking about this. There's also a very recent issue report on the Newtonsoft.Json GitHub repo.

[tmenier]

You're right about random.org, I tried that and got the same result. Not sure if it's an anti-screen-scraping thing or what, but both the browser and Postman return HTML content, whereas HttpClient returns a completely empty body.

There are 2 cases I know of where DeserializeObject (with default settings) returns null: empty string and the literal string null. Invalid JSON will throw, and Flurl's GetJsonAsync() follows suit in all cases.

If you're suggesting that Flurl should throw on GetJsonAsync() when the body is an empty string, I'm not willing to do that. A strong argument can be made that returning null there is valid and expected behavior, and changing it could break people's apps.

If you're suggesting there are other cases where you get null, where the body is neither empty nor null, you haven't demonstrated that. I don't believe there are such cases, and if I'm wrong you need to be able to demonstrate it with an example. The issue you linked to is isn't evidence of this at all, unless I missed something? He demonstrates with an empty string and {}; personally I get an empty object back in the latter case.

[matteocontrini]

If you're suggesting there are other cases where you get null, where the body is neither empty nor null, you haven't demonstrated that

I don't have any code that reproduces that situation right now. I don't actually remember how I got that result when I thought adding a null-check in my application was needed...

[tmenier]

If it's any consolation, #288 is a top priority for the next release. :)

[matteocontrini]

@tmenier that's great indeed, thank you 😉