Add a Request.wait_for_disconnection() method

[gjcarneiro]

as means of allowing request handlers to be notified of premature client
disconnections.

What do these changes do?

Adds a new async method Request.wait_for_disconnection(). This method does nothing except to wait until the connection that sent a request to be disconnected.

This may be useful, in certain cases, now that aiohttp no longer automatically cancels request handlers upon client disconnection. E.g., if there is a slow DB query, it is beneficial to be able to cancel that DB query as soon as the client disconnects, to avoid the DB doing useless work.

This async method provides the needed "signal" for that. Although making use of it is not pretty, it can be simplified by a utility function/wrapper, probably outside the scope of aiohttp (and outside the scope of this PR).

Are there changes in behavior for the user?

A new method was added, as described above. It is opt-in, so should not affect existing code.

Related issue number

#2492

Checklist

• I think the code is well written
• Unit tests for the changes exist
• Documentation reflects the changes
• If you provide code modification, please add yourself to CONTRIBUTORS.txt
  • The format is <Name> <Surname>.
  • Please keep alphabetical order, the file is sorted by names.
• Add a new news fragment into the CHANGES folder
  • name it <issue_id>.<type> for example (588.bugfix)
  • if you don't have an issue_id change it to the pr id after creating the pr
  • ensure type is one of the following:
    • .feature: Signifying a new feature.
    • .bugfix: Signifying a bug fix.
    • .doc: Signifying a documentation improvement.
    • .removal: Signifying a deprecation or removal of public API.
    • .misc: A ticket has been closed, but it is not of interest to users.
  • Make sure to use full sentences with correct case and punctuation, for example: "Fix issue with non-ascii contents in doctest text files."

[codecov-io]

Codecov Report

Merging #4200 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #4200      +/-   ##
==========================================
+ Coverage   97.55%   97.56%   +<.01%     
==========================================
  Files          43       43              
  Lines        8841     8855      +14     
  Branches     1383     1385       +2     
==========================================
+ Hits         8625     8639      +14     
  Misses         93       93              
  Partials      123      123

Impacted Files	Coverage Δ
aiohttp/web_protocol.py	92.69% <100%> (+0.02%)	⬆️
aiohttp/web_request.py	97.57% <100%> (+0.07%)	⬆️
aiohttp/connector.py	96.28% <0%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2cdabe2...831ce96. Read the comment docs.

[gjcarneiro]

One check is failing because of:

The job running on agent Azure Pipelines 7 has exceeded the maximum execution time of 10. For more information, see https://go.microsoft.com/fwlink/?linkid=2077134
The operation was canceled.

I don't think it's my fault that it's failing. Possibly the CI environment is broken. Limiting to 10 minutes seems rather restrictive...

[asvetlov]

Yes, tests hang on MacOS periodically.
I'm working on the fix. It is reproduced on a relative heavy load.

[asvetlov]

Please fix my note and update the documentation.

.. versionadded:: should use 4.0 as you use 4.0-specific request._cancel()

[asvetlov]

1. A client opens a connection in HTTP keep-alive mode
2. server code calls await request.wait_for_disconnection()
3. HTTP request is handled without socket closing (keep-alive mode is on)
4. await request.wait_for_disconnection() hangs forever

Please fix it

[gjcarneiro]

1. A client opens a connection in HTTP keep-alive mode
2. server code calls await request.wait_for_disconnection()
3. HTTP request is handled without socket closing (keep-alive mode is on)
4. await request.wait_for_disconnection() hangs forever

Please fix it

The method name clearly says "wait for disconnection". In case "HTTP request is handled without socket closing (keep-alive mode is on)" there is no disconnection. That is a use case I expected.

The way I envision this to be used, then request.wait_for_disconnection() coroutine call would always be cancelled at the end of each request, by the app code. This is meant to be just a building block for something more convenient to use, like a decorator. From the code I posted on the issue, I can create a decorator like:

