Skip to content

Commit

Permalink
Merge pull request #1199 from lsst-sqre/tickets/DM-48319
Browse files Browse the repository at this point in the history
Capture more information in Firestore exceptions
  • Loading branch information
rra authored Jan 7, 2025
2 parents d19628d + 252f5d8 commit c0751dc
Show file tree
Hide file tree
Showing 2 changed files with 85 additions and 2 deletions.
57 changes: 55 additions & 2 deletions src/gafaelfawr/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,21 @@

from __future__ import annotations

from typing import ClassVar
from typing import ClassVar, Self, override

import kopf
from fastapi import status
from google.api_core.exceptions import GoogleAPICallError
from pydantic import ValidationError
from safir.fastapi import ClientRequestError
from safir.models import ErrorLocation
from safir.slack.blockkit import SlackException, SlackWebException
from safir.slack.blockkit import (
SlackCodeBlock,
SlackException,
SlackMessage,
SlackTextBlock,
SlackWebException,
)

__all__ = [
"DatabaseSchemaError",
Expand Down Expand Up @@ -343,6 +350,52 @@ class FirestoreError(ExternalUserInfoError):
"""An error occurred while reading or updating Firestore data."""


class FirestoreAPIError(FirestoreError):
"""A Google API error occurred while talking to Firestore."""

@classmethod
def from_exception(cls, exc: GoogleAPICallError) -> Self:
"""Create an exception from a Google API failure exception.
Parameters
----------
exc
Underlying Google API exception.
Returns
-------
FirestoreAPIError
Corresponding wrapped exception.
"""
msg = f"Firestore API call failed (status {exc.code!s}): {exc!s}"
return cls(msg, errors=[str(e) for e in exc.errors], reason=exc.reason)

def __init__(
self, message: str, *, errors: list[str], reason: str | None = None
) -> None:
super().__init__(message)
self._errors = errors
self._reason = reason

@override
def to_slack(self) -> SlackMessage:
"""Format the exception for Slack reporting.
Returns
-------
SlackMessage
Message suitable for sending to Slack.
"""
result = super().to_slack()
if self._reason:
block = SlackTextBlock(heading="Reason", text=self._reason)
result.blocks.append(block)
if self._errors:
errors = "\n".join(self._errors)
result.blocks.append(SlackCodeBlock(heading="Errors", code=errors))
return result


class FirestoreNotInitializedError(FirestoreError):
"""Firestore has not been initialized."""

Expand Down
30 changes: 30 additions & 0 deletions src/gafaelfawr/storage/firestore.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,11 @@

from __future__ import annotations

from collections.abc import Callable, Coroutine
from functools import wraps

import sentry_sdk
from google.api_core.exceptions import GoogleAPICallError
from google.cloud import firestore
from structlog.stdlib import BoundLogger

Expand All @@ -14,6 +18,7 @@
UID_USER_MIN,
)
from ..exceptions import (
FirestoreAPIError,
FirestoreNotInitializedError,
NoAvailableGidError,
NoAvailableUidError,
Expand All @@ -29,6 +34,21 @@
__all__ = ["FirestoreStorage"]


def _convert_exception[**P, T](
f: Callable[P, Coroutine[None, None, T]],
) -> Callable[P, Coroutine[None, None, T]]:
"""Convert Firestore API exceptions to `FirestoreAPIError`."""

@wraps(f)
async def wrapper(*args: P.args, **kwargs: P.kwargs) -> T:
try:
return await f(*args, **kwargs)
except GoogleAPICallError as e:
raise FirestoreAPIError.from_exception(e) from e

return wrapper


class FirestoreStorage:
"""Google Firestore storage layer.
Expand All @@ -55,6 +75,7 @@ def __init__(
self._logger = logger

@sentry_sdk.trace
@_convert_exception
async def get_gid(self, group: str) -> int:
"""Get the GID for a group.
Expand Down Expand Up @@ -90,6 +111,7 @@ async def get_gid(self, group: str) -> int:
)

@sentry_sdk.trace
@_convert_exception
async def get_uid(self, username: str, *, bot: bool = False) -> int:
"""Get the UID for a user.
Expand All @@ -112,6 +134,8 @@ async def get_uid(self, username: str, *, bot: bool = False) -> int:
Raises
------
FirestoreError
Raised if some error occurs talking to Firestore.
FirestoreNotInitializedError
Raised if Firestore has not been initialized.
NoAvailableUidError
Expand All @@ -130,11 +154,17 @@ async def get_uid(self, username: str, *, bot: bool = False) -> int:
logger=self._logger,
)

@_convert_exception
async def initialize(self) -> None:
"""Initialize a Firestore document store for UID/GID assignment.
This is safe to call on an already-initialized document store and will
silently do nothing.
Raises
------
FirestoreError
Raised if some error occurs talking to Firestore.
"""
counter_refs = {
n: self._client.collection("counters").document(n)
Expand Down

0 comments on commit c0751dc

Please sign in to comment.