[libvncclient] Please consider adding a lastError field to rfbClient

[willbprog127]

While libvncclient does output errors (such as failed authentication) when used with logging facility, it is almost impossible to identify the precise rfbClient logging the error. In my app, there can be dozens of rfbClient instances at once so the general logging facility that libvncclient provides is inadequate. While most errors mention the IP address, this is not enough to identify an exact rfbClient because some of them have the same IP address but different ports.

I would like to see the addition of a lastError char * field added to the rfbClient structure that is set any time there is an error with that rfbClient.

Maybe the rfbClientLog facility could be modified to set the lastError of the rfbClient, cutting down on duplicate strings, etc, with an example signature of rfbDefaultClientLog(rfbClient* client, const char* format, ...). Consider the following (very oversimplified) example:

rfbBool
rfbHandleAuthResult(rfbClient* client)
{
...
    switch (authResult) {
...
    case rfbVncAuthFailed:
...
      rfbClientLog(client, "VNC authentication failed\n");    // <---### CHANGE ###
      return FALSE;

and

static void
rfbDefaultClientLog(rfbClient* client, const char* format, ...)     // <---### CHANGE ###
{
    va_list args;
    char buf[256];
    time_t log_clock;
    char msgBuf[1024] = {0};    // <---### CHANGE ###

    if(!rfbEnableClientLogging)
      return;

    va_start(args, format);

    time(&log_clock);
    strftime(buf, 255, "%d/%m/%Y %X ", localtime(&log_clock));
    fprintf(stderr, "%s", buf);

    vfprintf(stderr, format, args);
    vsnprintf(msgBuf, 1023, format, args);    // <---### CHANGE ###
    fflush(stderr);

    if (client != NULL) {    // <---### CHANGE ###
      // ### set lastError here ###
    }

    va_end(args);
}

Again, I have thought about using the IP address given in the logged error messages to identify, but this is not enough information when I have some remote facilities that have the same IP address but different ports.

Thanks! :+1:

[nicmorais]

This is a cool feature, I'm working on it. But some details need to be explained, to avoid breaking changes. So, the server must only send the error code to the client if it supports such feature. This means we must check the version of the client to define if we will send the error code or not. And how do we check this? By "rfb protocol version"? Or other means? What error codes will be listed? Should we list error codes or status codes? E.g.

enum rfbClientErr {
    EINVALIDPASSWD,
    EINVALIDCLIENT,
    ...
}

[willbprog127]

@nicmorais Thanks so much for looking into this. It has been a while since I submitted this, so I will try to explain better...

In my app, SpiritVNC-FLTK, I can have multiple connections running at the same time. Right now, libvncclient only logs errors to the general logging facility, instead of setting a per-connection error code or string. This makes it difficult to let the user know what specific error happened per-connection / per-client because libvncclient's logging facility doesn't always specify things like IP address, etc.

Basically I would like to see something similar to gtk-vnc -- you can connect gtk-vnc's error signal to a callback and handle the error as you wish. I am not looking for any additional errors to be added, I just want the ability to handle the error text libvncclient already has for each client's error separately. For example, gtk-vnc's error handler signature is like this:

static void vnc_error(GtkWidget *vncdisplay, const gchar *message)
{
  fprintf(stderr, "Error: %s\n", message);
}

It would be great if libvncclient provided:

• A way to connect a per-client callback (like how logging, authentication, etc is currently handled) to handle any errors that occur
• The callback signature would have, at least, client * and char * to provide the error string that libvncclient is already sending to the logging

Hopefully this makes sense. If not, please ask me for specific details and I will try my best :+1:

Thanks! :smile:

[nicmorais]

Hello @willbprog127 Right. My question is about what error codes should we put in an enum, because this would be a better way for handling them. Since libvncserver supports HTTP, OpenSSL, and other stuff, I wanted to make a long list of possible statuses/errors, just because I did not want an error handling system that did not cover most error cases... But, as you said, a callback that had (rfbClient cl, char error) is enough for now. This library logs almost everything on console, so it should be easier to get errors this way.

Thanks!

[willbprog127]

I will just give a few error codes because I have a tiny brain and not an rfb master:

• Invalid address
• Failed authentication
• Server closed connection
• Connection timeout
• Connection refused
• Unsupported rfb version
• Invalid server message
• Cannot allocate memory (for rfbClient, etc)

Sorry, cannot think of others right now. :man_shrugging: :+1: