fix: request session closed prematurely (#95)

stankudrow · Stanley Kudrow · patrick-zippenfenig · web-flow · commit de0301560397 · 2025-08-18T18:53:11.000+02:00
* remove context managers

* do not use async self._session object in the AsyncClient class

* fix: session handling

* linting

* fix: async sessions

* linting

* remove unused import

---------

Co-authored-by: Stanley Kudrow &lt;stankudrow@reply.no&gt;
Co-authored-by: patrick-zippenfenig &lt;patrick@zippenfenig.de&gt;
diff --git a/openmeteo_requests/Client.py b/openmeteo_requests/Client.py
@@ -4,7 +4,6 @@
 
 from collections.abc import MutableMapping
 from enum import Enum
-from functools import partial
 from typing import Any
 
 import niquests
@@ -45,8 +44,8 @@ class Client:
     """Open-Meteo API Client"""
 
     def __init__(self, session: niquests.Session | None = None) -> None:
+        self._close_session = session is None
         self._session = session or niquests.Session()
-        self._response_cls = WeatherApiResponse
 
     def _request(
         self,
@@ -59,19 +58,18 @@ def _request(
     ) -> list[WeatherApiResponse]:
         params["format"] = _FLAT_BUFFERS_FORMAT
 
-        with self._session as sess:
-            method = method.upper()
-            if method == HTTPVerb.GET:
-                response = sess.get(url, params=params, verify=verify, **kwargs)
-            if method == HTTPVerb.POST:
-                response = sess.post(url, data=params, verify=verify, **kwargs)
+        method = method.upper()
+        if method == HTTPVerb.POST:
+            response = self._session.post(url, data=params, verify=verify, **kwargs)
+        else:
+            response = self._session.get(url, params=params, verify=verify, **kwargs)
 
         if response.status_code in [400, 429]:
             response_body = response.json()
             raise OpenMeteoRequestsError(response_body)
 
         response.raise_for_status()
-        return _process_response(response=response, handler=self._response_cls)
+        return _process_response(response=response, handler=WeatherApiResponse)
 
     def weather_api(
         self,
@@ -95,14 +93,18 @@ def weather_api(
             msg = f"failed to request {url!r}: {e}"
             raise OpenMeteoRequestsError(msg) from e
 
+    def __del__(self):
+        """cleanup"""
+        if self._close_session:
+            self._session.close()
+
 
 # pylint: disable=too-few-public-methods
 class AsyncClient:
     """Asynchronous client for Open-Meteo API."""
 
     def __init__(self, session: niquests.AsyncSession | None = None) -> None:
-        self._session = session or niquests.AsyncSession()
-        self._response_cls = WeatherApiResponse
+        self._session = session
 
     async def _request(
         self,
@@ -115,25 +117,24 @@ async def _request(
     ) -> list[WeatherApiResponse]:
         params["format"] = _FLAT_BUFFERS_FORMAT
 
-        response: niquests.Response
-        async with self._session as sess:
-            method = method.upper()
-            if method == HTTPVerb.GET:
-                meth = partial(
-                    sess.get, url, params=params, verify=verify, **kwargs
-                )
-            if method == HTTPVerb.POST:
-                meth = partial(
-                    sess.post, url, data=params, verify=verify, **kwargs
-                )
-            response = await meth()
+        method = method.upper()
+        if method == HTTPVerb.POST:
+            if self._session:
+                response = await self._session.post(url, data=params, verify=verify, **kwargs)
+            else:
+                response = await niquests.apost(url, data=params, verify=verify, **kwargs)
+        else:
+            if self._session:
+                response = await self._session.get(url, params=params, verify=verify, **kwargs)
+            else:
+                response = await niquests.aget(url, params=params, verify=verify, **kwargs)
 
         if response.status_code in [400, 429]:
             response_body = response.json()
             raise OpenMeteoRequestsError(response_body)
 
         response.raise_for_status()
-        return _process_response(response=response, handler=self._response_cls)
+        return _process_response(response=response, handler=WeatherApiResponse)
 
     async def weather_api(
         self,
diff --git a/pyproject.toml b/pyproject.toml
@@ -23,7 +23,7 @@ classifiers = [
 ]
 keywords = ["python", "open", "meteo", "weather", "client"]
 dependencies = [
-    "niquests>=3.14.1",
+    "niquests>=3.15.2",
     "openmeteo-sdk>=1.20.1",
 ]
 
@@ -90,7 +90,7 @@ asyncio_default_fixture_loop_scope = "function"
 asyncio_default_test_loop_scope = "function"
 
 [tool.ruff]
-line-length = 80
+line-length = 100
 indent-width = 4
 target-version = "py39"
 
diff --git a/tests/test_clients.py b/tests/test_clients.py
@@ -5,11 +5,42 @@
 from typing import Any
 
 import pytest
+from niquests import AsyncSession, Session
 from openmeteo_sdk.Variable import Variable
 
 from openmeteo_requests import AsyncClient, Client, OpenMeteoRequestsError
 
 
+def _process_fetchall_basic_responses(responses: list) -> None:
+    assert len(responses) == 3
+    response = responses[0]
+    assert response.Latitude() == pytest.approx(52.5)
+    assert response.Longitude() == pytest.approx(13.4)
+    response = responses[1]
+    assert response.Latitude() == pytest.approx(48.1)
+    assert response.Longitude() == pytest.approx(9.3)
+    response = responses[0]
+
+    hourly = response.Hourly()
+    hourly_variables = list(map(lambda i: hourly.Variables(i), range(hourly.VariablesLength())))
+
+    temperature_2m = next(
+        filter(
+            lambda x: x.Variable() == Variable.temperature and x.Altitude() == 2,
+            hourly_variables,
+        )
+    )
+    precipitation = next(
+        filter(
+            lambda x: x.Variable() == Variable.precipitation,
+            hourly_variables,
+        )
+    )
+
+    assert temperature_2m.ValuesLength() == 48
+    assert precipitation.ValuesLength() == 48
+
+
 @pytest.fixture
 def client() -> Client:
     return Client()
@@ -44,90 +75,58 @@ def params() -> _ParamsType:
 
 
 class TestClient:
-    def test_fetch_all(
-        self, client: Client, url: str, params: _ParamsType
-    ) -> None:
+    def test_fetch_all(self, client: Client, url: str, params: _ParamsType) -> None:
         responses = client.weather_api(url=url, params=params)
-
-        assert len(responses) == 3
-        response = responses[0]
-        assert response.Latitude() == pytest.approx(52.5)
-        assert response.Longitude() == pytest.approx(13.4)
-        response = responses[1]
-        assert response.Latitude() == pytest.approx(48.1)
-        assert response.Longitude() == pytest.approx(9.3)
-        response = responses[0]
-
-        hourly = response.Hourly()
-        hourly_variables = list(
-            map(lambda i: hourly.Variables(i), range(hourly.VariablesLength()))
-        )
-
-        temperature_2m = next(
-            filter(
-                lambda x: x.Variable() == Variable.temperature
-                and x.Altitude() == 2,
-                hourly_variables,
-            )
-        )
-        precipitation = next(
-            filter(
-                lambda x: x.Variable() == Variable.precipitation,
-                hourly_variables,
-            )
-        )
-
-        assert temperature_2m.ValuesLength() == 48
-        assert precipitation.ValuesLength() == 48
+        _process_fetchall_basic_responses(responses)
 
     def test_empty_url_error(self, client: Client, params: _ParamsType) -> None:
         with pytest.raises(OpenMeteoRequestsError):
             client.weather_api(url="", params=params)
 
+    def test_sequential_requests_with_common_session(self, url: str, params: _ParamsType) -> None:
+        session = Session()
+        client = Client(session)
+        try:
+            # OK
+            _process_fetchall_basic_responses(client.weather_api(url=url, params=params))
+            # must not fail
+            _process_fetchall_basic_responses(client.weather_api(url=url, params=params))
+        finally:
+            session.close()
+        # expected result -> the session is already closed -> failure
+        # actual result -> OK. Niquests seems to allow reuse of closed sessions
+        client.weather_api(url=url, params=params)
+        # with pytest.raises(OpenMeteoRequestsError):
+        _process_fetchall_basic_responses(client.weather_api(url=url, params=params))
+
 
 @pytest.mark.asyncio
 class TestAsyncClient:
     async def test_async_fetch_all(
         self, async_client: AsyncClient, url: str, params: _ParamsType
     ) -> None:
         responses = await async_client.weather_api(url=url, params=params)
+        _process_fetchall_basic_responses(responses)
 
-        assert len(responses) == 3
-        response = responses[0]
-        assert response.Latitude() == pytest.approx(52.5)
-        assert response.Longitude() == pytest.approx(13.4)
-        response = responses[1]
-        assert response.Latitude() == pytest.approx(48.1)
-        assert response.Longitude() == pytest.approx(9.3)
-        response = responses[0]
-
-        hourly = response.Hourly()
-        hourly_variables = list(
-            map(lambda i: hourly.Variables(i), range(hourly.VariablesLength()))
-        )
-
-        temperature_2m = next(
-            filter(
-                lambda x: x.Variable() == Variable.temperature
-                and x.Altitude() == 2,
-                hourly_variables,
-            )
-        )
-        precipitation = next(
-            filter(
-                lambda x: x.Variable() == Variable.precipitation,
-                hourly_variables,
-            )
-        )
-
-        assert temperature_2m.ValuesLength() == 48
-        assert precipitation.ValuesLength() == 48
+    async def test_empty_url_error(self, async_client: AsyncClient, params: _ParamsType) -> None:
+        with pytest.raises(OpenMeteoRequestsError):
+            await async_client.weather_api(url="", params=params)
 
-    async def test_empty_url_error(
-        self, async_client: AsyncClient, params: _ParamsType
+    async def test_sequential_requests_with_common_session(
+        self, url: str, params: _ParamsType
     ) -> None:
+        session = AsyncSession()
+        client = AsyncClient(session)
+        try:
+            # OK
+            _process_fetchall_basic_responses(await client.weather_api(url=url, params=params))
+            # must not fail
+            _process_fetchall_basic_responses(await client.weather_api(url=url, params=params))
+        finally:
+            await session.close()
+        # The session is already closed -> failure
         with pytest.raises(OpenMeteoRequestsError):
-            await async_client.weather_api(url="", params=params)
+            _process_fetchall_basic_responses(await client.weather_api(url=url, params=params))
 
 
 def test_int_client():
diff --git a/uv.lock b/uv.lock

Original file line number	Diff line number	Diff line change
`@@ -23,7 +23,7 @@ classifiers = [`
`23`	`23`	`]`
`24`	`24`	`keywords = ["python", "open", "meteo", "weather", "client"]`
`25`	`25`	`dependencies = [`
`26`		`- "niquests>=3.14.1",`
	`26`	`+ "niquests>=3.15.2",`
`27`	`27`	`"openmeteo-sdk>=1.20.1",`
`28`	`28`	`]`
`29`	`29`
`@@ -90,7 +90,7 @@ asyncio_default_fixture_loop_scope = "function"`
`90`	`90`	`asyncio_default_test_loop_scope = "function"`
`91`	`91`
`92`	`92`	`[tool.ruff]`
`93`		`-line-length = 80`
	`93`	`+line-length = 100`
`94`	`94`	`indent-width = 4`
`95`	`95`	`target-version = "py39"`
`96`	`96`