Handling: Socket warnings/errors

[MinusGix]

I believe it's a goal to eventually give each error/warning a specific identifier so that they can be more easily detected properly by clients. (I'll be calling these 'errors', but I refer to warnings/errors)
The first solution that popped into my head is to have a counter that each module then increments, and uses that value as an id for one of their warnings. This, would not work as it makes error numbers dependent on when they're added (so different module loading order = different values), and if a new error is added to a module then it will shift every module error after it.

The second idea I had was to supply some simple function that takes in an identifier (short?), and you can ask it for the next identifier. Such as:

let error = {};
export function init (core) {
    let idc = createIDCounter('join');
    error.invalid_nickname = idc.next();
    error.not_admin = idc.next();
    error.nickname_taken = idc.next();
}

Which would then be used like:

server.reply({
    cmd: 'warn',
    error: error.nickname_taken,
}, socket)};

And, nickname_taken === ['join', 2].
So whenever you add a new error, you have to (well we can't enforce it, but it's a 'should only') add it to the end, and it's per module. Though, it has the downside that it has to pass a small string along.
You could probably automatically assign Error ids based on filename (so some number generated from it?), but the string is probably easier and more clear.
Thoughts?

[marzavec]

it's a goal to eventually give each error/warning a specific identifier so that they can be more easily detected properly by clients.

Yep! But not only for ease of handling/detecting within the client engine; this change touches heavily on the internationalization feature of the new client. Currently this client uses react-intl ( docs ) to automagically handle the translations. Each language is stored within it's own json file (en.json, de.json, jp.json, etc). The layout of the json is pretty simple, each separate language has common identifiers which are linked to the text in the corresponding language, example:

// en.json
{
  "JoinModal.title": "Join Channel"
}

// de.json
{
  "JoinModal.title": "Channel beitreten"
}

This is a brief example of the dialogue that happens when the client prompts the user to join a new channel. Essentially what happens is that the JoinModal.title attribute is referenced from the currently selected json which is passed as the value prop into a FormattedMessage component. I felt the client-side error translations would look something like this:

// de.json
{
  "Errors.join.InvalidNick": "Oh mein Gott, was zum Teufel versuchst du überhaupt?"
}

I wanted to explain all of this because it does effect the route that we choose (that and I enjoy throwing walls o' text at people ofc :D ). To boil it down, the question is "how should we generate error IDs". Your first solution is one I would shy away from personally- for exactly the reasons you stated, client-side we would be forced to update every label of every error of every translation (nothanks.jpg).

Your second solution is good, but may need some tweaking in order to maintain the flexibility. I like the idea of using a namespace then a number, the label in the json would then be like Errors.join.2, the client handling the extra steps to compile the label. This has the benefit of keep the bandwidth low/speedy and ease development. There are two or three downsides to this approach though.

The first deals with debugging when using the client engine alone- I'm not sure if I mentioned this before, but there is/will be a separate client engine module that the front-end includes, this engine handles all the websocket/protocol details which is then fed into a saga consumer by the client. Translations are not included in the engine, which means someone building a bot using the official engine will have to deal with debugging "mysterious" code numbers.

The second is really not a big deal, but I wonder if this naming scheme will bother people who are adding/updating translations?

The last touches on the flexibility aspect. Let's say a dev uses hack.chat on their website and adds a custom module that may need to output an error. The official path would be for them to use their own namespace + numeric combo then update their client from source- adding the error text + translations, recompiling and uploading. Maybe I'm just lazy, but to me that seems like a lot of work just to display an error which they likely don't care about translating. So it would be ideal if we could open an alternate pathway for them to easily display an error, which may mean still accepting some kind of error text sent from the server. This could be incorporated into the existing error handling system or as a completely separate event?

Tbh, I don't want to fall into the @microsoft meme of spooky error codes (wtf is a 0x0000004A IRQL_GT_ZERO_AT_SYSTEM_SERVICE anyway? =P ), but would like to shoot for a good middle ground between your slim & dynamic second solution and being overly descriptive / bloated. . .

How do you feel about this solution; updating server/src/utility/Constants.js or adding a new server/src/utility/ErrorCodes.js- adding the built-in error codes like:

// ErrorCodes.js
exports.RateLimError = {
  RATE_LIMITED: 0,
  BANNED: 1,
};
exports.JoinError = {
  NICKNAME_TAKEN: 2,
  INVALID_NICKNAME: 3,
};

// join.js
import { JoinError } from '../utility/ErrorCodes';
// ( . . . )
server.reply({
    cmd: 'warn',
    error: JoinError.NICKNAME_TAKEN,
}, socket)};

A mirror of the ErrorCodes.js constants can be kept in the engine, which would allow mapping of the error label, IE the engine could then say Error: 2 NICKNAME_TAKEN. We can likely do the same reversing process on the client front end for the translation files, so translators can work with the pretty label (as "Errors.INVALID_NICKNAME": "Oh mein Gott, was zum Teufel versuchst du überhaupt?"). This would keep the protocol slim, as it would send a single numeric id, but would make the code a bit more verbose and static. The third issue could be overcome by checking if the error code exists, if not then it's expected that the structure will be something like:

{
  cmd: 'warn',
  error: '',
  text: 'This is a custom error message',
}

I think the only thing we are losing here is your dynamic number generation and the namespaces (each error would have a unique numeric id). But what do you think? Maybe there is a better solution that I'm not seeing?

Sorry about the wall of text. Also sorry about creating needless issues by not releasing the v2 client yet- but it's an incomplete mess at the moment and I don't want the mess to confuse or seem more intimidating than the code already is.