MarkGodwin / tplink-omada-api

MIT License
12 stars 9 forks source link

Werid issue with clients endpoint #32

Closed danielangeloni closed 6 months ago

danielangeloni commented 8 months ago

Hey there

I came across a werid bug trying to get the connected clients.

This is the code i am using

import asyncio

from tplink_omada_client.omadaclient import OmadaClient

async def hello():
    async with OmadaClient(url=URL, username=USERNAME, password=PASSWORD) as client:
        sites = await client.get_sites()
        site = next((s for s in sites if s.name == SITE), None)

        site_client = await client.get_site_client(site)

        connected_clients = site_client.get_connected_clients()

        async for client in connected_clients:
            print(client.name)

asyncio.run(hello())

And this is the traceback

Traceback (most recent call last):
  File "/Users/daniel/tplink-omada-api/run.py", line 42, in <module>
    asyncio.run(hello())
  File "/opt/homebrew/Cellar/python@3.11/3.11.6_1/Frameworks/Python.framework/Versions/3.11/lib/python3.11/asyncio/runners.py", line 190, in run
    return runner.run(main)
           ^^^^^^^^^^^^^^^^
  File "/opt/homebrew/Cellar/python@3.11/3.11.6_1/Frameworks/Python.framework/Versions/3.11/lib/python3.11/asyncio/runners.py", line 118, in run
    return self._loop.run_until_complete(task)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/homebrew/Cellar/python@3.11/3.11.6_1/Frameworks/Python.framework/Versions/3.11/lib/python3.11/asyncio/base_events.py", line 653, in run_until_complete
    return future.result()
           ^^^^^^^^^^^^^^^
  File "/Users/daniel/tplink-omada-api/run.py", line 38, in hello
    async for client in connected_clients:
  File "/Users/daniel/tplink-omada-api/venv/lib/python3.11/site-packages/tplink_omada_client/omadasiteclient.py", line 125, in get_connected_clients
    async for client in self._api.iterate_pages(self._api.format_url("clients", self._site_id)):
  File "/Users/daniel/tplink-omada-api/venv/lib/python3.11/site-packages/tplink_omada_client/omadaapiconnection.py", line 158, in iterate_pages
    response = await self.request("get", url, request_params)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/daniel/tplink-omada-api/venv/lib/python3.11/site-packages/tplink_omada_client/omadaapiconnection.py", line 176, in request
    return await self._do_request(method, url, params=params, payload=payload)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/daniel/tplink-omada-api/venv/lib/python3.11/site-packages/tplink_omada_client/omadaapiconnection.py", line 215, in _do_request
    self._check_application_errors(content)
  File "/Users/daniel/tplink-omada-api/venv/lib/python3.11/site-packages/tplink_omada_client/omadaapiconnection.py", line 238, in _check_application_errors
    raise RequestFailed(response["errorCode"], response["msg"])
tplink_omada_client.exceptions.RequestFailed: Omada controller responded 'General error.' (-1)

I used Developer tools in Firefox and saw the web requests to get clients was /clients?currentPage=1&currentPageSize=10&filters.active=true&_=1700282586424

The only difference I could see between this and firefox the filters.active and _ params. I replayed the request using curl and tested removing the params and when I removed filters.active I got the same error of

{"errorCode":-1,"msg":"General error."}%                                                                                                                                         

I found a solution (although not very clean), adding this in _do_request function before it does the request in OmadaApiConnection class seems to fix it

if "clients" in url:
    params["filters.active"] = "true"
MarkGodwin commented 8 months ago

Very strange. Does the filtering actually filter anything out?I wonder if this could be an issue with paging the result set size. Do you have a large number of clients on your network?

danielangeloni commented 8 months ago

Does the filtering actually filter anything out?

It doesn't seem so, nothing changes (except for the uptime and lastSeen of the returned hosts) when running these two /clients?currentPage=1&currentPageSize=10&filters.active=true /clients?currentPage=1&currentPageSize=10&filters.active=false

but yeah running without &filters seems to break it. I even tried testing with jibberish, &filters.sdfnsdhjfbsandjifs=asndkjasdkjn runs without errors which is even weirder as &filter.sdfnsdhjfbsandjifs=asndkjasdkjn breaks as does &filtersnsdjif.sdfnsdhjfbsandjifs=asndkjasdkjn. It seems it is just looking for "filters" but not actually doing anything?

I wonder if this could be an issue with paging the result set size. Do you have a large number of clients on your network?

Not really, around 35 on average

Also I am running version 5.12.7 in Docker using https://hub.docker.com/r/mbentley/omada-controller/

MarkGodwin commented 6 months ago

Sorry for the wait. I'm now seeing this same problem locally, so I should be able to formulate a fix.

MarkGodwin commented 6 months ago

Fixed in release 1.3.7