Skip to content
This repository has been archived by the owner on Dec 8, 2018. It is now read-only.

Request for Unhandle Exception changes for non-page or non-MVC applications. #222

Closed
codematrix opened this issue Dec 5, 2015 · 6 comments

Comments

@codematrix
Copy link

Hi Team,

I would like to extend the Diagnostics repo to facilitate the ability to intercept unhandled exceptions but more from a "non-page or non-MVC6" diagnostics perspective. My goal is to provide developers an easy way to intercept unhandled exceptions by allowing them to add as many "exception intercepts" as necessary, to meet their application objectives. Going the "non-page or non-MVC6" route is a very common scenario, especially as more companies move towards AngularJS and/or exposing their backend services as REST APIs. The need for MVC6 views/controllers in these particular scenarios, is not required.

Now, I already created the so called "ExceptionHandlerIntercept" project and I was taking advantage of the "UseExceptionHandler" extension found in https://github.com/aspnet/Diagnostics/blob/dev/samples/ExceptionHandlerSample/Startup.cs
whereby, I added my hook to the existing “ExceptionHandlerMiddleware” as shown in one of your sample projects.

However, when doing a pull request to see where I can interweave my code, it just came to my realization that the "Diagnostic" repos seems to be geared towards page responses - the dead giveaway was the solution name: "DiagnosticsPages.sln" and all of the samples projects were basic page like responses.

Leaving my code there will work, however, is this the right repo for this? My reason for asking is, for non-page applications (i.e. REST API, etc.) the general response will more than likely be either Xml or JSON.

Is it OK for me to create a new Project called "ExceptionIntercept"? And if the answer is YES, I would also like to add a couple of interfaces to the Microsoft.AspNet.Diagnostics.Abstractions project but one of the interfaces requires an HttpContext property which in return, requires me to add the dependency "Microsoft.AspNet.Http". If this not OK, I can always use a "T" called context for the interface in question.

Please advise me in the direction(s) I should go in this regard.

Thanks for your time,

Ross Pellegrino

@davidfowl
Copy link
Member

Diagnostic pages is what it was called before and we didn't update the solution name. Why do we need another middleware? Why can you is it build on top of the exception handler middleware?

@Tratcher
Copy link
Member

Tratcher commented Dec 5, 2015

Sounds like he wants content negotiation.

@davidfowl
Copy link
Member

And that can't be done on top of the exception handler?

@codematrix
Copy link
Author

I'm actually plugging into the exception handler middleware. What I'm suggesting is something similar to how it works for webapi 2.1, whereby I can add IExceptionLogger or IExceptionHandler implementations.

Basically I want to simplify adding handlers with a consistent framework called ExceptionHandlerIntercept, which sits on top of the existing ExceptionHandlerMiddleware.

I already have the coding done and can submit it as a PR if you like? I think it can make what im purposing more clear. I'll submit with a sample project without unit testing if that's OK. If you like what I purposed l can add the unit testing in a final PR.

Ross

@davidfowl
Copy link
Member

I don't think we want anything like the web API 2 feature as a middleware in the box. We already have the bare mimimum required to implement such a feature.

This might be something you can add to aspnet contrib.

In general, I'd like to see us provide more building blocks that people can build on top of.

@codematrix
Copy link
Author

Hi David,

Makes sense. My version doesn't actually have a middleware. It's more or less a simple set of classes with a set of extension methods that allows a developer to add exception handlers. The existing ExceptionHandlerMiddleware does the job allowing us to hook into this nicely. The version that I'm suggesting doesn't supersede the current functional but rather provides a layer that sits on top of it, if the developer so requires it. We are literal only talking about 200 lines of code or less.

For example, in the Startup.cs, a developer can do this to configure it's usage:

public class Startup
{
    public void ConfigureServices(IServiceCollection services)
    {
        services.AddExceptionIntercepts(); 
        services.AddMvc();
    }

    public void Configure(IApplicationBuilder app)
    {
        // NOTE: the order of configuration is important as Middlewares are based on
        // a sequential Pipeline workflow.  

        // *** samples 
        // the order of addition determines the sequence of how each Exception Intercept Handler gets called.
        app.UseExceptionIntercepts();
        app.AddExceptionInterceptHandler(new ExceptionInitializer(new ExceptionCategorizer()));
        app.AddExceptionInterceptHandler(new ExceptionJIRALogger());
        app.AddExceptionInterceptHandler(new ExceptionDbLogger());
        app.AddExceptionInterceptHandler(new ExceptionFinalizer());

        app.UseMvc();
    }
}

Then for example, the developer can add a handler like this:

public class ExceptionJIRALogger : IExceptionInterceptHandler<HttpContext>
{
    public Task HandleAsync(IExceptionInterceptContext<HttpContext> exceptionContext)
    {
        var category = (ExceptionCategory)exceptionContext.Context.Items["exception.category"];
        if (category.Category == ExceptionCategoryType.Unhandled)
        {
            dynamic response = exceptionContext.Context.Items["exception.response"];

            // log whatever to JIRA for production issue tracking
        }

        return Task.FromResult(0);
    }
}

OR this to handle the final response from a REST API call.

public class ExceptionFinalizer : IExceptionInterceptHandler<HttpContext>
{
    public async Task HandleAsync(IExceptionInterceptContext<HttpContext> exceptionContext)
    {
        var category = (ExceptionCategory)exceptionContext.Context.Items["exception.category"];
        dynamic response = exceptionContext.Context.Items["exception.response"];
        dynamic finalResponse = category.DeveloperMode ? response : response.System;

        exceptionContext.Context.Response.StatusCode = (int)category.HttpStatus;
        exceptionContext.Context.Response.ContentType = "application/json";
        await exceptionContext.Context.Response.WriteAsync((string)JsonConvert.SerializeObject(finalResponse, Formatting.Indented));
    }
}

NOTE: the above are just some of the examples of what a developer may want to add.

It's not earthshattering but provides a clean way for developers to easily add intercepts in a consistent way, keeping inline with convention vs. build your own.

I can see your point as having building blocks. We can add it as a Nuget Package, something to the affect of: Microsoft.AspNet.Diagnostics.ExceptionIntercept (I have no affinity to the name, BTW). If a WebAPI developer requires this sort of mechanism then they can install this package.

I'll take a look at the aspnet contrib and what it entails.

Thanks,

Ross

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants