Skip to content

fix: error handling #479

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.

Sign up for GitHub

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
alexanto merged 1 commit into from
Nov 12, 2026
Merged

fix: error handling #479

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
56 changes: 28 additions & 28 deletions UnleashClient/api_client/_async.py
View file
Open in desktop

@ppicom ppicom Nov 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

docstrings are no longer accurate. They say only exceptions from the transport become False. Now a response the SDK can't read does too.

Original file line number Diff line number Diff line change
Expand Up @@ -164,8 +164,8 @@ async def register(self, payload: Dict[str, Any]) -> bool:
Register this client with the server.

Returns True on 200 or 202, False on any other status and when the
request fails: any exception from the transport is logged and reported
as False.
request fails: an exception from the transport, or a response the SDK
can't read, is logged and reported as False.

:param payload: as built by
:func:`UnleashClient._payloads._build_register_payload`.
Expand All @@ -185,30 +185,30 @@ async def register(self, payload: Dict[str, Any]) -> bool:
)
try:
resp = await self._transport.execute(request)

if resp.status not in {200, 202}:
_log_resp_info(resp)
LOGGER.warning(
"Unleash Client registration failed due to unexpected HTTP status code: %s",
resp.status,
)
return False

LOGGER.info("Unleash Client successfully registered!")

return True
except Exception as exc: # pylint: disable=broad-except
LOGGER.exception(
"Unleash Client registration failed due to exception: %s", exc
)
return False

if resp.status not in {200, 202}:
_log_resp_info(resp)
LOGGER.warning(
"Unleash Client registration failed due to unexpected HTTP status code: %s",
resp.status,
)
return False

LOGGER.info("Unleash Client successfully registered!")

return True

async def send_metrics(self, payload: Dict[str, Any]) -> bool:
"""
Send one metrics bucket.

Returns True only on 202. Every other status and any exception from the
transport return False.
Returns True only on 202. Every other status, an exception from the
transport and a response the SDK can't read return False.

:param payload: the metrics request body.
:raises AlreadyClosedError: if the API client has been closed.
Expand All @@ -228,6 +228,18 @@ async def send_metrics(self, payload: Dict[str, Any]) -> bool:
)
try:
resp = await self._transport.execute(request)

if resp.status != 202:
_log_resp_info(resp)
LOGGER.warning(
"Unleash Client metrics submission due to unexpected HTTP status code: %s",
resp.status,
)
return False

LOGGER.info("Unleash Client metrics successfully sent!")

return True
except asyncio.CancelledError:
LOGGER.warning("Unleash Client metrics submission was cancelled.")
raise
Expand All @@ -239,15 +251,3 @@ async def send_metrics(self, payload: Dict[str, Any]) -> bool:
"Unleash Client metrics submission failed due to exception: %s", exc
)
return False

if resp.status != 202:
_log_resp_info(resp)
LOGGER.warning(
"Unleash Client metrics submission due to unexpected HTTP status code: %s",
resp.status,
)
return False

LOGGER.info("Unleash Client metrics successfully sent!")

return True
61 changes: 30 additions & 31 deletions UnleashClient/api_client/_sync.py
View file
Open in desktop

@ppicom ppicom Nov 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

docstrings are no longer accurate. They say only exceptions from the transport become False. Now a response the SDK can't read does too.

Original file line number Diff line number Diff line change
Expand Up @@ -138,12 +138,12 @@ def register(self, payload: Dict[str, Any]) -> bool:
"""
Register this client with the Unleash server.

Any exception from the transport is a failed request: it is logged and
False is returned.
Never raises for a failed request: an exception from the transport, or
a response the SDK can't read, is logged and False is returned.

:param payload: Registration request body.
:return: True on 200 or 202; False on any other status and on a request
error.
:return: True on 200 or 202; False on any other status, on a request
error and on a response the SDK can't read.
"""
config = self._config
LOGGER.info("Registering unleash client with unleash @ %s", config.url)
Expand All @@ -157,33 +157,33 @@ def register(self, payload: Dict[str, Any]) -> bool:
)
try:
resp = self._transport.execute(request)
if resp.status not in {200, 202}:
_log_resp_info(resp)
LOGGER.warning(
"Unleash Client registration failed due to unexpected HTTP status code: %s",
resp.status,
)
return False

LOGGER.info("Unleash Client successfully registered!")

return True
except Exception as exc: # pylint: disable=broad-except
LOGGER.exception(
"Unleash Client registration failed due to exception: %s", exc
)
return False

if resp.status not in {200, 202}:
_log_resp_info(resp)
LOGGER.warning(
"Unleash Client registration failed due to unexpected HTTP status code: %s",
resp.status,
)
return False

LOGGER.info("Unleash Client successfully registered!")

return True

def send_metrics(self, payload: Dict[str, Any]) -> bool:
"""
Send one metrics bucket to the Unleash server.

Any exception from the transport is a failed request: it is logged and
False is returned.
Never raises for a failed request: an exception from the transport, or
a response the SDK can't read, is logged and False is returned.

:param payload: Metrics request body.
:return: True on 202; False on any other status and on a request error.
:return: True on 202; False on any other status, on a request error and
on a response the SDK can't read.
"""
config = self._config
LOGGER.info("Sending messages to with unleash @ %s", config.url)
Expand All @@ -197,20 +197,19 @@ def send_metrics(self, payload: Dict[str, Any]) -> bool:
)
try:
resp = self._transport.execute(request)
if resp.status != 202:
_log_resp_info(resp)
LOGGER.warning(
"Unleash Client metrics submission due to unexpected HTTP status code: %s",
resp.status,
)
return False

LOGGER.info("Unleash Client metrics successfully sent!")

return True
except Exception as exc: # pylint: disable=broad-except
LOGGER.warning(
"Unleash Client metrics submission failed due to exception: %s", exc
)
return False

if resp.status != 202:
_log_resp_info(resp)
LOGGER.warning(
"Unleash Client metrics submission due to unexpected HTTP status code: %s",
resp.status,
)
return False

LOGGER.info("Unleash Client metrics successfully sent!")

return True
8 changes: 8 additions & 0 deletions tests/unit_tests/test_async_unleash_api_client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -627,6 +627,14 @@ async def test_any_transport_exception_is_a_failed_request(exc):
assert await api_client.send_metrics(MOCK_METRICS_REQUEST) is False


@mark.asyncio
async def test_register_and_send_metrics_never_raise_on_a_broken_response():
api_client = api_client_over(RecordingTransport(object()))

assert await api_client.register({}) is False
assert await api_client.send_metrics({}) is False


@mark.asyncio
async def test_a_transport_closed_by_its_creator_gives_failed_requests():
transport = AiohttpTransport()
Expand Down
7 changes: 7 additions & 0 deletions tests/unit_tests/test_unleash_api_client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -461,6 +461,13 @@ def test_any_transport_exception_is_a_failed_request(exc):
assert api_client.send_metrics({}) is False


def test_register_and_send_metrics_never_raise_on_a_broken_response():
api_client = api_client_over(RecordingTransport(object()))

assert api_client.register({}) is False
assert api_client.send_metrics({}) is False


@mark.parametrize(
"call",
(
Expand Down
Loading