omarryhan/aiogoogle

HttpError doesn't include JSON error response with the "pipe_to" option enabled

Opened this issue · 1 comments

Hi, I came across inconsistent behaviour with the reason in the HttpError exception. It seems to depend on the pipe_to function argument being present or not. If we have a pipe_to argument, the non-200 response doesn't have the error reason field populated.

Why is it important?

According to google docs there are at least 8 possible reasons for getting a 403 response. The recommended approach to dealing with 403s depends on the returned reason in the non-200 response.

Code example to reproduce

import asyncio
from aiofiles.tempfile import NamedTemporaryFile
from aiogoogle import Aiogoogle
from aiogoogle.auth.creds import ServiceAccountCreds

creds = {...}

async def example_api_call(**kwargs):
    async with Aiogoogle(
        service_account_creds=ServiceAccountCreds(
            scopes=["https://www.googleapis.com/auth/drive.readonly"],
            **creds,
        )
    ) as google_client:
        drive = await google_client.discover(
            api_name="drive", api_version="v3"
        )
        first_page_with_next_attached = (
            await google_client.as_service_account(
                drive.files.export(**kwargs),
                full_res=True
            )
        )
        async for page_items in first_page_with_next_attached:
            yield page_items


async def example():
    await anext(example_api_call(
          fileId="...",
          mimeType="application/vnd.openxmlformats-officedocument.spreadsheetml.sheet",
        ))

In this scenario, when calling the client directly, the 403 response will cause a HttpError to be thrown with a reason:

Forbidden

Content:
{'code': 403,
 'errors': [{'domain': 'global',
             'message': 'This file is too large to be exported.',
             'reason': 'exportSizeLimitExceeded'}],
 'message': 'This file is too large to be exported.'}

Request URL:
https://www.googleapis.com/drive/v3/files/...

But when using a pipe_to argument, the HttpError won't contain the error reason.

async def example():
    async with NamedTemporaryFile(mode="wb", delete=False) as async_buffer:
        await anext(example_api_call(
              fileId="...",
              mimeType="application/vnd.openxmlformats-officedocument.spreadsheetml.sheet",
              pipe_to=async_buffer,
            ))

Thrown error:

Forbidden

Request URL:
https://www.googleapis.com/drive/v3/files/...

Next steps

I suppose it's not expected behaviour. The reason in the error response should (always?) be populated, meaning we can access it e.g. via e.res.reason in case we encounter HttpError e.

I'm happy to contribute with a PR if you feel like this issue should be addressed.

Also, thank you for building and maintaining this library. 🥇

Hi, thank you for taking the time to file the issue in such detail!

I'm happy to contribute with a PR if you feel like this issue should be addressed.

That would be amazing 🙏

Thanks!