Python-slack-sdk: Unclosed client session

[jourdanrodrigues]

Description

Getting Unclosed client session randomly and seldomly when performing hundreds of requests.

This seems to be the solution for this issue: https://github.com/aio-libs/aiohttp/issues/1175#issuecomment-247244796

What type of issue is this?

• [X] bug
• [ ] enhancement (feature request)
• [ ] question
• [ ] documentation related
• [ ] testing related
• [ ] discussion

Requirements

• [x] I've read and understood the Contributing guidelines and have done my best effort to follow them.
• [X] I've read and agree to the Code of Conduct.
• [X] I've searched for any related issues and avoided creating a duplicate issue.

---

Bug Report

Reproducible in:

slackclient version: 2.5.0

python version: 3.8.1

OS version(s): macOS Catalina 10.15.3

Steps to reproduce:

1. Perform hundreds (or thousands) of requests towards Slack API with the WebClient;
2. Logs will eventually show up.

Expected result:

No logs.

Actual result:

This is logged to the console:

2016-09-14 18:42:28,511 ERROR asyncio Unclosed client session
client_session: <aiohttp.client.ClientSession object at 0x7fd2b06469b0>

Attachments:

[jourdanrodrigues]

I think I know what's the issue: Since I'm performing ~2.700 requests, there are some exceptions that are raised from the line below. https://github.com/slackapi/python-slackclient/blob/7f923070f9d928825c5b9a0ed418dc5af62cd35f/slack/web/base_client.py#L244

I know about these exceptions because I'm working around them in my scripts. They are:

asyncio.exceptions.TimeoutError
aiohttp.ClientConnectorError
aiohttp.ServerDisconnectedError

Maybe catching them, closing the session and raising the exception again would suppress this issue?

[seratch]

662 fixes this for run_async=False (False is the default so that you're using False mode if you don't explicitly give this option) as the sync mode no longer depends on aiohttp library.

[jourdanrodrigues]

@seratch I was using it in async mode (forgot to specify). Does it change anything?

[seratch]

@jourdanrodrigues Thanks for responding. In the case, this may still exist depending on aiohttp's matters. Let me re-open this and remove the milestone v2.6.0.

[NoAnyLove]

Having the same issue, try-except caught a asyncio.TimeoutError and it showed "Unclosed client session" at the end,

try:
    await slack_client.chat_postMessage(...)
except Exception:
    logger.exception(f'failed to post message')

After some investigation, it seems that the exception might be raised inside the async with statement, https://github.com/slackapi/python-slackclient/blob/v2.3.1/slack/web/base_client.py#L259

So can we we use try-finally statement to make sure the aiohttp client session is correctly closed? I did some tests locally, it seems working for my case,

try:
    async with session.request(http_verb, api_url, **req_args) as res:
        data = {}
        try:
            data = await res.json()
        except aiohttp.ContentTypeError:
            self._logger.debug(
                f"No response data returned from the following API call: {api_url}."
            )
        response = {"data": data, "headers": res.headers, "status_code": res.status}
finally:
    if not use_running_session:
        await session.close()

[seratch]

@NoAnyLove Thanks for sharing this! Your suggestion looks good to me, too. To move things forward, I want to have a unit test that always reproduces the situation first, and then would love to apply its fix as a valid solution. If you're interested in working on both, I'm happy to be review and merge it. If you're busy, I will be working on it probably next week.

[NoAnyLove]

Thanks for the quick response. I'll try to work on it during the weekend.