def cancel_on_disconnect(handler):
  @functools.wraps(handler)
  async def wrap_handler(request):
     wait = asyncio.create_task(request.wait_for_disconnection())
     bgtask = asyncio.create_task(handler())
     asyncio.wait([wait, bgtask], return_when=asyncio.FIRST_COMPLETED)
     if bgtask.done():
         wait.cancel()
         return web.Response(bgtask.result())
     bgtask.cancel()
     raise web.HTTPGatewayTimeout()
  return wrap_handler

@cancel_on_disconnect
def my_handler(request):
   ...

The above code probably has bugs, I didn't even test it. But the point is that request.wait_for_disconnection() will always be cancelled at the end of the request anyway.

I can try to make it return at request end, even if there is no disconnection due to keepalive, but honestly I don't think it matters.

[asvetlov]

I have a feeling that time of request.wait_for_disconnection() waiting should be less than the lifetime of request object itself.
Cancelling all pending waiters after web-handler finishing makes API cleaner and safer to use, isn't it?

[gjcarneiro]

OK, fine... but then the documentation, and maybe method name, might need to be updated to reflect that the function also returns at the end of request, not just client disconnection.

[asvetlov]

I was talking about cancellation, not a regular None return.
Documentation should reflect it, sure.
Regarding the method name: do you have a proposal?

[gjcarneiro]

I was talking about cancellation, not a regular None return.

I didn't understand this, but makes sense.

Regarding the method name: do you have a proposal?

Because it cancels it, there is no need for renaming, I think.

[asvetlov]

looks good. The last remaining part is documentation update

[gjcarneiro]

Note: feel free to rebase/squash commits, or else I can do it.

[asvetlov]

Please apply the suggestion.

No rebase is required; we use squash-on-merging strategy for aiohttp RPs

[asvetlov]

Thanks!

[patchback]

Backport to 3.10: 💔 cherry-picking failed — conflicts found

❌ Failed to cleanly apply 7f77733 on top of patchback/backports/3.10/7f777333a4ec0043ddf2e8d67146a626089773d9/pr-4200

Backporting merged PR #4200 into master

1. Ensure you have a local repo clone of your fork. Unless you cloned it
   from the upstream, this would be your origin remote.
2. Make sure you have an upstream repo added as a remote too. In these
   instructions you'll refer to it by the name upstream. If you don't
   have it, here's how you can add it:

   $ git remote add upstream https://github.com/aio-libs/aiohttp.git

3. Ensure you have the latest copy of upstream and prepare a branch
   that will hold the backported code:

   $ git fetch upstream
   $ git checkout -b patchback/backports/3.10/7f777333a4ec0043ddf2e8d67146a626089773d9/pr-4200 upstream/3.10

4. Now, cherry-pick PR Add a Request.wait_for_disconnection() method #4200 contents into that branch:

   $ git cherry-pick -x 7f777333a4ec0043ddf2e8d67146a626089773d9

   If it'll yell at you with something like fatal: Commit 7f777333a4ec0043ddf2e8d67146a626089773d9 is a merge but no -m option was given., add -m 1 as follows instead:

   $ git cherry-pick -m1 -x 7f777333a4ec0043ddf2e8d67146a626089773d9

5. At this point, you'll probably encounter some merge conflicts. You must
   resolve them in to preserve the patch from PR Add a Request.wait_for_disconnection() method #4200 as close to the
   original as possible.
6. Push this branch to your fork on GitHub:

   $ git push origin patchback/backports/3.10/7f777333a4ec0043ddf2e8d67146a626089773d9/pr-4200

7. Create a PR, ensure that the CI is green. If it's not — update it so that
   the tests and any other checks pass. This is it!
   Now relax and wait for the maintainers to process your pull request
   when they have some cycles to do reviews. Don't worry — they'll tell you if
   any improvements are necessary when the time comes!

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.