-
Notifications
You must be signed in to change notification settings - Fork 124
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
fix(client): exit with non-zero on error #1786
Conversation
When a connection closes with an error, surface the error to the user and exit with non-zero.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1786 +/- ##
==========================================
+ Coverage 93.12% 93.14% +0.02%
==========================================
Files 116 116
Lines 36097 36097
==========================================
+ Hits 33614 33624 +10
+ Misses 2483 2473 -10 ☔ View full report in Codecov by Sentry. |
Benchmark resultsPerformance differences relative to 992d588.
Client/server transfer resultsTransfer of 134217728 bytes over loopback.
|
I think this may have broken some neqo-client QUIC interop tests. I'm looking at the resumption ( Any server that does not send a connection close before
I'm guessing this PR isn't the root cause, but it is causing an underlying issue (the client not exiting cleanly after the file has been transferred) to surface |
Thank you for tracing this down @WesleyRosenblum. I will take a look. |
@mxinden shouldn't the QNS CI step have caught that? (Did I break it?) |
Still need to take a look.
We are not running the resumption test on CI yet :( I am lagging behind with #1785. neqo/.github/workflows/qns.yml Line 74 in d74ad5e
|
Test failure reported in mozilla#1786 (comment). Signed-off-by: Max Inden <mail@max-inden.de>
* feat(qns): add resumption testcase Test failure reported in #1786 (comment). Signed-off-by: Max Inden <mail@max-inden.de> * fix: fallback for servers not sending NEW_TOKEN frame See https://github.com/mozilla/neqo/blob/main/neqo-transport/src/connection/mod.rs#L665-L676 for details. Fixes regression introduced in #1676. * Trigger CI * Still wait if there is none * Revert "Still wait if there is none" This reverts commit 710c500. * Refactor resumption logic --------- Signed-off-by: Max Inden <mail@max-inden.de>
There are two server implementations based on neqo: 1. https://github.com/mozilla/neqo/tree/main/neqo-bin/src/server - http3 and http09 implementation - used for manual testing and QUIC Interop 2. https://searchfox.org/mozilla-central/source/netwerk/test/http3server/src/main.rs - used to test Firefox I assume one was once an exact copy of the other. Both implement their own I/O, event loop, ... Since then, the two implementations diverged significantly. Especially (1) saw a lot of improvements in recent months: - mozilla#1564 - mozilla#1569 - mozilla#1578 - mozilla#1581 - mozilla#1604 - mozilla#1612 - mozilla#1676 - mozilla#1692 - mozilla#1707 - mozilla#1708 - mozilla#1727 - mozilla#1753 - mozilla#1756 - mozilla#1766 - mozilla#1772 - mozilla#1786 - mozilla#1787 - mozilla#1788 - mozilla#1794 - mozilla#1806 - mozilla#1808 - mozilla#1848 - mozilla#1866 At this point, bugs in (2) are hard to fix, see e.g. mozilla#1801. This commit merges (2) into (1), thus removing all duplicate logic and having (2) benefit from all the recent improvements to (1).
There are two server implementations based on neqo: 1. https://github.com/mozilla/neqo/tree/main/neqo-bin/src/server - http3 and http09 implementation - used for manual testing and QUIC Interop 2. https://searchfox.org/mozilla-central/source/netwerk/test/http3server/src/main.rs - used to test Firefox I assume one was once an exact copy of the other. Both implement their own I/O, event loop, ... Since then, the two implementations diverged significantly. Especially (1) saw a lot of improvements in recent months: - mozilla#1564 - mozilla#1569 - mozilla#1578 - mozilla#1581 - mozilla#1604 - mozilla#1612 - mozilla#1676 - mozilla#1692 - mozilla#1707 - mozilla#1708 - mozilla#1727 - mozilla#1753 - mozilla#1756 - mozilla#1766 - mozilla#1772 - mozilla#1786 - mozilla#1787 - mozilla#1788 - mozilla#1794 - mozilla#1806 - mozilla#1808 - mozilla#1848 - mozilla#1866 At this point, bugs in (2) are hard to fix, see e.g. mozilla#1801. This commit merges (2) into (1), thus removing all duplicate logic and having (2) benefit from all the recent improvements to (1).
* refactor(bin): introduce server/http3.rs and server/http09.rs The QUIC Interop Runner requires an http3 and http09 implementation for both client and server. The client code is already structured into an http3 and an http09 implementation since #1727. This commit does the same for the server side, i.e. splits the http3 and http09 implementation into separate Rust modules. * refactor: merge mozilla-central http3 server into neqo-bin There are two server implementations based on neqo: 1. https://github.com/mozilla/neqo/tree/main/neqo-bin/src/server - http3 and http09 implementation - used for manual testing and QUIC Interop 2. https://searchfox.org/mozilla-central/source/netwerk/test/http3server/src/main.rs - used to test Firefox I assume one was once an exact copy of the other. Both implement their own I/O, event loop, ... Since then, the two implementations diverged significantly. Especially (1) saw a lot of improvements in recent months: - #1564 - #1569 - #1578 - #1581 - #1604 - #1612 - #1676 - #1692 - #1707 - #1708 - #1727 - #1753 - #1756 - #1766 - #1772 - #1786 - #1787 - #1788 - #1794 - #1806 - #1808 - #1848 - #1866 At this point, bugs in (2) are hard to fix, see e.g. #1801. This commit merges (2) into (1), thus removing all duplicate logic and having (2) benefit from all the recent improvements to (1). * Move firefox.rs to mozilla-central * Reduce HttpServer trait functions * Extract constructor * Remove unused deps * Remove clap color feature Nice to have. Adds multiple dependencies. Hard to justify for mozilla-central.
When a connection closes with an error, surface the error to the user and exit with non-zero.