lucasheld / uptime-kuma-api

A Python wrapper for the Uptime Kuma Socket.IO API
https://uptime-kuma-api.readthedocs.io
MIT License
272 stars 21 forks source link

:bug: (api.py): Raise correct error when timeout #45

Open Zerka30 opened 1 year ago

Zerka30 commented 1 year ago

Related to : https://github.com/lucasheld/uptime-kuma-api/issues/44

I'll let you try it for yourself, and let me know if this function has any dependencies elsewhere.

lucasheld commented 1 year ago

I think that is not quite correct. The Uptime Kuma errors are in r.get("msg"). Now they are no longer intercepted, because an Expection is not thrown in this case.

I would do it like this:

def _call(self, event, data=None) -> Any:
    try:
        r = self.sio.call(event, data, timeout=self.timeout)
    except socketio.exceptions.TimeoutError:
        raise Timeout(f"Timed out while waiting for event {event}")
    if isinstance(r, dict) and "ok" in r:
        if not r["ok"]:
            raise UptimeKumaException(r.get("msg"))
        r.pop("ok")
    return r
Zerka30 commented 1 year ago

Hum :thinking: , wdym exactly?

Because the way I've done it, it will return the correct error if socket.io returns a TimeoutError, and for any other type of error it will return an UptimeKumaException with the error message contained in r.get("msg")

Zerka30 commented 1 year ago

@lucasheld , I ping you, to correct if necessary

lucasheld commented 1 year ago

I think that is not quite correct. The Uptime Kuma errors are in r.get("msg"). Now they are no longer intercepted, because an Expection is not thrown in this case.

I would do it like this:

def _call(self, event, data=None) -> Any:
    try:
        r = self.sio.call(event, data, timeout=self.timeout)
    except socketio.exceptions.TimeoutError:
        raise Timeout(f"Timed out while waiting for event {event}")
    if isinstance(r, dict) and "ok" in r:
        if not r["ok"]:
            raise UptimeKumaException(r.get("msg"))
        r.pop("ok")
    return r

I have tested this code. It works as expected.

The only problem I noticed is that there are still exceptions directly from socketio (for example socketio.exceptions.ConnectionError). Then these Excepetions should either be caught too or we have to use separate timeout exceptions, one for the logic within uptime-kuma-api and one for socketio timeouts. Currently, I prefer the latter.

Zerka30 commented 1 year ago

Alright, I understand and now its fix ! :D