osuAkatsuki / bancho.py

An osu! server for the generic public, optimized for maintainability in modern python
https://akatsuki.gg
MIT License
213 stars 129 forks source link

fix: discord webhook embed timestamp fails to serialize #555

Closed minisbett closed 7 months ago

minisbett commented 8 months ago

Describe your changes

The timestamp of embeds on a webhook fail to serialize into JSON properly.

Checklist

tsunyoku commented 8 months ago

these are like syntactically identical under the hood so i'd like to understand what the issue is

i'm not convinced there's an issue when this exists:

image

cmyui commented 8 months ago

have a sample error traceback for the issue @minisbett?

cmyui commented 8 months ago

have a sample error traceback for the issue @minisbett?

Ah I found in discord,

bancho_1  | Traceback (most recent call last):
bancho_1  |   File "/usr/local/lib/python3.11/site-packages/tenacity/_asyncio.py", line 50, in __call__
bancho_1  |     result = await fn(*args, **kwargs)
bancho_1  |              ^^^^^^^^^^^^^^^^^^^^^^^^^
bancho_1  |   File "/srv/root/app/discord.py", line 188, in post
bancho_1  |     response = await services.http_client.post(
bancho_1  |                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
bancho_1  |   File "/usr/local/lib/python3.11/site-packages/httpx/_client.py", line 1848, in post
bancho_1  |     return await self.request(
bancho_1  |            ^^^^^^^^^^^^^^^^^^^
bancho_1  |   File "/usr/local/lib/python3.11/site-packages/httpx/_client.py", line 1517, in request
bancho_1  |     request = self.build_request(
bancho_1  |               ^^^^^^^^^^^^^^^^^^^
bancho_1  |   File "/usr/local/lib/python3.11/site-packages/httpx/_client.py", line 358, in build_request
bancho_1  |     return Request(
bancho_1  |            ^^^^^^^^
bancho_1  |   File "/usr/local/lib/python3.11/site-packages/httpx/_models.py", line 339, in __init__
bancho_1  |     headers, stream = encode_request(
bancho_1  |                       ^^^^^^^^^^^^^^^
bancho_1  |   File "/usr/local/lib/python3.11/site-packages/httpx/_content.py", line 214, in encode_request
bancho_1  |     return encode_json(json)
bancho_1  |            ^^^^^^^^^^^^^^^^^
bancho_1  |   File "/usr/local/lib/python3.11/site-packages/httpx/_content.py", line 177, in encode_json
bancho_1  |     body = json_dumps(json).encode("utf-8")
bancho_1  |            ^^^^^^^^^^^^^^^^
bancho_1  |   File "/usr/local/lib/python3.11/json/__init__.py", line 231, in dumps
bancho_1  |     return _default_encoder.encode(obj)
bancho_1  |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
bancho_1  |   File "/usr/local/lib/python3.11/json/encoder.py", line 200, in encode
bancho_1  |     chunks = self.iterencode(o, _one_shot=True)
bancho_1  |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
bancho_1  |   File "/usr/local/lib/python3.11/json/encoder.py", line 258, in iterencode
bancho_1  |     return _iterencode(o, 0)
bancho_1  |            ^^^^^^^^^^^^^^^^^
bancho_1  |   File "/usr/local/lib/python3.11/json/encoder.py", line 180, in default
bancho_1  |     raise TypeError(f'Object of type {o.__class__.__name__} '
bancho_1  | TypeError: Object of type datetime is not JSON serializable
bancho_1  | 
bancho_1  | The above exception was the direct cause of the following exception:
bancho_1  | 
bancho_1  | Traceback (most recent call last):
bancho_1  |   File "/srv/root/app/commands.py", line 2499, in process_commands
bancho_1  |     res = await cmd.callback(
bancho_1  |           ^^^^^^^^^^^^^^^^^^^
bancho_1  |   File "/srv/root/app/commands.py", line 691, in _map
bancho_1  |     await webhook.post()
bancho_1  |   File "/usr/local/lib/python3.11/site-packages/tenacity/_asyncio.py", line 88, in async_wrapped
bancho_1  |     return await fn(*args, **kwargs)
bancho_1  |            ^^^^^^^^^^^^^^^^^^^^^^^^^
bancho_1  |   File "/usr/local/lib/python3.11/site-packages/tenacity/_asyncio.py", line 47, in __call__
bancho_1  |     do = self.iter(retry_state=retry_state)
bancho_1  |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
bancho_1  |   File "/usr/local/lib/python3.11/site-packages/tenacity/__init__.py", line 326, in iter
bancho_1  |     raise retry_exc from fut.exception()
bancho_1  | tenacity.RetryError: RetryError[<Future at 0x7f1ba1c8c4d0 state=finished raised TypeError>]
cmyui commented 8 months ago

Did you migrate your bancho.py to use httpx? Looks like master is using aiohttp and the exception mentions httpx.

cmyui commented 8 months ago

Ah nvm I understand - @tsunyoku I think you linked non-master code

tsunyoku commented 8 months ago

ok can we fix by changing httpx's serialization then?

minisbett commented 8 months ago

as this issue may exist in several other code spots

yup, getting that error elsewhere as well.

cmyui commented 8 months ago

I found this comment/discussion while browsing the issues on the httpx repo:

I think this comment is true - it should be possible to subclass httpx.AsyncClient to customize the serialization

cmyui commented 7 months ago

Hm, in the current httpx lib there isn't really a nice way to do this since request_class and response_class aren't configurable.

We can subclass AsyncClient to be able to hook BaseClient.build_request to use a custom Request class, but it involves quite a bit of copying from httpx.

Alternatively we could hook httpx._content.encode_json, but it's not ideal -- clearly not a public api and so it may change in a non-breaking release.

Might actually be best overall to make a PR to httpx to add support for request_class and response_class, then as tomchristie proposes, we can do something like:

class APIClient(httpx.Client):
    request_class = APIRequest
    response_class = APIResponse

class APIRequest(httpx.Request):
    def __init__(self, *args, **kwargs):
        if 'json' in kwargs:
            content = orjson.dumps(kwargs.pop('json'))
            headers = kwargs.get('headers', {})
            headers['Content-Length'] = len(content)
            kwargs['content'] = content
            kwargs['headers'] = headers
        return super().__init__(*args, **kwargs)

class APIResponse(httpx.Response):
    def json(self):
        return orjson.loads(self.content)
cmyui commented 7 months ago

I'll turn this into an issue as I think implementation is a bit pre-mature - will tag everyone there for further discussion