Skip to content

aiohttp migration fixes #117

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jun 7, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions onvif/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -248,12 +248,12 @@
self.dt_diff = dt_diff
self.binding_name = binding_name
# Create soap client
connector = TCPConnector(
self._connector = TCPConnector(
ssl=_NO_VERIFY_SSL_CONTEXT,
keepalive_timeout=KEEPALIVE_EXPIRY,
)
session = ClientSession(
connector=connector,
self._session = ClientSession(
connector=self._connector,
timeout=aiohttp.ClientTimeout(
total=_DEFAULT_TIMEOUT,
connect=_CONNECT_TIMEOUT,
Expand All @@ -262,12 +262,12 @@
)
self.transport = (
AsyncTransportProtocolErrorHandler(
session=session,
session=self._session,
verify_ssl=False,
)
if no_cache
else AIOHTTPTransport(
session=session,
session=self._session,
verify_ssl=False,
cache=SqliteCache(),
)
Expand Down Expand Up @@ -316,6 +316,8 @@
async def close(self):
"""Close the transport."""
await self.transport.aclose()
await self._session.close()
await self._connector.close()

Check warning on line 320 in onvif/client.py

View check run for this annotation

Codecov / codecov/patch

onvif/client.py#L319-L320

Added lines #L319 - L320 were not covered by tests

@staticmethod
@safe_func
Expand Down
2 changes: 1 addition & 1 deletion onvif/managers.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@
@property
def closed(self) -> bool:
"""Return True if the manager is closed."""
return not self._subscription or self._subscription.transport.client.is_closed
return not self._subscription or self._subscription.transport.session.closed

Check warning on line 65 in onvif/managers.py

View check run for this annotation

Codecov / codecov/patch

onvif/managers.py#L65

Added line #L65 was not covered by tests

async def start(self) -> None:
"""Setup the manager."""
Expand Down
16 changes: 5 additions & 11 deletions onvif/zeep_aiohttp.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
from zeep.utils import get_version
from zeep.wsdl.utils import etree_to_string

import aiohttp
import httpx
from aiohttp import ClientResponse, ClientSession
from requests import Response
Expand Down Expand Up @@ -160,24 +159,21 @@ async def _post(
proxy=self.proxy,
timeout=self._client_timeout,
)
response.raise_for_status()

# Read the content to log it
# Read the content to log it before checking status
content = await response.read()
_LOGGER.debug(
"HTTP Response from %s (status: %d):\n%s",
address,
response.status,
content.decode("utf-8", errors="replace"),
content,
)

# Convert to httpx Response
return self._aiohttp_to_httpx_response(response, content)

except TimeoutError as exc:
raise TimeoutError(f"Request to {address} timed out") from exc
except aiohttp.ClientError as exc:
raise ConnectionError(f"Error connecting to {address}: {exc}") from exc

async def post_xml(
self, address: str, envelope: _Element, headers: dict[str, str]
Expand Down Expand Up @@ -239,24 +235,22 @@ async def _get(
proxy=self.proxy,
timeout=self._client_timeout,
)
response.raise_for_status()

# Read content
# Read content and log before checking status
content = await response.read()

_LOGGER.debug(
"HTTP Response from %s (status: %d)",
"HTTP Response from %s (status: %d):\n%s",
address,
response.status,
content,
)

# Convert directly to requests.Response
return self._aiohttp_to_requests_response(response, content)

except TimeoutError as exc:
raise TimeoutError(f"Request to {address} timed out") from exc
except aiohttp.ClientError as exc:
raise ConnectionError(f"Error connecting to {address}: {exc}") from exc

def _httpx_to_requests_response(self, response: httpx.Response) -> Response:
"""Convert an httpx.Response object to a requests.Response object"""
Expand Down
46 changes: 44 additions & 2 deletions tests/test_zeep_transport.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ async def test_post_returns_httpx_response():
mock_aiohttp_response.url = "http://example.com/service"
mock_aiohttp_response.charset = "utf-8"
mock_aiohttp_response.cookies = {}
mock_aiohttp_response.raise_for_status = Mock()

mock_content = b"<response>test</response>"
mock_aiohttp_response.read = AsyncMock(return_value=mock_content)
Expand Down Expand Up @@ -201,7 +200,7 @@ async def test_connection_error_handling():

transport.session = mock_session

with pytest.raises(ConnectionError, match="Error connecting to"):
with pytest.raises(aiohttp.ClientError, match="Connection failed"):
await transport.get("http://example.com/wsdl")


Expand Down Expand Up @@ -869,3 +868,46 @@ async def test_cookie_jar_type():
# Verify cookies are accessible in requests response
assert hasattr(requests_result.cookies, "__getitem__")
assert "test" in requests_result.cookies


@pytest.mark.asyncio
async def test_http_error_responses_no_exception():
"""Test that HTTP error responses (401, 500, etc.) don't raise exceptions."""
mock_session = create_mock_session()
transport = AIOHTTPTransport(session=mock_session)

# Test 401 Unauthorized
mock_401_response = Mock(spec=aiohttp.ClientResponse)
mock_401_response.status = 401
mock_401_response.headers = {"Content-Type": "text/xml"}
mock_401_response.method = "POST"
mock_401_response.url = "http://example.com/service"
mock_401_response.charset = "utf-8"
mock_401_response.cookies = {}
mock_401_response.read = AsyncMock(return_value=b"<error>Unauthorized</error>")

mock_session = Mock(spec=aiohttp.ClientSession)
mock_session.post = AsyncMock(return_value=mock_401_response)
transport.session = mock_session

# Should not raise exception
result = await transport.post("http://example.com/service", "<request/>", {})
assert isinstance(result, httpx.Response)
assert result.status_code == 401
assert result.read() == b"<error>Unauthorized</error>"

# Test 500 Internal Server Error
mock_500_response = Mock(spec=aiohttp.ClientResponse)
mock_500_response.status = 500
mock_500_response.headers = {"Content-Type": "text/xml"}
mock_500_response.charset = "utf-8"
mock_500_response.cookies = {}
mock_500_response.read = AsyncMock(return_value=b"<error>Server Error</error>")

mock_session.get = AsyncMock(return_value=mock_500_response)

# Should not raise exception
result = await transport.get("http://example.com/wsdl")
assert isinstance(result, RequestsResponse)
assert result.status_code == 500
assert result.content == b"<error>Server Error</error>"