aspnet / BasicMiddleware

[Archived] Basic middleware components for ASP.NET Core. Project moved to https://github.com/aspnet/AspNetCore
Apache License 2.0
169 stars 84 forks source link

Feature: Break Invoke method into two (PreInvoke and PostInvoke) #364

Closed a-patel closed 5 years ago

a-patel commented 6 years ago

As per existing mechanism, there is only one method (Invoke) to handle request and response which is separated by RequestDelegate (next). For some developers, it is quite confusing. It will better if there are two separate methods for pre/before invocation and post/after invocation.

`

public class CustomMiddleware
{
    private readonly RequestDelegate _next;

    public CustomMiddleware(RequestDelegate next)
    {
        _next = next;
    }

    protected override void PreInvokeNext(HttpContext context)
    {
    }

    protected override void PostInvokeNext(HttpContext context)
    {
    }
}

`

Tratcher commented 6 years ago

Those overrides imply a base class not referenced here?

While I understand the conceptual appeal, this model does not end up being useful in practice for a few different reasons. I'll reference this response buffering middleware as an example: https://github.com/aspnet/BasicMiddleware/blob/2a731baaa3a4f6c91abad0bd68b1ea01f025d10a/src/Microsoft.AspNetCore.Buffering/ResponseBufferingMiddleware.cs#L19

A. You usually need to flow per request state from PreInvoke to PostInvoke. While possible, it's much easier to do this within Invoke. B. People over-estimate what's possible in PostInvoke. Because responses are not normally buffered, response status code, headers, and body are sent immediately from the inner layer and cannot be modified from PostInvoke. PostInvoke ends up being more about cleaning up any leftover state. C. Many middleware need to do some handling of exceptions from _next either via catch or finally blocks. D. PreInvoke and/or PostInvoke logic often needs to be asynchronous.

aspnet-hello commented 5 years ago

We periodically close 'discussion' issues that have not been updated in a long period of time.

We apologize if this causes any inconvenience. We ask that if you are still encountering an issue, please log a new issue with updated information and we will investigate.