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

Connectivity::Connection_closed application trigger type is confusing #410

Open
LPardue opened this issue Mar 8, 2024 · 1 comment
Open

Comments

@LPardue
Copy link
Member

LPardue commented Mar 8, 2024

ConnectivityConnectionClosed = {

    ; which side closed the connection
    ? owner: Owner
    ? connection_code: TransportError /
                       CryptoError /
                       uint32
    ? application_code: $ApplicationError /
                        uint32
    ? internal_code: uint32
    ? reason: text
    ? trigger:
        "clean" /
        "handshake_timeout" /
        "idle_timeout" /
        ; this is called the "immediate close" in the QUIC RFC
        "error" /
        "stateless_reset" /
        "version_mismatch" /
        ; for example HTTP/3's GOAWAY frame
        "application"
}

GOAWAY doesn't close the connection, so I find this example a bit confusing.

In quiche, I would be able to close a connection using transport or an application CONNECTION_CLOSE and log that information in the other fields like connection_code etc.

Maybe we should remove that trigger variant?

@rmarx
Copy link
Contributor

rmarx commented Mar 8, 2024

I see the confusion. I think the intent was to make a difference between e.g., a programmer-triggered close on the QUIC level ("clean") vs a GOAWAY triggered one at the application level. Even though the GOAWAY doesn't immediately trigger the close directly, it was the conceptual "trigger" that led to the connection being closed down. Of course, you can make an argument that stacks won't track this and will not log this, and I can follow that :)

I was also thinking of non-HTTP3 stuff running over QUIC however that might have application-level direct-close things (especially at the side initiating the close of course), so I wouldn't just do away with application here. Maybe just remove the comment?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants