Skip to content
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

DM-38660: Add SlackWebException for HTTPX errors #154

Merged
merged 1 commit into from
Apr 12, 2023
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
11 changes: 6 additions & 5 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,21 +20,22 @@ X.Y.Z (YYYY-MM-DD)
- The new `safir.redis.PydanticRedisStorage` class enables you to store Pydantic model objects in Redis.
A `safir.redis.EncryptedPydanticRedisStorage` class also encrypts data in Redis.
To use the `safir.redis` module, install Safir with the `redis` extra (i.e., `pip install safir[redis]`).
- Add `safir.slack.webhook.SlackWebException`, which is a child class of `safir.slack.webhook.SlackException` that knows how to capture and report details from an underlying HTTPX exception.
- Add a `safir.testing.kubernetes.strip_none` helper function that makes it easier to compare Kubernetes objects against expected test data.
- Add a `get_namespace_objects_for_test` method to the Kubernetes mock to retreive all objects (of any kind) in a namespace.
- Add a mock for the `list_nodes` Kubernetes API, which returns data set by a call to the new `set_nodes_for_test` method.
- Add optional rudimentary namespace tracking to the Kubernetes mock. Namespaces can be created, deleted (which deletes all objects in the namespace), read, and listed. Explicit namespace creation remains optional; if an object is created in a namespace and that namespace does not exist, one will be implicitly created by the mock.
- Add support for namespaced events (the older core API, not the new events API) to the Kubernetes mock. Newly-created pods with the default initial status post an event by default; otherwise, events must be created by calling the mocked `create_namespaced_event` API. The `list_namespaced_event` API supports watches with timeouts.
- Add rudimentary support for `NetworkPolicy`, `ResourceQuota`, and `Service` objects to the Kubernetes mock.

### Other changes

- The `safir.testing.kubernetes.MockKubernetesApi` mock now has rudimentary API documentation for the Kubernetes APIs that it supports.

### Bug fixes

- Stop adding the `X-Auth-Request-User` header to the OpenAPI schema for routes that use `auth_dependency` or `auth_logger_dependency`.

### Other changes

- The `safir.testing.kubernetes.MockKubernetesApi` mock now has rudimentary API documentation for the Kubernetes APIs that it supports.

## 3.8.0 (2023-03-15)

### New features
Expand Down Expand Up @@ -166,7 +167,7 @@ X.Y.Z (YYYY-MM-DD)

### Bug fixes

- Restore previous `http_client_dependency` behavior by enabling following redirects by default. This adjusts for the change of defaults in httpx 0.20.0.
- Restore previous `http_client_dependency` behavior by enabling following redirects by default. This adjusts for the change of defaults in HTTPX 0.20.0.

## 2.1.1 (2021-10-29)

Expand Down
1 change: 1 addition & 0 deletions docs/_rst_epilog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,4 @@
.. _PyPI: https://pypi.org/project/safir/
.. _redis-py: https://redis.readthedocs.io/en/stable/
.. _respx: https://lundberg.github.io/respx/
.. _HTTPX: https://www.python-httpx.org/
6 changes: 6 additions & 0 deletions docs/api.rst
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,10 @@ API reference
:include-all-objects:

.. automodapi:: safir.database
:include-all-objects:

.. automodapi:: safir.datetime
:include-all-objects:

.. automodapi:: safir.dependencies.arq
:include-all-objects:
Expand Down Expand Up @@ -59,8 +61,10 @@ API reference
:inherited-members:

.. automodapi:: safir.slack.blockkit
:include-all-objects:

.. automodapi:: safir.slack.webhook
:include-all-objects:

.. automodapi:: safir.testing.gcs
:include-all-objects:
Expand All @@ -69,5 +73,7 @@ API reference
:include-all-objects:

.. automodapi:: safir.testing.slack
:include-all-objects:

.. automodapi:: safir.testing.uvicorn
:include-all-objects:
4 changes: 2 additions & 2 deletions docs/user-guide/http-client.rst
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
######################
Using the httpx client
Using the HTTPX client
######################

