ClientSession gets closed unexpected leading to RuntimeError: Session is closed

[spacemanspiff2007]

Describe the bug
The asyncio session gets closed after the first request through the context manager

To Reproduce
Steps to reproduce the behavior:

    session = aiohttp.ClientSession()
    
    print('Start forecast')
    forecast = await self.darksky.get_forecast(
        latitude=CONFIG.location.latitude,
        longitude=CONFIG.location.longitude,
        lang='de',
        client_session=session,
    )
    print(forecast.currently)
    
    await asyncio.sleep(1)
    
    print('Start forecast')
    forecast = await self.darksky.get_forecast(
        latitude=CONFIG.location.latitude,
        longitude=CONFIG.location.longitude,
        lang='de',
        client_session=session,
    )
    print(forecast.currently)

output:

Start forecast
CurrentlyForecast(2020-04-19 06:24:43+02:00)
Start forecast

Error in func: Session is closed
Traceback (most recent call last):
  File "C:\venv\lib\site-packages\darksky\api.py", line 139, in get_forecast
    client_session=client_session,
  File "C:\venv\lib\site-packages\darksky\request_manager.py", line 51, in make_request
    url, params=params, headers=self.headers
  File "C:\venv\lib\site-packages\aiohttp\client.py", line 1012, in __aenter__
    self._resp = await self._coro
  File "C:\venv\lib\site-packages\aiohttp\client.py", line 357, in _request
    raise RuntimeError('Session is closed')
RuntimeError: Session is closed

Expected behavior
If a session is passed into the request it is not allowed to be closed

Desktop (please complete the following information):

• Dark Sky Version: 1.8.0
• Python Version: 3.6 & 3.7

Additional context
Add any other context about the problem here.

[spacemanspiff2007]

#42 is what caused the change

[allenlawrence94]

The bug is that the client_session passed into RequestMangerAsync.make_request is used as a context manager, when it should just be used directly. I think an ideal usage of DarkSkyAsync would look like this:

from aiohttp import ClientSession
import asyncio
import os
from darksky.api import DarkSkyAsync

API_KEY = os.getenv('DARKSKY_SECRET_KEY')


async def test():
    client = DarkSkyAsync(API_KEY)
    async with ClientSession() as session:
        forecast1 = await client.get_forecast(
            latitude=40,
            longitude=-80,
            lang='de',
            client_session=session
        )
        print(forecast1.currently)
        async with session.get('https://google.com') as google_response:
            print(await google_response.text())
        forecast2 = await client.get_forecast(
            latitude=40,
            longitude=-80,
            lang='de',
            client_session=session
        )
        print(forecast2.currently)


if __name__ == '__main__':
    asyncio.get_event_loop().run_until_complete(test())

I tossed in a request to google to show what I mean by using the same session for non-darksky requests.

This code still fails for the same reason as yours. Fixing this bug would require little more than removing line 49 of request_manager.py. All I'm saying is that DarkSkyAsync shouldn't directly manage an aiohttp.ClientSession - it should use a ClientSession instance that is being managed at a higher level.

[spacemanspiff2007]

This code still fails for the same reason as yours. Fixing this bug would require little more than removing line 49 of request_manager.py. All I'm saying is that DarkSkyAsync shouldn't directly manage an aiohttp.ClientSession - it should use a ClientSession instance that is being managed at a higher level.

That would make sense. Imho it should be enough if you remove line 49 of the request_manager.py and remove the ClientSession creation. Of course client_session then has to be mandatory in get_forecast.

class RequestMangerAsync(BaseRequestManger):
    async def make_request(
        self,
        url: str,
        client_session: ClientSession,
        **params
    ):
        assert isinstance(client_session, ClientSession), type(client_session)

        for key in list(params.keys()):
            if params[key] is None:
                del params[key]
            elif isinstance(params[key], list):
                params[key] = ",".join(params[key])

        async with client_session.get(
            url, params=params, headers=self.headers
        ) as resp:
            response = await resp.json()
            if "error" in response:
                raise DarkSkyException(response["code"], response["error"])
        response["timezone"] = params.get("timezone") or response["timezone"]
        return response

[allenlawrence94]

That looks awesome to me. Seems like you should go ahead and open a PR!

[Detrous]

Thank you!
Release 1.9.0 now available.