Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Unable to get status from recent version of Velocity server #711

Closed
clrxbl opened this issue Dec 21, 2023 · 6 comments · Fixed by #715 · May be fixed by #716
Closed

Unable to get status from recent version of Velocity server #711

clrxbl opened this issue Dec 21, 2023 · 6 comments · Fixed by #715 · May be fixed by #716
Labels
area: API Related to core API of the project type: bug Something isn't working

Comments

@clrxbl
Copy link

clrxbl commented Dec 21, 2023

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
@ItsDrike ItsDrike added type: bug Something isn't working area: API Related to core API of the project labels Dec 21, 2023
@PerchunPak
Copy link
Member

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.

@kevinkjt2000
Copy link
Contributor

I recall the number of attempts is adjustable. It should be possible to set that to 1 and skip our retry logic.

@ItsDrike
Copy link
Member

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.

@PerchunPak
Copy link
Member

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.

@ItsDrike
Copy link
Member

ItsDrike commented Dec 24, 2023 •
edited
Loading

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.

@PerchunPak PerchunPak mentioned this issue Dec 26, 2023
Merged
@PerchunPak
Copy link
Member

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.

@PerchunPak PerchunPak mentioned this issue Dec 26, 2023
Open
@PerchunPak PerchunPak linked a pull request Dec 26, 2023 that will close this issue
Deprecate retries feature #716
Open
@PerchunPak PerchunPak changed the title Unable to ping connect.2b2t.org Unable to get status from recent Velocity versions Jan 2, 2024
@PerchunPak PerchunPak changed the title Unable to get status from recent Velocity versions Unable to get status from recent version of Velocity server Jan 2, 2024
@elmurato elmurato mentioned this issue Jan 8, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area: API Related to core API of the project type: bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Allow strings in extra Motd py-mine/mcstatus
Deprecate retries feature py-mine/mcstatus
4 participants
@kevinkjt2000 @ItsDrike @clrxbl @PerchunPak