SpigotMC / BungeeCord

BungeeCord, the 6th in a generation of server portal suites. Efficiently proxies and maintains connections and transport between multiple Minecraft servers.
https://www.spigotmc.org/go/bungeecord
Other
1.58k stars 1.11k forks source link

ServerKickedEvent doesn't fire for fallback kicks #3183

Open almic opened 3 years ago

almic commented 3 years ago

Pretty simple, no kick events are called when players are disconnected because fallback connections are exhausted.

To reproduce:

  1. Configure bungeecord to send to some server
  2. Take the server offline, just having the bungee proxy running
  3. Have listener for ServerKickEvent that just prints the event to console
  4. Connect to proxy, get kicked with message Could not connect to a default or fallback server, please try again later:
  5. No event fired

I think this should be implemented into BungeeCord as it's annoying that I can't change the kick message to something more tasteful with events.

Janmm14 commented 3 years ago

It is no ServerKick, because the player was never on a server ;)

almic commented 3 years ago

Good point, but I still want my events :(

MrIvanPlays commented 2 years ago

maybe a new event, KickedFromProxyEvent? either way I don't really understand why such a thing is needed. What's the usecase?

almic commented 2 years ago

Sorry for the long explanation, but as this issue seems to not be convincing enough, I felt like I had to provide a strong case.

The use of this would be mainly for proper tracking of the progress through connection attempts, and for altering the message or redirecting a player prior to the final disconnection once all options are exhausted. It would just be nice to know when a kick is going to automatically attempt a redirect or if it's the final try and proxy will fully disconnect them afterwards. I'll provide an example using three servers, the simplest set up:

Steps

  1. A client running 1.16 connects to the proxy, their last session was on server B, they are first directed there.
  2. The connection fails as expected, a kick event is fired with the message "Server is on version 1.17!"
  3. Proxy then tries to connect them to server A. The server is offline, however. Kick event is fired with no message.
  4. Proxy is out of options and disconnects them with "Could not connect to a default or fallback server, please try again later: [java error]"

The idea behind this set up is that you want a player to be connected to one of either A or B, and failing that, disconnected by informing them the servers were offline or bad versions. This is the worst when a server disconnects because the client version is unsupported. On the final attempt, the proxy will override that reason and replace it with the very unhelpful java error message.

This problem cannot be fixed by adding a redirection server to the kick events, because that obviously will result in infinite loops. It also seems like it should be an unnecessary solution to "recreate the wheel" and store the attempts being made for each player, because the proxy could just give you that information itself. A developer having to track what is already being tracked by the proxy can be avoided by firing one final kick event letting the developer know all options were exhausted.

I have overcome the problem by recreating what the proxy does, and providing the most useful error message to the player. I've noticed many people connecting with the wrong versions and not being given that information because a different server was offline, and then never trying to reconnect. My proxy regularly tests server connections and knows which ones are offline. So when the final attempt is made, the most helpful message is provided. My specific problem is gone now, because they are properly told they can reconnect on a different version, and they do. However, it required an unneeded amount of work that could all be avoided by simply firing the dang kick event.

MrIvanPlays commented 2 years ago

Why don't you PR what you need as a new event because I can't really understand what you want us to do. You want an event which is fired when the proxy reaches state that it will kick the player with "Could not connect to a default or fallback server" or?

almic commented 2 years ago

The reason I opened an issue was to get some opinions on this and see if I was missing something. I have a problem but I don't know what the ideal solution would be. The ones I can think of would sprawl across the code here and touch many files.

I don't really want to make a PR because as far as I know, this problem is pretty unique to myself. It doesn't seem anyone else who has this problem is coming here for a work around. Anything I found were just work-arounds on the spigot forums, mostly just detecting the netty exception string and changing it, and seemed naive to me. If I had to make this a PR, this would be my solution:

Everything begins around here: https://github.com/SpigotMC/BungeeCord/blob/c7b0c3cd48c9929c6ba41ff333727adba89b4e07/proxy/src/main/java/net/md_5/bungee/UserConnection.java#L348-L359

Instead of having an outright disconnection on line 355, just call "connect" with a dummy "none" target ServerInfo. This would allow a developer to catch it here and set a target themselves. Perhaps pre-cancelled as well, with JOIN_PROXY as the reason. If the event still comes back cancelled, then disconnect with the fallback kick reason. I think this would be the right way to do it. If a ServerInfo is made to represent an empty target and subsequent disconnection, it would be backwards compatible and curb the original problem I have, and offer developers a nice way to give their own disconnection reasons or redirect to a hidden server for special players.