Safir helps you manage a single ``httpx.AsyncClient`` for your application.
Using a single HTTP client improves performance by reusing connections.
Using a single HTTPX_ client improves performance by reusing connections.

Setting up the httpx.AsyncClient
================================
Expand Down
41 changes: 41 additions & 0 deletions docs/user-guide/slack-webhook.rst
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,47 @@ For example:

The full exception message (although not the traceback) is sent to Slack, so it should not contain any sensitive information, security keys, or similar data.

Reporting HTTPX exceptions
--------------------------

A common source of exceptions in Safir applications are exceptions raised by HTTPX_ while making calls to other web services.
Safir provides a base class for those exceptions, `~safir.slack.blockkit.SlackWebException`, which behaves the same as `~safir.slack.blockkit.SlackException` but captures additional information from the underlying HTTPX exception.

The advantages of `~safir.slack.blockkit.SlackWebException` over using `~safir.slack.blockkit.SlackException` directly, possibly with the text of the HTTPX exception, are:

- If the exception is due to an error returned by the remote server, the stringification of the exception, and the main Slack message if posted to Slack, always includes the URL, method, and status code.
(You therefore will want to override the ``__str__`` method if your URLs may contain secret data that should not be sent in Slack alerts, such as Slack webhook URLs.)
- The body of any reply is included in the stringification and in a block of the Slack message.
(Again, override this behavior if the bodies of error replies may include secrets that should not be sent to Slack.)
- For other exceptions, the stringification and main Slack message include both the type and the stringification of the underlying exception.
- Where possible, the URL and method are included in a field in the Slack message.

The normal way to use this class or exception classes derived from it is to call the class method ``from_exception``, passing in the underlying HTTPX exception.
For example:

.. code-block:: python

from httpx import AsyncClient, HTTPError
from safir.slack.blockkit import SlackWebException


class FooServiceError(SlackWebException):
"""An error occurred sending a request to the foo service."""


async def do_something(client: AsyncClient) -> None:
# ... set up some request to the foo service ...
try:
r = await client.get(url)
r.raise_for_status()
except HTTPError as e:
raise FooServiceError.from_exception(e) from e

Note the ``from e`` clause when raising the derived exception, which tells Python to include the backtraces from both exceptions.
Higher-level code may then catch this exception and post it to Slack if desired.

As with `~safir.slack.blockkit.SlackException`, a username may be provided as a second argument to ``from_exception`` or set later by catching the exception, setting its ``user`` attribute, and re-raising it.

.. _slack-uncaught-exceptions:

Reporting uncaught exceptions to a Slack webhook
Expand Down
2 changes: 1 addition & 1 deletion docs/user-guide/uvicorn.rst
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
Starting an app with Uvicorn for testing
########################################

Normally, testing of FastAPI apps should be done by passing the app to ``httpx.AsyncClient`` and using httpx's built-in support for sending requests directly to an ASGI application.
Normally, testing of FastAPI apps should be done by passing the app to ``httpx.AsyncClient`` and using HTTPX's built-in support for sending requests directly to an ASGI application.
To do this, use the ``client`` fixture provided by the ``fastapi_safir_app`` template (see :ref:`create-from-template`).

However, in some test scenarios it may be necessary for the app being tested to respond to regular HTTP requests, such as for Selenium_ testing with a browser.
Expand Down
2 changes: 1 addition & 1 deletion src/safir/dependencies/http_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
DEFAULT_HTTP_TIMEOUT = 20.0
"""Default timeout (in seconds) for outbound HTTP requests.

The default httpx timeout has proven too short in practice for calls to, for
The default HTTPX timeout has proven too short in practice for calls to, for
example, GitHub for authorization verification. Increase the default to 20
seconds. Users of this dependency can always lower it if needed.
"""
Expand Down
115 changes: 114 additions & 1 deletion src/safir/slack/blockkit.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,9 @@

