From 4378283f953d4e2a4a6d4407b90872f270ce3c32 Mon Sep 17 00:00:00 2001 From: Ferdinand Holzer Date: Tue, 18 Jul 2023 06:31:31 +0200 Subject: [PATCH] surface connection error as connection context cancelation cause (#3961) * connection: surface connection error as connection context cancellation cause * docs: add note about connection context canellation cause --- README.md | 2 +- connection.go | 16 ++++++++-------- connection_test.go | 14 ++++++++++---- interface.go | 2 ++ 4 files changed, 21 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index 3cc6b5a9c..3eaf0583b 100644 --- a/README.md +++ b/README.md @@ -132,7 +132,7 @@ The `quic.Transport` contains a few configuration options that don't apply to an #### When the remote Peer closes the Connection -In case the peer closes the QUIC connection, all calls to open streams, accept streams, as well as all methods on streams immediately return an error. Users can use errors assertions to find out what exactly went wrong: +In case the peer closes the QUIC connection, all calls to open streams, accept streams, as well as all methods on streams immediately return an error. Additionally, it is set as cancellation cause of the connection context. Users can use errors assertions to find out what exactly went wrong: * `quic.VersionNegotiationError`: Happens during the handshake, if there is no overlap between our and the remote's supported QUIC versions. * `quic.HandshakeTimeoutError`: Happens if the QUIC handshake doesn't complete within the time specified in `quic.Config.HandshakeTimeout`. diff --git a/connection.go b/connection.go index 9afd0ca03..4823cd463 100644 --- a/connection.go +++ b/connection.go @@ -176,7 +176,7 @@ type connection struct { closeChan chan closeError ctx context.Context - ctxCancel context.CancelFunc + ctxCancel context.CancelCauseFunc handshakeCtx context.Context handshakeCtxCancel context.CancelFunc @@ -283,7 +283,7 @@ var newConnection = func( connIDGenerator, ) s.preSetup() - s.ctx, s.ctxCancel = context.WithCancel(context.WithValue(context.Background(), ConnectionTracingKey, tracingID)) + s.ctx, s.ctxCancel = context.WithCancelCause(context.WithValue(context.Background(), ConnectionTracingKey, tracingID)) s.sentPacketHandler, s.receivedPacketHandler = ackhandler.NewAckHandler( 0, getMaxPacketSize(s.conn.RemoteAddr()), @@ -404,7 +404,7 @@ var newClientConnection = func( connIDGenerator, ) s.preSetup() - s.ctx, s.ctxCancel = context.WithCancel(context.WithValue(context.Background(), ConnectionTracingKey, tracingID)) + s.ctx, s.ctxCancel = context.WithCancelCause(context.WithValue(context.Background(), ConnectionTracingKey, tracingID)) s.sentPacketHandler, s.receivedPacketHandler = ackhandler.NewAckHandler( initialPacketNumber, getMaxPacketSize(s.conn.RemoteAddr()), @@ -525,7 +525,10 @@ func (s *connection) preSetup() { // run the connection main loop func (s *connection) run() error { - defer s.ctxCancel() + var closeErr closeError + defer func() { + s.ctxCancel(closeErr.err) + }() s.timer = *newTimer() @@ -552,10 +555,7 @@ func (s *connection) run() error { } } - var ( - closeErr closeError - sendQueueAvailable <-chan struct{} - ) + var sendQueueAvailable <-chan struct{} runLoop: for { diff --git a/connection_test.go b/connection_test.go index c8e7edb15..9eb602516 100644 --- a/connection_test.go +++ b/connection_test.go @@ -395,6 +395,7 @@ var _ = Describe("Connection", func() { } Expect(conn.handleFrame(ccf, protocol.Encryption1RTT, protocol.ConnectionID{})).To(Succeed()) Eventually(conn.Context().Done()).Should(BeClosed()) + Expect(context.Cause(conn.Context())).To(MatchError(testErr)) }) It("errors on HANDSHAKE_DONE frames", func() { @@ -499,6 +500,7 @@ var _ = Describe("Connection", func() { conn.CloseWithError(0x1337, "test error") Eventually(areConnsRunning).Should(BeFalse()) Expect(conn.Context().Done()).To(BeClosed()) + Expect(context.Cause(conn.Context())).To(MatchError(expectedErr)) }) It("includes the frame type in transport-level close frames", func() { @@ -566,6 +568,7 @@ var _ = Describe("Connection", func() { tracer.EXPECT().Close() conn.shutdown() Eventually(returned).Should(BeClosed()) + Expect(context.Cause(conn.Context())).To(MatchError(context.Canceled)) }) It("doesn't send any more packets after receiving a CONNECTION_CLOSE", func() { @@ -2010,19 +2013,21 @@ var _ = Describe("Connection", func() { tracer.EXPECT().Close() conn.shutdown() Eventually(done).Should(BeClosed()) + Expect(context.Cause(conn.Context())).To(MatchError(context.Canceled)) }) It("passes errors to the connection runner", func() { testErr := errors.New("handshake error") + expectedErr := &qerr.ApplicationError{ + ErrorCode: 0x1337, + ErrorMessage: testErr.Error(), + } done := make(chan struct{}) go func() { defer GinkgoRecover() cryptoSetup.EXPECT().StartHandshake().MaxTimes(1) err := conn.run() - Expect(err).To(MatchError(&qerr.ApplicationError{ - ErrorCode: 0x1337, - ErrorMessage: testErr.Error(), - })) + Expect(err).To(MatchError(expectedErr)) close(done) }() streamManager.EXPECT().CloseWithError(gomock.Any()) @@ -2034,6 +2039,7 @@ var _ = Describe("Connection", func() { tracer.EXPECT().Close() Expect(conn.CloseWithError(0x1337, testErr.Error())).To(Succeed()) Eventually(done).Should(BeClosed()) + Expect(context.Cause(conn.Context())).To(MatchError(expectedErr)) }) Context("transport parameters", func() { diff --git a/interface.go b/interface.go index e3c8d0614..265fe6923 100644 --- a/interface.go +++ b/interface.go @@ -178,6 +178,8 @@ type Connection interface { // The error string will be sent to the peer. CloseWithError(ApplicationErrorCode, string) error // Context returns a context that is cancelled when the connection is closed. + // The cancellation cause is set to the error that caused the connection to + // close, or `context.Canceled` in case the listener is closed first. Context() context.Context // ConnectionState returns basic details about the QUIC connection. // Warning: This API should not be considered stable and might change soon.