Feature Request: option to unconditionally run BackgroundTask in StreamingResponse

[AllSeeingEyeTolledEweSew]

Is your feature related to a problem? Please describe.

StreamingResponse currently doesn't provide a natural way to release resources or do any tear-down. I can provide a BackgroundTask to StreamingResponse, but if an exception is thrown (either from the asgi send, or from my iterator itself), the task is never run.

My use case: I'm building an HTTP bridge that streams data from bittorrent. StreamingResponse is a natural choice. My app has a singleton bittorrent client which downloads data from the swarm, and prioritizes among the active requests to my app. This downloading is slow, so it downloads data with a large buffer. If a request errors out, I want to notify the bittorrent client as quickly as possible, so it doesn't continue to download data unnecessarily.

Describe the solution you would like.

One of

• Change StreamingResponse to always run its BackgroundTask, e.g. in a finally: block in its __call__. I expect this isn't acceptable since it would break some consumers. But I'm struggling to think of the case where a consumer really wants the task to not be run in the event of an error, so maybe this is the right choice
• Add an argument StreamingResponse(background=task, always_run_background=True), to ensure the task gets run as described above.

Describe alternatives you considered

• Do teardown in gc: I could use a weakref with finalizer (or Iterable with a __del__) to do the cleanup. Depending on when gc runs, my cleanup code might not run immediately and I might continue to download unnecessary data. There are also a lot of arcane restrictions on what code may/should run in finalizers; I would much rather not need to think about these
• Subclass StreamingResponse myself: Easy enough, but awkward. I need to either re-implement __call__, or do some things to ensure I don't call the BackgroundTask twice:

class AlwaysRunStreamingResponse(StreamingResponse):
    async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None:
        try:
            super().__call__(scope, receive, send)
        except Exception:
            # This will have been run in the non-error case, only have to run it here
            await self.background()
            raise

[AllSeeingEyeTolledEweSew]

I just learned how async generators really work, and now I don't have a need for this feature.

Given:

async def generate() -> AsyncIterator[bytes]:
    try:
        yield ...
    finally:
        print("finally!")

I learned that asyncio actually guarantees the finally: clause will always run, even if the task is cancelled. This is the natural place for the generator's cleanup code, and it's good enough for me.

[graingert]

I'm looking to fix this by contextlib.aclosing the aiter in StreamingResponse. This means that cancellation will be delivered to your finally: block.

[graingert]

I could make this opt in/out by deprecating AsyncIterators in favour of inspect.isasyncgenfunctions only, then only aclosing the aiter if StreamingResponse created it.

[AllSeeingEyeTolledEweSew]

I don't think that's necessary. I claim the problem is actually solved, though it's not clear it is.

It looks like asyncio uses sys.set_asyncgen_hooks() to ensure any async generator is always closed when garbage collected. From a user perspective, either both the "first line" of code (until the first await or yield) and the finally: block run, or neither runs. Per PEP 525:

The idea behind sys.set_asyncgen_hooks() is to allow event loops to intercept asynchronous generators iteration and finalization, so that the end user does not need to care about the finalization problem, and everything just works.

This does have tradeoffs for StreamingResponse: the resources getting cleaned up in finally: must be initialized at the top of the generator, not before the generator is created; I've already hit this subtle bug. Also, cleaning up at gc may not be timely in some cases, e.g. if your streaming video is chewing CPU time and should be cleaned up ASAP.

But IMO, the tradeoffs are fine for general-purpose code. I think it's fine to keep the argument an AsyncIterator, and users can just use AsyncGenerator if they need it.