from abc import ABCMeta, abstractmethod
from datetime import datetime
from typing import Any, ClassVar, Optional
from typing import Any, ClassVar, Optional, Self

from httpx import HTTPError, HTTPStatusError, RequestError
from pydantic import BaseModel, validator

from safir.datetime import current_datetime, format_datetime_for_logging
Expand All @@ -19,6 +20,7 @@
"SlackMessage",
"SlackTextBlock",
"SlackTextField",
"SlackWebException",
]


Expand Down Expand Up @@ -280,6 +282,117 @@ def to_slack(self) -> SlackMessage:
return SlackMessage(message=str(self), fields=fields)


class SlackWebException(SlackException):
"""Parent class of exceptions arising from HTTPX_ failures.

Captures additional information from any HTTPX_ exception. Intended to be
subclassed. Subclasses may wish to override the ``to_slack`` method.

Parameters
----------
message
Exception string value, which is the default Slack message.
failed_at
When the exception happened. Omit to use the current time.
method
Method of request.
url
URL of the request.
user
Username on whose behalf the request is being made.
status
Status code of failure, if any.
body
Body of failure message, if any.
"""

@classmethod
def from_exception(
cls, exc: HTTPError, user: Optional[str] = None
) -> Self:
"""Create an exception from an HTTPX_ exception.

Parameters
----------
exc
Exception from HTTPX.
user
User on whose behalf the request is being made, if known.

Returns
-------
SlackWebException
Newly-constructed exception.
"""
if isinstance(exc, HTTPStatusError):
status = exc.response.status_code
method = exc.request.method
message = f"Status {status} from {method} {exc.request.url}"
return cls(
message,
method=exc.request.method,
url=str(exc.request.url),
user=user,
status=status,
body=exc.response.text,
)
else:
message = f"{type(exc).__name__}: {str(exc)}"
if isinstance(exc, RequestError):
return cls(
message,
method=exc.request.method,
url=str(exc.request.url),
user=user,
)
else:
return cls(message, user=user)

def __init__(
self,
message: str,
*,
failed_at: Optional[datetime] = None,
method: Optional[str] = None,
url: Optional[str] = None,
user: Optional[str] = None,
status: Optional[int] = None,
body: Optional[str] = None,
) -> None:
self.message = message
self.method = method
self.url = url
self.status = status
self.body = body
super().__init__(message, user, failed_at=failed_at)

def __str__(self) -> str:
result = self.message
if self.body:
result += f"\nBody:\n{self.body}\n"
return result

def to_slack(self) -> SlackMessage:
"""Convert to a Slack message for Slack alerting.

Returns
-------
SlackMessage
Slack message suitable for posting as an alert.
"""
message = super().to_slack()
if self.url:
if self.method:
text = f"{self.method} {self.url}"
else:
text = self.url
message.blocks.append(SlackTextField(heading="URL", text=text))
if self.body:
block = SlackCodeBlock(heading="Response", code=self.body)
message.blocks.append(block)
return message


def _format_and_truncate_at_end(string: str, max_length: int) -> str:
"""Format a string for Slack, truncating at the end.

Expand Down
2 changes: 1 addition & 1 deletion src/safir/testing/uvicorn.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
"""Utiility functions for managing an external Uvicorn test process.

Normally, ASGI apps are tested via the built-in support in httpx for running
Normally, ASGI apps are tested via the built-in support in HTTPX for running
an ASGI app directly. However, sometimes the app has to be spawned in a
separate process so that it can be accessed over HTTP, such as when testing it
with Selenium or when testing Uvicorn integration. This module provides
Expand Down
2 changes: 1 addition & 1 deletion tests/middleware/x_forwarded_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ async def handler(request: Request) -> dict[str, str]:
async def test_too_many_headers() -> None:
"""Test handling of duplicate headers.

httpx doesn't allow passing in duplicate headers, so we cannot test end to
HTTPX doesn't allow passing in duplicate headers, so we cannot test end to
end. Instead, test by generating a mock request and then calling the
underling middleware functions directly.
"""
Expand Down
Loading