dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.14k stars 4.71k forks source link

HttpHandlerDiagnosticListener did not send DiagnosticSource Ex.Stop event on netfx in W3C mode #40777 #38152

Open eduard-bystrov opened 4 years ago

eduard-bystrov commented 4 years ago

see https://github.com/dotnet/corefx/pull/40777 first

forgot to fix for Ex.Stop ? or I'm wrong

https://github.com/dotnet/runtime/blob/037e7fccf7389108f7537d6855a23ca815f7c749/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/HttpHandlerDiagnosticListener.cs#L674


and second question. Request with status code 401(Unauthorized) can be autoredirected, which leads to the execution of the second request on the same object HttpWebRequest.

If this happens we have the following chain of events:

1) System.Net.Http.Desktop.HttpRequestOut.Start 2) System.Net.Http.Desktop.HttpRequestOut.Ex.Stop (got 401) 3) System.Net.Http.Desktop.HttpRequestOut.Stop (success)

it's a bit strange... hardly anyone expects to get Stop and Stop.Ex for one request.

ghost commented 4 years ago

Tagging subscribers to this area: @dotnet/ncl Notify danmosemsft if you want to be subscribed.

ghost commented 4 years ago

Tagging subscribers to this area: @tommcdon, @krwq Notify danmosemsft if you want to be subscribed.

tommcdon commented 4 years ago

@noahfalk

noahfalk commented 4 years ago

forgot to fix for Ex.Stop ? or I'm wrong

Yeah it does appear that the fix was incomplete. Thanks for filing the issue! If you are interested in submitting the PR to fix it I'd be happy to review it, otherwise we can leave this until someone else can get to it (which could be a while).

and second question... Request with status code 401(Unauthorized)

I'm not sure what the question was, were you asking if the behavior is intentional? I'm not the original author so I can't say for sure but if I had to guess it probably is emergent behavior rather than intentional. If you wanted to propose a change in behavior or have more discussion about it lets open a 2nd issue to ensure we keep track of both.