Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Session not terminating upon closed CONNECT stream #197

Closed
friendlymatthew opened this issue Aug 7, 2024 · 9 comments · Fixed by #239
Closed

Session not terminating upon closed CONNECT stream #197

friendlymatthew opened this issue Aug 7, 2024 · 9 comments · Fixed by #239
Assignees
Labels
bug Something isn't working enhancement New feature or request

Comments

@friendlymatthew
Copy link

friendlymatthew commented Aug 7, 2024

The wtransport server is not terminating the session upon receiving a closed CONNECT stream.

According to the WebTransport Internet Draft:

WebTransport sessions are initiated inside a given HTTP/3 connection by the client, who sends an extended CONNECT request. If the server accepts the request, a WebTransport session is established. The resulting stream will be further referred to as a CONNECT stream, and its stream ID is used to uniquely identify a given WebTransport session within the connection. The ID of the CONNECT stream that established a given WebTransport session will be further referred to as a Session ID.

A WebTransport session is terminated when the CONNECT stream that created it is closed.

Also under 5. Session Termination

A WebTransport session over HTTP/3 is considered terminated when either of the following conditions is met:

  1. the CONNECT stream is closed, either cleanly or abruptly, on either side; or
  2. a CLOSE_WEBTRANSPORT_SESSION capsule is either sent or received.

Ruling out quinn

I added tracing logs in quinn to see the various Frames processed and dispatched. Maybe the desired Frame wasn't being sent!

To verify this wasn't a problem on quinn's end, I wrote a JS client in Chrome. That is:

options = {
    "congestionControl":"low-latency",
    "requireUnreliable":true,
    "serverCertificateHashes": [
        {
            "algorithm":"sha-256",
            "value": "some_certificate_hash"
        }
    ]
}

webtransport = new WebTransport(url, options);

To close the session, I called:

await webtransport.close();

When closing, I immediately receive a Frame::Stream from quinn:

Screenshot 2024-08-07 at 9 44 20 AM

This payload indicates that the CONNECT stream has been closed. This is because the session_id is 0 and fin is set to true.

Testing this behavior on Firefox, I received a CLOSE_WEBTRANSPORT_SESSION upon closing the JS client. This is fine since it is specified in the Session Termination section of the internet draft.

Our wtransport server is similar to the implementation in examples. It seems other people are experiencing #176 (comment) @BiagioFesta hinted this may be a browser bug as well.

Specifications

  • wtransport version: 0.1.12
  • Browsers tested:
    - Chrome (Version 127.0.6533.89)
    - (Firefox 129.0 (64-bit))

Issues mentioned

#176 (comment)

@BiagioFesta
Copy link
Owner

SessionStream (i.e., CONNECT stream) is not currently handled. See:

// TODO(biagio): run stream_session

@BiagioFesta BiagioFesta added bug Something isn't working enhancement New feature or request labels Aug 7, 2024
@ktff
Copy link
Contributor

ktff commented Sep 20, 2024

@BiagioFesta if nobody is working on this I'll start with it.

@BiagioFesta
Copy link
Owner

Please, it would be great. Definitely you can try a shot.

I started a branch: https://github.com/BiagioFesta/wtransport/tree/close-session-capsule (close-session-capsule) you might want to have a look.

In particular, the commit ae63af0 implementing the capsule protocol, with the webtransport close session capsule

@ktff
Copy link
Contributor

ktff commented Sep 22, 2024

Nice, I'll take a look.

@wegylexy
Copy link

Image
awaiting connection.closed() results in connection timed out instead of the expected ApplicationClosed

@wegylexy
Copy link

I figure that I only need to tokio::select! on accept_uni, accept_bi, and receive_datagram for ApplicationClosed to propagate as expected. However, the connection is not closed right after the driver reports the code and reason, but stays until idle timeout.

@wegylexy
Copy link

So, #8 is unresolved yet.

@ktff
Copy link
Contributor

ktff commented Feb 12, 2025

@wegylexy can you provide a minimal example? Also what version of this crate are you using?

Based on the logs, the closure was detected and its propagation to local streams is right after it so I'm not sure what went wrong in your case.

@wegylexy
Copy link

wegylexy commented Feb 12, 2025

@ktff If you look at the timestamps, the propagation took 30 seconds, which is exactly my configured max_idle_timeout. I tried different timeout durations, and confirmed it took such configured duration for the following code to return the ConnectionError::ApplicationClosed after the underlying driver debug! log.

// server.rs
// following example server code but using latest commit on `master` branch
async fn handle_session_core(incoming: IncomingSession) -> Result<()> {
    let request = incoming.await?;
    let connection = request.accept().await?
    tokio::select! {
        biased; // <-- doesn't matter
        // closed = connection.closed() => { /* only TimedOut in this case, no ApplicationClosed */ }
        stream = connection.accept_bi() => { stream? }
        stream = connection.accept_uni() => { stream? }
        datagram = connection.receive_datagram() => { datagram? }
    }
}
// all 3 cases propagate the ConnectionError::ApplicationClosed after max_idle_timeout
// client.js
const wt = new WebTransport(..., { requireUnreliable: true, congestionControl: "low-latency", ... });
// must wait until connected for closing code and reason to go through
wt.close(); // with code and reason or not
await wt.closed // if didn't wait til connected, promise is rejected immediately; server won't know

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants