HttpClient null warning in C# 9+

Question

I've just migrated an .NET3.1 application to .NET6 (I am using VS 2022) and I noticed that they've introduced some constraints related to nullable types. I fixed all my code to follow best practices, but not sure how to go about this issue:

When creating a new HttpClient instance it returns a nullable HttpClient type (code is executed inside a Polly retry policy):

   public async Task<List<InterestedParty>> GetInterestedPartiesAsync()
    {
        return await _retryPolicy.ExecuteAsync(async () =>
        {
            using var client = new HttpClient()
            const string endpoint = "my-endpoint";
            var url = $"{endpoint}/test";

            var response = await client.GetAsync(url);
            response.EnsureSuccessStatusCode();

            var jsonString = await response.Content.ReadAsStringAsync();
            return JsonConvert.DeserializeObject<List<InterestedParty>>(jsonString);
        });
    }

The compiler is complaining about a possible null reference return. I know I could suppress the warning by using:

#pragma warning disable CS8603 // Possible null reference return.

but I don't think it's the best solution.

Any other suggestions? Thank you.

What if you explicitly write: `using (HttpClient client = new HttpClient())` or shorter: `using (HttpClient client = new())` — Jeroen van Langen, Dec 14 '21 at 10:36
What is your method actually *returning*? Not the `HttpClient`, I take it. The warning, as taken literally, is nonsense of course -- `new` will not return `null`. I suspect that's not what the warning is about, though (look at all the rest that's also squigglied). Ignore the type hint over the `var`, that's a separate matter. — Jeroen Mostert, Dec 14 '21 at 10:39
Show us the full code please, screenshots of code are mostly useless. Also, make sure you include enough context so that we can replicate it. Basically we need a [mre] — DavidG, Dec 14 '21 at 10:45
@JeroenMostert you are right! My method returns Task> but the problem is on the returned object. I am returning a deserialized object which in theory may be null, that's the root cause. thanks for pointing me in the right direction — Dragos Stoica, Dec 14 '21 at 10:46
Other thing though - you really shouldn't create HttpClient on every request. Proper way to handle HttpClient is to store single static reference or use HttpClientFactory. I really cannot stress enough how important that is. More details: https://www.aspnetmonsters.com/2016/08/2016-08-27-httpclientwrong/ — Jakoss, Dec 14 '21 at 11:04
@Jakoss, totally agree, but in my specific case, this is a Lambda Function running this code and it is the only external call. No need to inject HttpClient as a singleton or use the factory. — Dragos Stoica, Dec 14 '21 at 12:32
@DragosStoica is the only external call but how many times is this call performed? surely you don't want to create a new instance every time, that'll hinder performance and you might encounter a few bugs along the way too since the http client is not made to be used in such a way. — Bargros, Dec 14 '21 at 12:55
@Bargos thanks for your comment, it's used only once when initializing the Lambda function in order to gather more info. Again, this is an edge case where I think it should be OK with having the client instantiated inline. — Dragos Stoica, Dec 14 '21 at 12:58
@DragosStoica I don't think any edge case is OK with that. Using static class with single HttpClient is very cheap and easy. You are doing this in inside retry policy, so only by this logic you can create multiple http clients. It's even more funny that you don't have to dispose HttpClient: https://stackoverflow.com/questions/15705092/do-httpclient-and-httpclienthandler-have-to-be-disposed-between-requests#:~:text=The%20general%20consensus%20is%20that,in%20memory%20leak%20for%20reference. — Jakoss, Dec 14 '21 at 14:23
hmm, actually I think you're right, I'll do the change, thanks for clarifications. — Dragos Stoica, Dec 14 '21 at 14:38

Dragos Stoica · Accepted Answer · 2021-12-14T12:38:52.957

The issue itself it's not related to HttpClient as I thought in the beginning.

The return type of the method it's Task<List> and inside the using statement I was returning

JsonConvert.DeserializeObject<List<InterestedParty>>(jsonString); Indeed, the deserialized object can be null, so one solution would be add the "null forgiving" operator (!) at the end:

   public async Task<List<InterestedParty>> GetInterestedPartiesAsync()
    {
        return await _retryPolicy.ExecuteAsync(async () =>
        {
            using var client = new HttpClient();
            const string endpoint = "my-endpoint";
            var url = $"{endpoint}/test";

            var response = await client.GetAsync(url);
            response.EnsureSuccessStatusCode();

            var jsonString = await response.Content.ReadAsStringAsync();
            return JsonConvert.DeserializeObject<List<InterestedParty>>(jsonString)!;
        });
    }

Another better suggestion would be :

  public async Task<IEnumerable<InterestedParty>> GetInterestedPartiesAsync()
{
    return await _retryPolicy.ExecuteAsync(async () =>
    {
        using var client = new HttpClient();
        const string endpoint = "my-endpoint";
        var url = $"{endpoint}/test";

        var response = await client.GetAsync(url);
        response.EnsureSuccessStatusCode();

        var jsonString = await response.Content.ReadAsStringAsync();
        return JsonConvert.DeserializeObject<List<InterestedParty>>(jsonString) ??
               Enumerable.Empty<InterestedParty>();
    });
}

Thanks @JeroenMostert and @DavidG for your suggestion.

I don't think this is the proper way to fix things. `Null forgiving` operator should be used rarely, almost never - and it is not made to fix these issues. Better way would be to return `Task?>` — Archeg, Dec 14 '21 at 11:04
Or, depending on your data model, return `?? new List()`; empty collections are generally better than `null`, since they require less code to handle the exceptional situation (unless there is a business requirement to make the distinction, of course). — Jeroen Mostert, Dec 14 '21 at 11:07
Yes, @JeroenMostert is right, `?? new List()` even more appropriate in many situations — Archeg, Dec 14 '21 at 11:08
@JeroenMostert would you mind adding an answer? I'd like to approve your answer as the correct one. — Dragos Stoica, Dec 14 '21 at 11:09
Or even better, return `IEnumerable` and use `?? Enumerable.Empty()` since that won't create a new List and is cached. — DavidG, Dec 14 '21 at 11:09

score 1 · Answer 2 · answered Dec 14 '21 at 18:21

1

In .NET 6 in .csproj you could set nullable disable:

<PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net6.0</TargetFramework>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>disable</Nullable>
</PropertyGroup>

answered Dec 14 '21 at 18:21

ggeorge

1,496
2
13
19

HttpClient null warning in C# 9+

2 Answers2