Failed request status code is 200 about applicationinsights-aspnetcore HOT 12 CLOSED

I could not repro this problem. After discussing with @abaranch, it may have occurred if the Application Insights RequestTrackingMiddleware was added after the ASP.NET's ErrorHandlerMiddleware, which is responsible for setting the HttpResponse.StatusCode. When added correctly, as the first middleware in Configure method, the RequestTrackingMiddleware correctly sets RequestTelemetry.StatusCode from HttpResponse.StatusCode.

from applicationinsights-aspnetcore.

It turns out that the ErrorPageMiddleware, used by aspnet in development mode does not set the status code.
P.S. This is not accurate. ErrorPageMiddleware relies on error pages to set the status code and I couldn't repro the problem in my local environment. Will try the original environment we saw this problem.

from applicationinsights-aspnetcore.

Opened aspnet/Diagnostics#133 to understand how Response.StatusCode is supposed to be set.

from applicationinsights-aspnetcore.

@Eilon I updated description of the issue. Who will be the best person to discuss error processing practices in ASP.NET 5 and how Application Insights should implement it to show credible data to developer?

from applicationinsights-aspnetcore.

Logging would get you more info. But ultimately there's obviously no absolute way to get all possible information because components are free to have undiagnosable behavior. Having said that, the built-in components should "play well with others" as much as possible.

I'm not sure how you could have an HTTP 500 (or whatever) to represent an error yet also redirect to another page... maybe it's a best practice not to do redirects on errors to begin with.

from applicationinsights-aspnetcore.

Redirect on error is a default behavior in music store. And it is frustrating not to see requests as failed in this situation (we define request as failed if status code is 400+). I'd assume developer will want to see the status code just before "UseStatusCodePagesWithRedirects" middleware, not 302 and 200 in telemetry reports.

It also interesting what Application Insights should do if request tracking middleware see exception. As we recommend to put it as a very first middleware it makes sense to assume status code 500. Or at least mark request as failed.

Today we have two middleware - one for exceptions and one for requests. Logic is very simple and straightforward - exceptions middleware collect exception information and requests - only care about request. This logic is clearly not working for the cases above. Here is the assumptions we can make:

1. Mark request as failed if exceptions middleware see exception
2. Mark request as failed if request middleware see exception
3. Change request status code to 500 if requests middleware see exception
4. Implement a separate middleware that collects request information, but do not track it's time so it can be used after errorRedirect-like middleware.

We can choose NOT to implement any of these assuming that typical application will have proper error handling logic and will set proper response status code explicitly. We can also ask customer to put request middleware right after "UseStatusCodePagesWithRedirects" middleware to get a proper response codes.

So the questions really are:

• How likely do you think applications will not handle exceptions and set status codes explicitly?
• Will logic like redirect on error be used widely so we need to have a good support for it (perhaps excessive documentation or separate middleware that collects request information, but do not track it's time)?

from applicationinsights-aspnetcore.

I think the challenge here is that it's pretty much impossible to make generalizations about how all web developers want to treat errors. If someone wants to handle certain types of "errors" that would have been an HTTP 500 and redirect to some other page, how can you say with any certainty that it was truly an error? Maybe it's just a wacky app that is "supposed to" occasionally return 500's and they get converted to something else. Should such an "error" be tracked?

I think that intercepting logging might be the right way to go here because logging will tell you that something "bad" happened regardless of how that bad thing is dealt with. If you trace Warning and higher (or maybe Error and higher?) then you should see all these events happening. I'm not at all saying that it necessarily has exactly the behavior you want (or the customer wants), but it ought to be more reliable.

from applicationinsights-aspnetcore.

Perhaps the Application Insights RequestTrackingMiddleware could decorate the HttpContext before passing it to the next middleware. That way it can observe all status codes and redirects applied to the HttpResponse. So, if a request failed with original status code 500, and was then redirected to a static "oops" page, the RequestTelemetry can still have the Success set to false even though the actual status code is a successful 301 or 302.

from applicationinsights-aspnetcore.

Wrapping the entire context seems dangerous. Plus, even if you did, I'm not sure that would be the right behavior either: just because a component set the value doesn't mean it's necessarily an error.

The real question is: exactly what types of errors are you trying to catch? All exceptions in the app? All times when the status code was set to 400+? Both of these seem ill-advised to me because they're not necessarily problems.

from applicationinsights-aspnetcore.

Why do you feel that wrapping the HttpContext is dangerous? For most properties and methods the wrapper would simply pass the call through to the original implementation. The only actual logic would be in the StatusCode setter to "remember" if it was ever set to an "error" code.

from applicationinsights-aspnetcore.

My key point is that wrapping won't get you what you want. Code that sets a status code won't help you determine anything because you won't know why the status code was set.

Catching exceptions just prior to the error page / error handler middleware is the closest you can get to knowing that an exception happened. And having middleware also detect whether the outgoing HTTP status code is 400+ can also tell you that something "bad" happened.

from applicationinsights-aspnetcore.

Created new wiki page to describe some of scenarios. Our middleware is very simple today and if it's logic doesn't satisfy customer - it can be easily replaced by hand-made implementation.

Closing issue for now.

from applicationinsights-aspnetcore.