Skip to content

refactor: error handling #471

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

Open
alexanto wants to merge 1 commit into from
Open

refactor: error handling #471

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
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
* (Major): The API token is passed as `api_key="<API token>"`. An `Authorization` header in `HttpOptions.headers` raises `ValueError`.
* (Major): The deprecated `environment` setting has been removed. The `environment` field of the evaluation context and the impact metrics `environment` label come from the API token, or are `"default"` when the token doesn't name one.
* (Major): The `UnleashClient` and `AsyncUnleashClient` constructors raise `ValueError` if `url` is not an absolute http(s) URL with a host, for every client.
* (Major): The constructors raise `ValueError` if `api_key`, `app_name` or `IdentityOptions.instance_id` contains a line break, such as an API token read from a file with a trailing newline, and `TypeError` if one isn't a str. These values are sent as headers.
* (Major): `initialize_client()` no longer raises when registration fails. A failed registration is logged and the client keeps running, the same way for both clients.
* (Major): Upgraded to yggdrasil-engine 3, new engine built with PyO3 for better performance.
* (Minor): New `FETCH_COMPLETED` event, emitted after every feature fetch with how it ended: `CHANGED`, `NOT_CHANGED`, `UNAUTHORIZED`, `RATE_LIMITED`, `UNAVAILABLE` or `INVALID_RESPONSE`.
* (Minor): `feature_definitions()` reports whether each feature is enabled in the environment.
Expand Down
122 changes: 56 additions & 66 deletions UnleashClient/_async_unleash_api_client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,6 @@

RETRYABLE_ERRORS = (aiohttp.ClientConnectionError, asyncio.TimeoutError)

FATAL_URL_ERRORS = (aiohttp.InvalidURL, aiohttp.NonHttpUrlClientError)


class _AsyncUnleashApiClient:
"""The asyncio twin of :class:`UnleashClient._unleash_api_client._UnleashApiClient`."""
Expand Down Expand Up @@ -187,9 +185,9 @@ 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 on a general
``ClientError``. The two errors aiohttp uses for a url it cannot make a
request against at all are re-raised rather than swallowed.
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.

:param payload: as built by
:func:`UnleashClient._payloads._build_register_payload`.
Expand All @@ -198,48 +196,41 @@ async def register(self, payload: Dict[str, Any]) -> bool:
self._raise_if_closed()

config = self._config
LOGGER.info("Registering unleash client with unleash @ %s", config.url)
LOGGER.info("Registration request information: %s", payload)

request = Request(
method="POST",
url=_normalized_url(config.url, REGISTER_URL),
headers=self._headers.base(),
body=json.dumps(payload).encode("utf-8"),
)
try:
LOGGER.info("Registering unleash client with unleash @ %s", config.url)
LOGGER.info("Registration request information: %s", payload)

resp = await self._transport.execute(
Request(
method="POST",
url=_normalized_url(config.url, REGISTER_URL),
headers=self._headers.base(),
body=json.dumps(payload).encode("utf-8"),
)
)

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 FATAL_URL_ERRORS as exc:
LOGGER.exception(
"Unleash Client registration failed fatally due to exception: %s", exc
)
raise exc
except aiohttp.ClientError as exc:
resp = await self._transport.execute(request)
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

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, a ``ClientError`` and a
timeout all return False.
Returns True only on 202. Every other status and any exception from the
transport return False.

:param payload: the metrics request body.
:raises AlreadyClosedError: if the API client has been closed.
Expand All @@ -248,38 +239,37 @@ async def send_metrics(self, payload: Dict[str, Any]) -> bool:
self._raise_if_closed()

config = self._config
LOGGER.info("Sending messages to with unleash @ %s", config.url)
LOGGER.info("unleash metrics information: %s", payload)

request = Request(
method="POST",
url=_normalized_url(config.url, METRICS_URL),
headers=self._headers.metrics(),
body=json.dumps(payload).encode("utf-8"),
)
try:
LOGGER.info("Sending messages to with unleash @ %s", config.url)
LOGGER.info("unleash metrics information: %s", payload)

