feat: expose more `connectionId` information

tegefaulkes commented 10 months ago

Description

I need to expose more connectionId information so I compare connections better. This exposes the destination id as connectionIdPeer of the connection as well as a derived connectionIdShared.

Changes:

Exposed connection destination ID
Created a common ID that is identical for both sides of the connection. Derived from the source and destination ID.
The quiche connection is now the Source Of Truth for these IDs. The destinationId is only know after establishing and is only valid while running.
Added test demonstrating the new Ids and how they relate to each other.

As for logging, I'm switching non-internal connection and stream error logs to debug level. closing stream or connection with a non-0 code is not exceptional. Internal errors are.

Issues Fixed

No issues directly addressed.
Related: https://github.com/MatrixAI/Polykey/pull/609

Tasks

[x] expose more connectionId information on the QUICConnection.
[x] Reduce ERROR and WARN level logs for failing connections and streams. Application can make a decision to log these as errors or not.

Final checklist

[x] Domain specific tests
[x] Full tests
[x] Updated inline-comment documentation
[x] Lint fixed
[x] Squash and rebased
[x] Sanity check the final build

ghost commented 10 months ago

👇 Click on the image for a new way to code review

#### [![Review these changes using an interactive CodeSee Map](https://s3.us-east-2.amazonaws.com/maps.codesee.io/images/github/MatrixAI/js-quic/73/16220582/e4fe8487cb7657c8ad29965bde3ba1973c3c1f7e.svg)](https://app.codesee.io/r/reviews?pr=73&src=https%3A%2F%2Fgithub.com%2FMatrixAI%2Fjs-quic) #### Legend CodeSee Map legend

tegefaulkes commented 10 months ago

It made more sense to me to get the source ID and destination ID from quiche. It would be the best source of truth.

It's possible that these can change over the life of the connection and that would cause problems with QUICSocket logic. But that would only happen if the connection switches paths and I don't think we currently support that.

tegefaulkes commented 10 months ago

I'd like the logging messages for connections to use a shared connection ID that is the same for both sides of the connection.

CMCDragonkai commented 10 months ago

I don't think connection IDs can change. There is a connection migration thing but the ID is meant to be stable.

I'm wondering what is the main reason to expose these IDs and that the existing connection ID isn't sufficient?

tegefaulkes commented 10 months ago

We only expose the source ID of the connection. But there are two IDs, the source and destination, these are reversed when comparing the two ends of the connection. So both sides have a different connectionId. So I can't actually tell what connections form pairs when logging because we only have half the information we need.

On top of that, I needed some kind of information that is shared between the two connections for making some decisions in https://github.com/MatrixAI/Polykey/issues/592. So it's just good to have.

tegefaulkes commented 10 months ago

I don't know the details, but I think connection IDs can change when the connection migrates.

CMCDragonkai commented 10 months ago

In QUIC as specified by RFC 9000, connection migration is a feature that allows the endpoints to change their IP address and/or port without terminating the existing connection. This is particularly useful for mobile clients that may switch between different networks, such as WiFi and cellular data.

The Connection ID (CID) is an identifier used in QUIC to distinguish different connections, and it can be used to route packets to the correct endpoint. The CID is especially useful in connection migration scenarios where the IP address and/or port may change. It provides a stable identifier that survives such changes, enabling uninterrupted communication.

In the context of connection migration in QUIC as per RFC 9000, changing the Connection ID is not a requirement. The specification does allow for endpoints to change the CID when migrating, but this is optional. If the CID does change, the endpoints need to inform each other using specific QUIC frames, such as NEW_CONNECTION_ID, to communicate the new CIDs.

So to directly answer your question: No, the Connection ID does not necessarily have to change when a QUIC connection is migrated, according to RFC 9000. It's optional and based on the strategies employed by the endpoints.

tegefaulkes commented 10 months ago

https://docs.rs/quiche/latest/quiche/struct.Connection.html#method.source_id

Returns the source connection ID.

When there are multiple IDs, and if there is an active path, the ID used on that path is returned. Otherwise the oldest ID is returned.

Note that the value returned can change throughout the connection’s lifetime.

So according to quiche, the source and destination ID can change based on what path is being used. I don't know if we support paths. I don't think we do, so I don't think it's a problem. But it's something to be aware of.

tegefaulkes commented 10 months ago

I've changed any QUICServer, QUICClient and QUICConnection error logs to info level and any in QUICStream to debug level.

MatrixAI / js-quic