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

h3i: implement close trigger frames #1890

Merged
merged 1 commit into from
Jan 13, 2025

Conversation

evanrittenhouse
Copy link
Contributor

@evanrittenhouse evanrittenhouse commented Dec 19, 2024

  • Squash commits

Close trigger frames allow the user to specify a list of frames that the h3i client expects to receive over a given connection.

If h3i sees all of the close trigger frames over the course of the connection, it will pre-emptively close the connection with a CONNECTION_CLOSE frame. If h3i does not see all of the trigger frames, the resulting ConnectionSummary will contain a list of the missing target frames for future inspection.

This gives users a way to close tests out without waiting for the idle timeout, or adding Wait/ConnectionClose actions to the end of each test. This should vastly speed up test suites that have a large number of h3i tests.

In the absence of full-fledged integration tests, I modified the content_length_mismatch example to expect a 400, and remove the existing Wait/CC actions:

-        Action::Wait {
-            wait_type: WaitType::StreamEvent(StreamEvent {
-                stream_id: STREAM_ID,
-                event_type: StreamEventType::Headers,
-            }),
-        },
-        Action::ConnectionClose {
-            error: quiche::ConnectionError {
-                is_app: true,
-                error_code: quiche::h3::WireErrorCode::NoError as u64,
-                reason: vec![],
-            },
-        },
     ];
 
-    let summary =
-        sync_client::connect(config, &actions, None).expect("connection failed");
+    let summary = sync_client::connect(
+        config,
+        &actions,
+        Some(vec![ExpectedFrame::new(
+            0,
+            vec![quiche::h3::Header::new(b":status", b"400")].into(),
+        )]),
+    )
+    .expect("connection failed");

Taking a PCAP:
Screenshot 2024-12-19 at 10 27 50 AM

You can see that the client instantly sends the CC frame upon receiving the headers frame, without us needing to specify the action.

@evanrittenhouse evanrittenhouse requested a review from a team as a code owner December 19, 2024 18:28
@evanrittenhouse evanrittenhouse force-pushed the evanrittenhouse/target-frame branch 6 times, most recently from 54874db to e0ff2ba Compare December 20, 2024 00:00
@evanrittenhouse evanrittenhouse force-pushed the evanrittenhouse/target-frame branch from e0ff2ba to 8ac9f2c Compare December 20, 2024 19:08
h3i/src/client/connection_summary.rs Outdated Show resolved Hide resolved
h3i/src/client/connection_summary.rs Outdated Show resolved Hide resolved
h3i/src/client/connection_summary.rs Outdated Show resolved Hide resolved
h3i/src/client/connection_summary.rs Outdated Show resolved Hide resolved
h3i/src/client/connection_summary.rs Outdated Show resolved Hide resolved
@evanrittenhouse evanrittenhouse force-pushed the evanrittenhouse/target-frame branch 3 times, most recently from e4dbd46 to f6f9be3 Compare December 20, 2024 20:16
@evanrittenhouse evanrittenhouse changed the title h3i: implement expected frames [DO NOT MEGE] h3i: implement expected frames Dec 20, 2024
@evanrittenhouse evanrittenhouse force-pushed the evanrittenhouse/target-frame branch 3 times, most recently from 545f564 to 4f0ea98 Compare December 21, 2024 07:28
@evanrittenhouse evanrittenhouse changed the title [DO NOT MEGE] h3i: implement expected frames h3i: implement expected frames Jan 3, 2025
@evanrittenhouse evanrittenhouse changed the title h3i: implement expected frames [DRAFT] h3i: implement expected frames Jan 5, 2025
@evanrittenhouse evanrittenhouse force-pushed the evanrittenhouse/target-frame branch from 897d31e to 09db922 Compare January 6, 2025 01:05
@evanrittenhouse evanrittenhouse marked this pull request as draft January 6, 2025 01:17
@evanrittenhouse evanrittenhouse force-pushed the evanrittenhouse/target-frame branch 3 times, most recently from 5dbf361 to 09db922 Compare January 6, 2025 01:31
@evanrittenhouse evanrittenhouse force-pushed the evanrittenhouse/target-frame branch 3 times, most recently from 6d879e0 to ae37f11 Compare January 6, 2025 21:37
@evanrittenhouse evanrittenhouse marked this pull request as ready for review January 7, 2025 02:52
@evanrittenhouse evanrittenhouse changed the title [DRAFT] h3i: implement expected frames h3i: implement expected frames Jan 7, 2025
@evanrittenhouse evanrittenhouse force-pushed the evanrittenhouse/target-frame branch 2 times, most recently from 9cfce61 to 524c45c Compare January 8, 2025 16:48
Copy link
Contributor

@LPardue LPardue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly just nits

h3i/src/client/sync_client.rs Outdated Show resolved Hide resolved
h3i/examples/content_length_mismatch.rs Outdated Show resolved Hide resolved
h3i/src/client/connection_summary.rs Show resolved Hide resolved
h3i/src/client/connection_summary.rs Outdated Show resolved Hide resolved
h3i/src/client/connection_summary.rs Outdated Show resolved Hide resolved
h3i/src/client/connection_summary.rs Outdated Show resolved Hide resolved
h3i/src/client/connection_summary.rs Outdated Show resolved Hide resolved
h3i/src/client/sync_client.rs Outdated Show resolved Hide resolved
h3i/src/lib.rs Outdated Show resolved Hide resolved
h3i/src/main.rs Outdated Show resolved Hide resolved
@evanrittenhouse evanrittenhouse force-pushed the evanrittenhouse/target-frame branch 3 times, most recently from 2b1fa86 to da3a38b Compare January 9, 2025 22:52
@evanrittenhouse evanrittenhouse changed the base branch from master to evanrittenhouse/readme January 9, 2025 22:53
@evanrittenhouse evanrittenhouse force-pushed the evanrittenhouse/readme branch 2 times, most recently from a7a1b62 to 271babb Compare January 9, 2025 23:21
@evanrittenhouse evanrittenhouse force-pushed the evanrittenhouse/target-frame branch from da3a38b to 5a9da5d Compare January 9, 2025 23:21
Base automatically changed from evanrittenhouse/readme to master January 10, 2025 10:35
@evanrittenhouse evanrittenhouse force-pushed the evanrittenhouse/target-frame branch from 5a9da5d to 907adfe Compare January 10, 2025 13:21
@evanrittenhouse evanrittenhouse force-pushed the evanrittenhouse/target-frame branch 2 times, most recently from ded0f03 to 059ea03 Compare January 10, 2025 14:50
@evanrittenhouse evanrittenhouse changed the title h3i: implement expected frames h3i: implement close trigger frames Jan 10, 2025
@evanrittenhouse evanrittenhouse force-pushed the evanrittenhouse/target-frame branch from 059ea03 to 42bc3c6 Compare January 10, 2025 21:00
Close trigger frames allow the user to specify a list of frames that the
h3i client expects to receive over a given connection.

If h3i sees all close triggers over the course of the connection, it
will pre-emptively close the connection with a CONNECTION_CLOSE frame.
If h3i does _not_ see all of the triggers, the resulting
ConnectionSummary will contain a list of the missing triggers for future
inspection.

This gives users a way to close tests out without waiting for the idle
timeout, or adding Wait/ConnectionClose actions to the end of each test.
This should vastly speed up test suites that have a large number of h3i
tests.
@evanrittenhouse evanrittenhouse force-pushed the evanrittenhouse/target-frame branch from 42bc3c6 to 623f8db Compare January 13, 2025 13:50
Copy link
Contributor

@LPardue LPardue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@evanrittenhouse evanrittenhouse merged commit 122780a into master Jan 13, 2025
36 checks passed
@evanrittenhouse evanrittenhouse deleted the evanrittenhouse/target-frame branch January 13, 2025 18:17
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

Successfully merging this pull request may close these issues.

2 participants