[malkerdi]

Why

A connect middleware that does slow work (an auth lookup, for example) cannot tell whether the client is still there when the work finishes: Socket::connected() is false for every socket until after the connect packet, and the engine socket is private. So the middleware accepts a client that already left.

Also, when the client drops right before the connect packet is sent, Namespace::connect removes the socket directly and no disconnect handler runs, so bookkeeping a middleware set up (a connection counter, for example) is never undone.

What

• Socket::is_closed(): passthrough to the engine socket, meaningful inside a middleware.
• Namespace::connect: on a failed connect packet, close the socket through Socket::close with TransportError so disconnect handlers run, instead of remove_socket.

Tests

Two integration tests in tests/connect.rs on the websocket fixture: is_closed() flips to true after the client drops mid-middleware, and the disconnect handler fires with TransportError when the client drops before the connect packet (fails without the ns.rs change).

[codspeed]

Merging this PR will not alter performance

⚠️ 4 benchmarks spent significant time in system calls

System calls cannot be consistently instrumented, so they are not included in the measure, which understates the real cost. Please switch to the Walltime instrument to accurately measure system calls.

Measurement and system calls

• Decode packet binary ack on /custom_nsp
• Decode packet binary ack on /
• Decode packet binary event on /
• Decode packet binary event on /custom_nsp

✅ 87 untouched benchmarks

---

_{Comparing malkerdi:expose-engine-socket-closed (0047482) with main (6b2d605)}

[Totodore]

Hey, thanks for your contribution. I'd like to solve your problem with a more idiomatic solution, IMO running disconnect handler whereas the socket is not yet connected is not the appropriate solution. I agree that we should make an is_closed fn and we should also add a closed() fn:

/// # Wait for the underlying engine.io connection to be closed.
///
/// Resolves when the client disconnects or the server closes the transport.
/// Inside a connect middleware, this lets slow work (an auth lookup, say)
/// be abandoned as soon as the client leaves.
pub async fn closed(&self) {
    self.esocket.closed().await
}

You could then use it like that:

async |s: SocketRef| {
    tokio::select! {
        res = auth_lookup(&s) => res,
        _ = s.closed() => Err(AuthError::ClientGone),
    }
}

About your issue, what are the reason for handling of your bookkeeping in the middleware rather than in the connect handler, is it because it needs to run before connection or is it because you want composability?

[malkerdi]

Hey, thanks for your contribution. I'd like to solve your problem with a more idiomatic solution, IMO running disconnect handler whereas the socket is not yet connected is not the appropriate solution. I agree that we should make an is_closed fn and we should also add a closed() fn:

/// # Wait for the underlying engine.io connection to be closed.
///
/// Resolves when the client disconnects or the server closes the transport.
/// Inside a connect middleware, this lets slow work (an auth lookup, say)
/// be abandoned as soon as the client leaves.
pub async fn closed(&self) {
    self.esocket.closed().await
}

You could then use it like that:

async |s: SocketRef| {
    tokio::select! {
        res = auth_lookup(&s) => res,
        _ = s.closed() => Err(AuthError::ClientGone),
    }
}

About your issue, what are the reason for handling of your bookkeeping in the middleware rather than in the connect handler, is it because it needs to run before connection or is it because you want composability?

Agreed on is_closed(), and closed() is a good addition.

On the bookkeeping: it has to run before the connection, composability is not the reason. The connect handler is not a safe place for it because of this window in Namespace::connect:

The socket is inserted into ns.sockets before the connect packet is sent, and the connect handler runs (spawned, when async) after it. If the engine closes between that insertion and the handler's on_disconnect registration, Socket::close finds no disconnect handler and removes the socket directly. The handler registered a moment later never runs, so the counter it incremented is never decremented. We used to work around this with a connected() recheck plus a once-flag in the connect handler. Registering on_disconnect in the middleware, before the socket enters the namespace, removes the race. (The event handlers go there for the same reason: they must exist before the connect packet goes out.)

Once the disconnect handler lives in the middleware, the only path where it is skipped is the failed connect packet, which is what the ns.rs change covers. I agree it is odd to fire disconnect for a socket that never got connected. What we need is a guarantee that a disconnect handler registered before the socket enters the namespace always runs when the socket leaves it. Running it on that path, or a dedicated hook for a failed connect, would both work for us.

Thanks!