WebSocket::send() - Skip sending and report success if we are over the limit of maxBackpressure

[ronen37]

In WebSocket::send(), if we are over the limit of maxBackpressure, it skips sending and reports success:

/* Send or buffer a WebSocket frame, compressed or not. Returns false on increased user space backpressure. */
bool send(std::string_view message, uWS::OpCode opCode = uWS::OpCode::BINARY, bool compress = false) {
    WebSocketContextData<SSL> *webSocketContextData = (WebSocketContextData<SSL> *) us_socket_context_ext(SSL,
        (us_socket_context_t *) us_socket_context(SSL, (us_socket_t *) this)
    );

    /* Skip sending and report success if we are over the limit of maxBackpressure */
    if (webSocketContextData->maxBackpressure && webSocketContextData->maxBackpressure < getBufferedAmount()) {
        /* Also defer a close if we should */
        if (webSocketContextData->closeOnBackpressureLimit) {
            us_socket_shutdown_read(SSL, (us_socket_t *) this);
        }
        return true;
    }

    /* Transform the message to compressed domain if requested */
    if (compress) {
        WebSocketData *webSocketData = (WebSocketData *) Super::getAsyncSocketData();

That way the message is lost and the user doesn't know it was not sent.
This doesn't seem acceptable to me, but I am clearly missing something here.
What am I missing ?

Also, in the main comment you have

Returns false on increased user space backpressure.

What do you mean by "user space backpressure" ?

[ghost]

If you try to send over the backpressure limit the message will be dropped (because a limit is a limit). Returning true or false doesn't really matter - maybe false is more logical but it still doesn't say that the message was dropped. You have to check getBufferedAmount before sending if you care to know for sure. Or just set maxBackpressure to 0. But then you definitely have to check getBufferedAmount before sending either way.

[ghost]

User space backpressure is backpressure in user space ;)

[ronen37]

I understand that a limit is a limit, and the message has to be dropped.
The user can and should avoid this situation, but if he didn't because of a bug or something, he should get a clear indication that this is what happened so he can recover or fix what caused it in the first place.
dropping the message silently will cause an unexpected application behavior that is difficult to debug.
If neither true or false can be used for this, maybe another way is appropriate for indicating it.

[ghost]

Yes but that would break current API. The backpressure feature was bolted-on without messing with existing API (which is pretty much the same since 2018).

Maybe a swap to int instead of bool could work if values map to true/false in a backwards compatible way (Dropped = 2 = true, Sent = 1 = true)

[ghost]

#1183