Unable to get status from recent version of Velocity server
clrxbl opened this issue · 6 comments
Not sure why but connect.2b2t.org specifically does not seem to respond to mcstatus and I'm not getting blocked by their proxy or TCPShield. It seems to be able to get a response from websites like https://mcsrvstat.us, but fails with mcstatus.
Traceback (most recent call last):
File "/opt/homebrew/lib/python3.11/site-packages/mcstatus/server.py", line 128, in status
return self._retry_status(connection, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/opt/homebrew/lib/python3.11/site-packages/mcstatus/utils.py", line 66, in sync_wrapper
raise last_exc # type: ignore # (This won't actually be unbound)
^^^^^^^^^^^^^^
File "/opt/homebrew/lib/python3.11/site-packages/mcstatus/utils.py", line 62, in sync_wrapper
return func(*args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^
File "/opt/homebrew/lib/python3.11/site-packages/mcstatus/server.py", line 134, in _retry_status
result = pinger.read_status()
^^^^^^^^^^^^^^^^^^^^
File "/opt/homebrew/lib/python3.11/site-packages/mcstatus/pinger.py", line 114, in read_status
response = self.connection.read_buffer()
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/opt/homebrew/lib/python3.11/site-packages/mcstatus/protocol/connection.py", line 313, in read_buffer
length = self.read_varint()
^^^^^^^^^^^^^^^^^^
File "/opt/homebrew/lib/python3.11/site-packages/mcstatus/protocol/connection.py", line 254, in read_varint
part = self.read(1)[0]
^^^^^^^^^^^^
File "/opt/homebrew/lib/python3.11/site-packages/mcstatus/protocol/connection.py", line 539, in read
raise IOError("Server did not respond with any information!")
OSError: Server did not respond with any information!
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "/opt/homebrew/bin/mcstatus", line 8, in <module>
sys.exit(main())
^^^^^^
File "/opt/homebrew/lib/python3.11/site-packages/mcstatus/__main__.py", line 101, in main
args.func(server)
File "/opt/homebrew/lib/python3.11/site-packages/mcstatus/__main__.py", line 15, in status
response = server.status()
^^^^^^^^^^^^^^^
File "/opt/homebrew/lib/python3.11/site-packages/mcstatus/server.py", line 127, in status
with TCPSocketConnection(self.address, self.timeout) as connection:
File "/opt/homebrew/lib/python3.11/site-packages/mcstatus/protocol/connection.py", line 519, in __exit__
self.close()
File "/opt/homebrew/lib/python3.11/site-packages/mcstatus/protocol/connection.py", line 512, in close
self.socket.shutdown(socket.SHUT_RDWR)
OSError: [Errno 57] Socket is not connected
Discussed on Discord, actual traceback is:
Traceback (most recent call last):
File "/app/src/logic/get_status.py", line 54, in get_status
status = await server.async_status(tries=1)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/local/lib/python3.11/site-packages/mcstatus/server.py", line 147, in async_status
return await self._retry_async_status(connection, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/local/lib/python3.11/site-packages/mcstatus/utils.py", line 52, in async_wrapper
raise last_exc # type: ignore # (This won't actually be unbound)
^^^^^^^^^^^^^^
File "/usr/local/lib/python3.11/site-packages/mcstatus/utils.py", line 48, in async_wrapper
return await func(*args, **kwargs) # type: ignore # (We know func is awaitable here)
^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/local/lib/python3.11/site-packages/mcstatus/server.py", line 153, in _retry_async_status
result = await pinger.read_status()
^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/local/lib/python3.11/site-packages/mcstatus/pinger.py", line 107, in read_status
return JavaStatusResponse.build(raw, latency=(received - start) * 1000)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/local/lib/python3.11/site-packages/mcstatus/status_response.py", line 134, in build
motd=Motd.parse(raw["description"], bedrock=False),
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/local/lib/python3.11/site-packages/mcstatus/motd/__init__.py", line 52, in parse
parsed = cls._parse_as_dict(raw, bedrock=bedrock)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/local/lib/python3.11/site-packages/mcstatus/motd/__init__.py", line 137, in _parse_as_dict
parsed_motd.extend(cls._parse_as_dict(element, auto_add=auto_add.copy()))
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/local/lib/python3.11/site-packages/mcstatus/motd/__init__.py", line 113, in _parse_as_dict
if (color := item.get("color")) is not None:
^^^^^^^^
AttributeError: 'str' object has no attribute 'get'
To summarize, we get OSError
because we retry too fast. For the first two attempts, we got an exception on parsing, and on the third retry, the server bans us because of spamming. The same issue with the retry decorator happened already in #511.
We can fix it (retry decorator) by removing it or by retrying only the actual logic (not parsing). To be honest, this kind of logic should be on the user, not on us.
I recall the number of attempts is adjustable. It should be possible to set that to 1 and skip our retry logic.
Yeah, we could just set the retry amount to 1 by default, but I think we should also make sure the retry logic only runs on the actual connection, and not on parsing logic.
However I'm also not opposed to just removing the retry logic entirely.
We can also set the default value to None
(which would mean one) for the deprecation period and raise a warning if the retry
value is not None
. Then, after the deprecation period, remove the retry logic entirely.
I would prefer setting it to a custom private sentinel object, say like this:
from typing import NewType
_SENTINEL_T = NewType("_SENTINEL_T", object)
_SENTINEL = _SENTINEL_T(object())
def some_func(tries: int | _SENTINEL_T = _SENTINEL):
if tries is _SENTINEL:
tries = 1 # or the original 3
else:
deprecation_warn()
...
This way no one will leave a some_func(tries=None)
in their code, as that would cause issues once removed. It's just a small nitpicky edge-case, but I'd rather see this approach, as it would be more reliable. But yeah in general, the principle is the same.
There is still the alternative of just setting the default to 1 and making sure we're retrying only the connection logic, instead of completely dropping retries. But yeah, I'm not sure they're worth keeping around.
I also thought about sentinel, but then I realized that None is just for such cases (as the default value). Anyway, will use sentinel, so it is harder for a user to shoot their own leg.