FAForever / client

FAF Python Client
GNU General Public License v3.0
74 stars 88 forks source link

exception in clientwindow.py, client stops working #490

Closed CookieNoob closed 6 years ago

CookieNoob commented 7 years ago

client stopped working after playing a game. The "FAF" logo turned red and the games tab was empty, chat was not refreshed anymore.

relevant part of the forever.log: Traceback (most recent call last): File "src\client_clientwindow.py", line 988, in readFromServer File "src\client_clientwindow.py", line 1118, in dispatch File "src\client_clientwindow.py", line 1144, in handle_invalid Exception: {u'command': u'invalid'}

forever.txt game.txt

duk3luk3 commented 7 years ago

which version of client?

CookieNoob commented 7 years ago

0.11.60+577 so the latest version I guess?

muellni commented 7 years ago

It turns red when opening the modvault in current develop:

2016-11-16 12:30:08,542 INFO     client._clientwindow           Outgoing JSON Message: {"type": "start", "command": "modvault"}
2016-11-16 12:30:08,641 ERROR    client._clientwindow           Error dispatching JSON: {"command": "invalid"}
Traceback (most recent call last):
  File "src\client\_clientwindow.py", line 993, in readFromServer
  File "src\client\_clientwindow.py", line 1123, in dispatch
  File "src\client\_clientwindow.py", line 1152, in handle_invalid
Exception: {u'command': u'invalid'}
muellni commented 7 years ago

Same behavior with the last release: Opening the mod vault tab turns the connection icon red.

muellni commented 7 years ago

Server log from downlord:

 >>: {'likes': 299.0, 'version': 37, 'bugreports': [], 'ui': 0, 'date': 1475174335, 'command': 'modvault_info', 'thumbnail': 'http://content.faforever.com/faf/vault/mods_thumbs/Equilibrium_Balance_Mod.png', 'downloads': 4065, 'description': 'This is the develop version of equilibrium. Use at your own risk! We released this as-is because there was an opportunity to do so.', 'name': 'Equilibrium_Balance_Mod', 'played': 7650, 'author': 'Ithilis', 'comments': [], 'uid': 'DEVELOP2-V37D-5264-1254-EQBALANCEMOD', 'link': 'http://content.faforever.com/faf/vault/mods/Equilibrium_Balance_Mod.v0037.zip'}
 quote_from_bytes() expected bytes
 Traceback (most recent call last):
   File "/code/server/lobbyconnection.py", line 143, in on_message_received
     await handler(message)
   File "/code/server/lobbyconnection.py", line 1008, in command_modvault
     thumbstr = urllib.parse.urljoin(config.CONTENT_URL, "faf/vault/mods_thumbs/" + urllib.parse.quote(icon))
   File "/usr/local/lib/python3.5/urllib/parse.py", line 712, in quote
     return quote_from_bytes(string, safe)
   File "/usr/local/lib/python3.5/urllib/parse.py", line 737, in quote_from_bytes
     raise TypeError("quote_from_bytes() expected bytes")
 TypeError: quote_from_bytes() expected bytes
 Client Grothe2 dropped. Error processing command
duk3luk3 commented 7 years ago

This is clearly an error on the server. Probably an encoding / conversion problem with the icon field.

Paging @micheljung...

micheljung commented 7 years ago

icon is null, but the server checks for empty string: https://github.com/FAForever/server/blob/663a6c163163c29ca8c93f17193ad607bb6de690/server/lobbyconnection.py#L1009

duk3luk3 commented 7 years ago

Definitely a server bug, but should probably be handled more gracefully by the client, so I'm leaving this bug open.

GrotheFAF commented 7 years ago

Question is in what cases the server sends {u'command': u'invalid'}, cause this leads directly to: def handle_invalid(self, message): self.state = ClientState.DISCONNECTED raise Exception(message) So to put some 'grace' into it, the cases - beside the one at hand - where this is send by the server, should be known. But one could argue: 'don't be a pussy client, don't disconnect, let's see what happens', and in this case, the client loses irc/chat, with output: QAbstractSocket::connectToHost() called when already looking up or connecting/connected to "lobby.faforever.com" each time entering the modvault tab. (and not during search or any other action there) (+ one after some time: QSocketNotifier: Multiple socket notifiers for same socket 2216 and type Read ) ( or same with other sockets 24224, 24584)

replacing: def tabOpened(self): client.instance.send(dict(command="modvault",type="start")) <- goes wrong with that with: def tabOpened(self): if not self.mods: ____self.search() -> client.instance.statsServer.send(dict(command="modvault_search", typemod=typemod, search=self.searchString)) avoids the problem.

It seems that the other .send(command="modvault", ...) in modwidget.py have simular issues.

duk3luk3 commented 7 years ago

The client can't simply ignore the server when it says "you did something very bad". It means the server and client are probably out of synch and continuing can lead to who-knows-what happening.

and in this case, the client loses irc/chat, with output: QAbstractSocket::connectToHost() called when already looking up or connecting/connected to "lobby.faforever.com" each time entering the modvault tab. (and not during search or any other action there) (+ one after some time: QSocketNotifier: Multiple socket notifiers for same socket 2216 and type Read ) ( or same with other sockets 24224, 24584)

That sounds like a bug, connections should never be left in a state that leads to problems like that.

def tabOpened(self) ....

You can use to make code blocks. You can even get them highlighted by writing "python".

Could you please link the code you're talking about?

GrotheFAF commented 7 years ago

https://github.com/FAForever/client/blob/develop/src/modvault/__init__.py#L205 As downlord cleaned the mods, and it's a server problem, this was only meant as a temp fix and is obsolet now.

duk3luk3 commented 7 years ago

What is the statsServer?

I can't find anything that defines the modvault_search command.

GrotheFAF commented 7 years ago

https://github.com/FAForever/legacy-secondaryServer/blob/master/replays/replayServerThread.py#L89

duk3luk3 commented 7 years ago

hmm. changing a method that currently uses the new server to use the old server sounds backwards :)

Changing the new server to support searching mods should be really easy.

GrotheFAF commented 7 years ago

I took it from here: https://github.com/FAForever/client/blob/develop/src/modvault/__init__.py#L156 well go for it, then. I don't do server.

duk3luk3 commented 7 years ago

Is anyone still having the original issue that modvault is completely nonfunctional?

duk3luk3 commented 7 years ago

This seems to be fixed, I'm moving it to the 'Next' milestone in case someone wants to do a thorough investigation of which commands the client still issues that aren't implemented in the server (and therefore cause an "invalid" reply from the server and for the client to be kicked).

Wesmania commented 6 years ago

I'm considering this fixed.