Cannot access a closed Stream exception when using LiveReload

[CodeBlanch]

@RickStrahl Hey! Ran into an issue, may or may not be a bug depending on how you look at it. But I thought I'd run it up the flag pole to see.

A developer introduced the LiveReload library into one of our ASP.NET Core 3.1 apps. Suddenly all requests to that service started blowing up with a "Cannot access a closed Stream" exception.

I tracked it down to... https://github.com/RickStrahl/Westwind.AspnetCore.LiveReload/blob/4759aff17dd61085eae8b5427fe11ca1ae16b1d4/Westwind.AspnetCore.LiveReload/LiveReloadMiddleware.cs#L92-L103

The middleware is wrapping the response in a wrapper. When the middleware exits, it disposes the wrapper AND the underlying stream.

It just so happens we also have some middleware wrapping the response for logging in debug mode. Our wrapper is being closed by the middleware and the exception is thrown when we try to access the stream.

Should LiveReload middleware leave the underlying stream open because it doesn't technically own it?

[RickStrahl]

What type of service? Some sort of custom middleware? MVC based service should work the same as as Pages/View based MVC.

I think the issue is that you need to make sure that LiveReload sits at the very top of the pipeline, so it's last in line to close anything. The pipeline goes FIFO, so LiveReload needs to look at all requests pretty much including the potential errors which can short circuit.

Make sure app.UseLiveReload() is set at the beginning of the Configure() method in startup before any other output producing services.

Also in the latest version there are some additional safe-guards to not assign the stream if it's not there. This may in some cases work better especially if another middleware has already completed the request.

[RickStrahl]

Any more info on this? Otherwise closing as not reproducible.

[CodeBlanch]

So in our case we have some custom middleware that we want to run first (& last, really). Its job is to log a bunch of details about the request before we invoke the next delegate in the chain, and then log a bunch of details about the response when everything has completed. It works by swapping out the response stream with a MemoryStream to buffer up the response. Terrible for perf so we only do this in debug builds. The LiveReload middleware closes the response stream, so our code reading back the MemoryStream started to throw. We were able to work around the problem, but I would argue that LiveReload shouldn't close the response stream because it doesn't own it. Someone up the chain owns that (AspNetCore, presumably). I'm sure that is debatable though 😄

[RickStrahl]

Are you on recent versions?

The middleware uses a response wrapper to wrap the response and middleware doesn't explicitly close the stream. If anything ASP.NET Core pipeline does the closing and that's out of the middleware's control. It seems to me based on the error that the response that comes in has already closed the response stream and that's what's actually failing.

Can you give some sort of simple repro scenario?

The simple solution for this should be that you run the LiveReload first before your middleware. LiveReload has to run before anything else that produces output in order to work correctly on the content. However, it shouldn't fail if it's not. I suspect what's happening is that perhaps your middleware hook is closing the stream actually? I ask about recent versions because I added logic that will skip any write operations and basically pass through if the stream or Repsonse is not there.

The problem here is this: LR needs to hook both the beginning and end of the request to work. The beginning it sets up the screen capture so that it can look at the output and detect HTML content and because that's really the only way that we output can be captured for review.

If your middleware writes a complete requests, it probably should not call next() or you should be using app.Run() to terminate the pipeline. If you start writing to the Response then yes this will break the flow because LR has to also modify headers potentially and that can't be done after the Response started outputting.

Bottom line I think the only way that LR can work is at the top of the pipeline before any other output is sent (or potentially sent). As is mentioned in the docs :-)

I use this middleware with API services, and middleware services without issue, but yeah - I run the middleware at the very beginning to make sure there's no funny business with half finished responses getting closed at the wrong time.

[CodeBlanch]

The middleware uses a response wrapper to wrap the response and middleware doesn't explicitly close the stream.

It's closed when disposed by the wrapper:

https://github.com/RickStrahl/Westwind.AspnetCore.LiveReload/blob/4759aff17dd61085eae8b5427fe11ca1ae16b1d4/Westwind.AspnetCore.LiveReload/ResponseStreamWrapper.cs#L161

If that wasn't there, everything would work fine. The point I'm making is: It doesn't need to be there. No reason that I can think of for LiveReload to dispose of the base stream, whoever owns it is responsible for that part.

I'll shoot over a repro a bit later.

[RickStrahl]

Good catch @CodeBlanch...

I'm not sure why that's there offhand - you are correct, in theory the middleware shouldn't be closing the wrapped stream. Have to try it out and make sure that everything gets closed at the end of a request.

[CodeBlanch]

@RickStrahl Here's a repo with a small-ish project that reproduces the exception: https://github.com/CodeBlanch/LiveReloadRepro

Throw happens on this line: https://github.com/CodeBlanch/LiveReloadRepro/blob/32ee90cde78567624a6264cdf8cff1cb99cde9cd/LiveReloadRepro/CustomMiddleware/RequestLoggingMiddleware.cs#L47

[RickStrahl]

Pushed out an update of 0.2.14 to NuGet with the removed stream closure.

[CodeBlanch]

Thanks @RickStrahl!