Codify server responses (errors!)

[gajop]

Currently the server sends a lot of responses which are just natural sentences.
Most of these happen due to errors, and none of them are defined.
Examples:
"Already logged in."
"User is already ignored."
and even stuff like
"The HASH command is no longer supported."

Some of these are actual user errors, but some are lobby client errors. In any case it's hard to handle and properly present them to the user as they're never defined! Also, I'm not sure if there's any reference as to what command (not just type, but also id) caused it. Lastly, these messages cannot be easily displayed in other languages.

(it's doable, but really ugly)

Code: Select all

if msg == 'The HASH command is no longer supported.' then return i18n("hash-unsupported") end

And if some uberserver dev decides to change the full stop to an exclamation mark you will have to change your code.
This is not ideal.

So I propose the following: use return codes instead.
These could be just simple integers, and we could have a list of meanings of each integer, or they could be variable-like strings, e.g. "too-many-attempts". In either case, they should all be defined by the protocol.
Also, all errors should also contain the id of the command if it was passed.
It's possible to send commands like this to uberserver:
#1 IGNORE xyz
To which a response could be
#1 ERROR 3 -> in this case, code 3 could represent 'No such user' for the IGNORE command.

(Maybe we should have a single ERROR command, or multiple $COMMAND$ERROR commands, not yet decided).

[abma]

oops, i silently introduced sth. like this:

https://github.com/spring/uberserver/bl ... l.py#L1163

but yeah, your proposal sounds much better, it would only add some integer value for the error code. the message imo should be kept for lobbies which can't/don't want to use the error codes.

maybe this format?

Code: Select all

ERROR code=1 cmd=LOGIN msg=invalid password

and yes, i think such a generic error cmd would be very useful, as a lot of commands could be removed from lobby protocol.

[gajop]

Actually, the FAILED command seems OK as it is.
What do you think about using it like this? (remove integer error and use a human readable variable error)

Code: Select all

FAILED cmd=LOGIN msg=invalid_password

Note that all client commands should have an id, which would make it easy to identify which command caused the error. Example:

Code: Select all

#5 LOGIN userName password cpu localIP
#5 FAILED cmd=LOGIN msg=invalid_password

The question is really how much is the message intended for humans really needed?

[abma]

The question is really how much is the message intended for humans really needed?

most error messages are meant for lobby client devs: i.e. its bad when client sends a command and the response is only "ERROR". the more verbose it is the easier usally it is for the client or maybe enduser to fix the issue. also i'm not sure if in all cases a static error message string can be used, imo in some cases parameters are needed as well, i.e. username/channelname/ ...

also i would slightly modifiy the syntax:

FAILED cmd=LOGIN code=invalid_password msg=Invalid password.

msg is the "human readable message" and code is the "static" error code which will be always the same.

[gajop]

There are 3 different scenarios I want to cover with this issue:
1. Lobby client software errors. This includes both syntactical things like malformed commands, wrong parameter count or type, as well as semantical things, like trying to join a non-existant battle, ignore oneself and similar. All these errors are the case of badly written software, and they should definitely have a human readable message, while the code is not really needed (but there's no harm to have it, helps debugging and removes the need of null checks). The client software shouldn't be displaying these errors, but instead be fixed.
2. Lobby client user errors. This covers login/battle join/channel join attempts with wrong passwords, trying to register a username that already exists, and so on. Note that this doesn't cover trying to join a passworded battle without specifying a password (this is a lobby client software error as it should never allow the user to specify an empty password when joining a passworded battle). Anyhow, these errors should be displayed and they need to be localized.
3. Non-error codes. Certain commands could be codified to allow battle software handling on both ends. Example of commands that could allow codes (as an optional or mandatory replacement/addition, depending on the command): SERVERMSG, SERVERMSGBOX, BROADCAST, EXIT, IGNORE, KICKFROMBATTLE, LEAVE/LEFT.

For 1. and 2. I'd even use different commands (main difference being the name): EXCEPTION/ERROR and FAIL maybe? 3. should just modify the existing commands with the addition of msg/code (one or both, on a per-case basis).

also i'm not sure if in all cases a static error message string can be used, imo in some cases parameters are needed as well, i.e. username/channelname/ ...

Error code should be static, msg is dynamic. An addition would be introducing error params (param1, param2... paramN, or a simple params array if using JSON) that can be used to generate local error messages (see how a count impacts i18n: https://github.com/gajop/i18n#pluralization). Each param meaning should be defined according to the error code (although in the case of json a simple key-value dict could contain the param name/value and make things easier).

FAILED cmd=LOGIN code=invalid_password msg=Invalid password.

msg is the "human readable message" and code is the "static" error code which will be always the same.

Agreed.