HENNGE / aiodynamo

Asynchronous, fast, pythonic DynamoDB Client
https://aiodynamo.readthedocs.io/
Other
72 stars 21 forks source link

Add response information to caught exceptions #181

Closed darro45 closed 5 months ago

darro45 commented 5 months ago

Currently, many of the exceptions caught from the DynamoDB API do not include detailed information about the service's response, making diagnosis and issue resolution challenging. I propose modifying the existing exception classes to include the service's response in each exception, providing additional useful information for debugging and error handling.

Proposed Changes:

Benefits:

darro45 commented 5 months ago

AIODynamoError can be adjusted as follows:

class AIODynamoError(Exception):
    def __init__(self, response_body: Optional[bytes] = None):
        self.response_body = response_body
        super().__init__(response_body)

And send_request can be modified as follows:

    async def send_request(
        self,
        *,
        action: str,
        payload: Mapping[str, Any],
    ) -> Dict[str, Any]:
        """
        Send a request to DynamoDB and handle retries if necessary.

        The self.throttle_config.attempts() async iterable handles retries
        by yielding each time we should do another attempt and raising
        RetryTimeout once the time limit is reached.

        In each iteration of the loop, we send a request to DynamoDB and check
        its result. If it's good, we break out of the loop and return the parsed
        JSON data to the caller.
        If a non-200 status is returned, we determine the error type via
        exception_from_response and check if we should retry. If we should retry,
        log the error and go to the next iteration, otherwise raise the exception.
        In either case, we store the last exception found so if we hit the retry
        limit, we raise that exception.
        If the loop never executed, we raise a BrokenThrottleConfig because
        RetryConfig.attempts() should always yield at least once.
        """
        exception: Optional[Exception] = None
        try:
            async for _ in self.throttle_config.attempts():
                key = await self.credentials.get_key(self.http)
                if key is None:
                    logger.debug("no credentials found")
                    exception = NoCredentialsFound()
                    continue
                request = signed_dynamo_request(
                    key=key,
                    payload=payload,
                    action=action,
                    region=self.region,
                    endpoint=self.endpoint,
                )
                logger.debug("sending request %r", request)
                try:
                    response = await self.http(
                        Request(
                            method="POST",
                            url=str(request.url),
                            headers=request.headers,
                            body=request.body,
                        )
                    )
                except asyncio.TimeoutError as exc:
                    logger.debug("http timeout")
                    exception = exc
                    continue
                except RequestFailed as exc:
                    logger.debug("request failed")
                    exception = exc.inner
                    continue
                logger.debug("got response %r", response)
                if response.status == 200:
                    return cast(Dict[str, Any], json.loads(response.body))
                exception = exception_from_response(response.status, response.body)
                if isinstance(exception, AIODynamoError):
                    exception.response_body = response.body
                if isinstance(exception, Throttled):
                    logger.debug("request throttled")
                elif isinstance(exception, ProvisionedThroughputExceeded):
                    logger.debug("provisioned throughput exceeded")
                elif isinstance(exception, ExpiredToken):
                    logger.debug("token expired")
                    if not self.credentials.invalidate():
                        raise exception
                elif isinstance(exception, ServiceUnavailable):
                    logger.debug("service unavailable")
                elif isinstance(exception, InternalDynamoError):
                    logger.debug("internal dynamo error")
                else:
                    raise exception
        except RetryTimeout:
            if exception is not None:
                raise exception
            raise
        raise BrokenThrottleConfig()
dimaqq commented 5 months ago

My 2c: I'm not sure if the enhancement is really needed, however, if the response is an error response, there's hardly any overhead.

I'd entertain a draft PR to understand the proposal in detail.

What I wonder though, is to expose the error details in a convenient way. Let's say the caller does except Exception: logging.exception("oops"), should the extra info show up in __str__ or __repr__ of the exception? Some other way?

ojii commented 5 months ago

known AIODynamoError already store the (json-decoded) response in .args[0]. UnkownError stores both the status and the (raw) response. Is that not enough?

darro45 commented 5 months ago

@ojii You're right. Thank you.