Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HttpHandlerDiagnosticListener does not send second chain of events if the request was autoredirected on netfx in W3C mode #40777

Closed
eduard-bystrov opened this issue Aug 13, 2020 · 5 comments · Fixed by #55392
Assignees
Milestone

Comments

@eduard-bystrov
Copy link

eduard-bystrov commented Aug 13, 2020

Here from #38152

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

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.

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added area-System.Net.Http untriaged New issue has not been triaged by the area owner labels Aug 13, 2020
@ghost
Copy link

ghost commented Aug 13, 2020

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

@karelz
Copy link
Member

karelz commented Aug 13, 2020

@eduard-bystrov were you able to reproduce it also on .NET Core?
Or is the report only about .NET Framework?

@eduard-bystrov
Copy link
Author

@eduard-bystrov were you able to reproduce it also on .NET Core?
Or is the report only about .NET Framework?

only about .NET Framework
I have no information about .NET Core

@karelz karelz added this to the 6.0.0 milestone Aug 20, 2020
@karelz karelz added bug and removed untriaged New issue has not been triaged by the area owner labels Aug 20, 2020
@karelz
Copy link
Member

karelz commented Aug 20, 2020

We won't be able to fix .NET Framework, unless it has high business impact. (just to set the expectation -- the bar for .NET Framework fixes is pretty high in general)

Triage: It will likely repro in .NET Core as well, we should think about it in our DiagnosticHandler changes in 6.0. We may decide it is by design potentially.

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Jul 9, 2021
@MihaZupan
Copy link
Member

Note: the issue here is that DiagnosticHandler was wrapping the underlying handler and as such could never intercept redirects.

#55392 fixes that by moving DiagnosticHandler to the start of the chain so that any redirect flows through it again and can be instrumented.
With this, automatic redirects will light up in 6.0 like any other regular request.

@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Jul 13, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Aug 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants