Response.OnCompleted exceptions not caught when hosted InProcess

[remyblok]

Issue #2170 has been reported on this same topic and has been fixed. But now with the InProcess hosting model the same problem exists again.

Using ASP.NET Core 2.2 using <AspNetCoreHostingModel>InProcess</AspNetCoreHostingModel>
A simple code like this shows the issue:

public async Task<IActionResult> TestOnComplete()
{
        Response.OnCompleted(async () =>
        {
            throw new Exception();
        });
}

Response.OnCompleted is said to be invoked after the response is sent to the user, so the user should already have got the answer, whatever happens there.

https://github.com/aspnet/AspNetCore/blob/master/src/Http/Http.Abstractions/src/HttpResponse.cs#L87

        /// <summary>
        /// Adds a delegate to be invoked after the response has finished being sent to the client.
        /// </summary>
        /// <param name="callback">The delegate to invoke.</param>
        /// <param name="state">A state object to capture and pass back to the delegate.</param>
        public abstract void OnCompleted(Func<object, Task> callback, object state);

But this breaks the response. You can also set a breakpoint on the OnComplete action, the user will wait for the response until the breakpoint is passed.

This means that the OnCompleted is fired before the response is actually sent.

Now, by setting <AspNetCoreHostingModel>OutOfProcess</AspNetCoreHostingModel>, the issue is no longer present and the OnComplete action is executed after the response is send.

While fixing #2170 the file https://github.com/aspnet/AspNetCore/blob/master/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs#L692 has been changed to await FireOnCompleted(); after calling await ProduceEnd(); It seems that this still happens for OutOfProcessing.
While the InProcess model uses https://github.com/aspnet/AspNetCore/blob/master/src/Servers/IIS/IIS/src/Core/IISHttpContextOfT.cs#L61 where the await FireOnCompleted(); is still called before await ProduceEnd();

[Tratcher]

Good analysis. Yes this is an ordering bug we should deal with.

FireOnCompleted should also not call ReportApplicationError, it should directly log and suppress the exceptions and not otherwise fail the response.

Can you give an example of how this impacted you? Were you able to work around it? That information will help us prioritize getting this fixed.

[remyblok]

I wanted to use it to do some cleanup of temp stuff. It's not important for the request to finish. If it fails it does not impact the end result of the request. Since it calls into another web service it will also take some time, so I didn't want the end user to wait for it. Yet the cleanup is tight to the request, that's where I have easy access the right information. So the OnComplete-call seemed appropriate.

Currently it does not impact any production code. I caught the behaviour early on due to exceptions impacting the response.

As a workaround I'm considering just running it OutOfProcess, then it just works already. Once the bug is fixed I can switch the hosting model. At the moment I don't think this will impact me too much.
I've also been thinking about a coding solution, either just using old-school ThreadPool.QueueUserWorkItem(), or queued background task as described here: https://docs.microsoft.com/en-us/aspnet/core/fundamentals/host/hosted-services?view=aspnetcore-2.2&tabs=visual-studio#queued-background-tasks. But for the moment switching the hosting model seems easiest. In any case there are good options to work around it.

[Phylum123]

I have the same issue. I want a fire and forget action that runs for a long time and another action is used to get messages from the long running action. The ability to fire it after the response is sent is vital for this. For now we have had to make it into two actions and it is a little messy.

EDIT: I switched over to OutOfHost processing to compensate until this is fixed. It worked like a charm.

[analogrelay]

Planning Notes: We think the main work here is just to catch exceptions from OnCompleted callbacks when they are called by the managed IIS code. Marking help wanted and leaving in the 5.0 queue.

[Tratcher]

Note that the ordering issue is covered by #17268.

This issue only tracks the exception handling.