IDaprSidecarHttpClientFactory.CreateInvokeHttpClient created HttpClient throws exception

helmerm commented 2 years ago

Expected Behavior

Perform service invocation via HTTP client created via IDaprSidecarHttpClientFactory.CreateInvokeHttpClient(appid);

Actual Behavior

System.NullReferenceException: Object reference not set to an instance of an object. at Man.Dapr.Sidekick.Http.HttpContextInvocationHandler.OnAfterSendAsync(HttpRequestMessage request, CancellationToken cancellationToken) at Man.Dapr.Sidekick.DaprClient.InvocationHandler.d__10.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)

Steps to Reproduce the Problem

Dapr 1.4 sidekick 1.2

                var daprClient = componentRegistration.ComponentHost.Services
                        .GetRequiredService<IDaprSidecarHttpClientFactory>();
                var appid = componentRegistration.DaprSidecarHost.GetProcessOptions().AppId;
                Debug.Assert(!string.IsNullOrEmpty(appid));
                using var http = daprClient.CreateInvokeHttpClient(appid);
                Debug.Assert(http != null);
                // 🧨🧨🧨
                var result = await http.GetAsync("testMethod");

Release Note

RELEASE NOTE: FIX Bug in IDaprSidecarHttpClientFactory.CreateInvokeHttpClient

badgeratu commented 2 years ago

Hi, can you please provide a complete solution/project on GitHub that we can use to investigate this?

helmerm commented 2 years ago

I am using this inside my integration test setup, so it is hard to reproduce the same setup. For that I run multiple sidecars inside the same process and orchestrate some setup logic from my test setup.

After looking more into the code, I see that my understanding of how the DaprPort is retrieved was wrong. The IDaprSidecarHttpClientFactory does not use the ProcessOptions to look up the DaprPort of the sidecar. It uses the environment variable. This might cause some unexpected behavior in the InvocationHandler if the variable is not set.

I assume the NullReferenceException is a consecutive fault of the wrong DaprPort. Looking at the code, it only makes sense if the context is Null, since the OnAfterSendAsync is called in the finally block.

badgeratu commented 2 years ago

I think we can close this one, as I don't think there's a bug here. When Sidekick launches a Dapr sidecar it always sets the environment variable and the HTTP invocation client uses the same approach as the Dapr .NET SDK which also requires the environment variable, so provided its being used as intended it should work. And without a test harness to reproduce the issue it's going to be difficult to dig into I'm afraid. However if you'd like to submit a PR to resolve the issue then please re-open it and do so :)

man-group / dapr-sidekick-dotnet