resp = await self._transport.execute(
Request(
method="POST",
url=_normalized_url(config.url, METRICS_URL),
headers=self._headers.metrics(),
body=json.dumps(payload).encode("utf-8"),
)
resp = await self._transport.execute(request)
except asyncio.CancelledError:
LOGGER.warning("Unleash Client metrics submission was cancelled.")
raise
except asyncio.TimeoutError:
LOGGER.warning("Unleash Client metrics submission timed out.")
return False
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
except aiohttp.ClientError as exc:
if resp.status != 202:
_log_resp_info(resp)
LOGGER.warning(
"Unleash Client metrics submission failed due to exception: %s", exc
"Unleash Client metrics submission due to unexpected HTTP status code: %s",
resp.status,
)
except asyncio.TimeoutError:
LOGGER.warning("Unleash Client metrics submission timed out.")
except asyncio.CancelledError:
LOGGER.warning("Unleash Client metrics submission was cancelled.")
raise
return False

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

return False
return True
111 changes: 50 additions & 61 deletions UnleashClient/_unleash_api_client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,6 @@
import uuid
from typing import Any, Dict, NamedTuple, Optional, Tuple

import requests
from requests.exceptions import InvalidHeader, InvalidSchema, InvalidURL, MissingSchema

from UnleashClient._event_dispatcher import _EventDispatcher
from UnleashClient._headers import _HeaderFactory
from UnleashClient._transport import Request, RequestsTransport, Response, Transport
Expand Down Expand Up @@ -206,87 +203,79 @@ def register(self, payload: Dict[str, Any]) -> bool:
"""
Register this client with the Unleash server.

Malformed-URL errors (``MissingSchema``, ``InvalidSchema``,
``InvalidHeader``, ``InvalidURL``) propagate, so ``initialize_client()``
fails loudly instead of starting a client that can never reach the
server.
Any exception from the transport is a failed request: it 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.
"""
config = self._config
LOGGER.info("Registering unleash client with unleash @ %s", config.url)
LOGGER.info("Registration request information: %s", payload)

request = Request(
method="POST",
url=_normalized_url(config.url, REGISTER_URL),
headers=self._headers.base(),
body=json.dumps(payload).encode("utf-8"),
)
try:
LOGGER.info("Registering unleash client with unleash @ %s", config.url)
LOGGER.info("Registration request information: %s", payload)

resp = self._transport.execute(
Request(
method="POST",
url=_normalized_url(config.url, REGISTER_URL),
headers=self._headers.base(),
body=json.dumps(payload).encode("utf-8"),
)
)

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 (MissingSchema, InvalidSchema, InvalidHeader, InvalidURL) as exc:
LOGGER.exception(
"Unleash Client registration failed fatally due to exception: %s", exc
)
raise exc
except requests.RequestException as exc:
resp = self._transport.execute(request)
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 False
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.

:param payload: Metrics request body.
:return: True on 202; False on any other status and on a request error.
"""
config = self._config
LOGGER.info("Sending messages to with unleash @ %s", config.url)
LOGGER.info("unleash metrics information: %s", payload)

request = Request(
method="POST",
url=_normalized_url(config.url, METRICS_URL),
headers=self._headers.metrics(),
body=json.dumps(payload).encode("utf-8"),
)
try:
LOGGER.info("Sending messages to with unleash @ %s", config.url)
LOGGER.info("unleash metrics information: %s", payload)

resp = self._transport.execute(
Request(
method="POST",
url=_normalized_url(config.url, METRICS_URL),
headers=self._headers.metrics(),
body=json.dumps(payload).encode("utf-8"),
)
resp = self._transport.execute(request)
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
except requests.RequestException as exc:
if resp.status != 202:
_log_resp_info(resp)
LOGGER.warning(
"Unleash Client metrics submission failed due to exception: %s", exc
"Unleash Client metrics submission due to unexpected HTTP status code: %s",
resp.status,
)
return False

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

return False
return True
7 changes: 1 addition & 6 deletions UnleashClient/clients/async_unleash_client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
from UnleashClient.constants import ETAG, METRIC_LAST_SENT_TIME
from UnleashClient.events import BaseEvent
from UnleashClient.impact_metrics import ImpactMetrics
from UnleashClient.utils import LOGGER, InstanceAllowType, _validate_url
from UnleashClient.utils import LOGGER, InstanceAllowType


class AsyncUnleashClient:
Expand Down Expand Up @@ -308,9 +308,6 @@ async def initialize_client(self, fetch_toggles: bool = True) -> None:
:param fetch_toggles: Accepted for parity with :class:`UnleashClient`. It
has no effect: the client always polls.
:raises ValueError: If a custom strategy is invalid.
:raises aiohttp.InvalidURL: If registration is enabled and aiohttp
rejects a URL that passed the constructor's
check.
"""
if self._closed or self._starting or self._run_state > _RunState.UNINITIALIZED:
warnings.warn(
Expand All @@ -320,8 +317,6 @@ async def initialize_client(self, fetch_toggles: bool = True) -> None:

self._starting = True
try:
_validate_url(self._config.url)

self._cache.mset(
{METRIC_LAST_SENT_TIME: datetime.now(timezone.utc), ETAG: ""}
)
Expand Down
8 changes: 1 addition & 7 deletions UnleashClient/clients/unleash_client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,7 @@
UnleashReadyEvent,
)
from UnleashClient.impact_metrics import ImpactMetrics
from UnleashClient.utils import (
LOGGER,
InstanceAllowType,
_validate_url,
)
from UnleashClient.utils import LOGGER, InstanceAllowType

INSTANCES = _get_instance_registry()

Expand Down Expand Up @@ -290,8 +286,6 @@ def initialize_client(self, fetch_toggles: bool = True) -> None:
try:
start_scheduler = False

_validate_url(self._config.url)

if self._custom_strategies:
self._engine.register_custom_strategies(self._custom_strategies)

Expand Down
Loading
Loading