Merge pull request #2210 from lucas-clemente/application-error-codes

use the application-specific variant of CONNECTION_CLOSE

internal/qerr/quic_error.go

@@ -39,6 +39,7 @@ func CryptoError(tlsAlert uint8, errorMessage string) *QuicError {
 	}
 }
 
+// ApplicationError creates a new QuicError instance for an application error
 func ApplicationError(errorCode ErrorCode, errorMessage string) *QuicError {
 	return &QuicError{
 		ErrorCode:          errorCode,
@@ -65,6 +66,11 @@ func (e *QuicError) IsCryptoError() bool {
 	return e.ErrorCode.isCryptoError()
 }
 
+// IsApplicationError says if this error is an application error
+func (e *QuicError) IsApplicationError() bool {
+	return e.isApplicationError
+}
+
 // Temporary says if the error is temporary.
 func (e *QuicError) Temporary() bool {
 	return false

internal/qerr/quic_error_test.go

@@ -11,6 +11,7 @@ var _ = Describe("QUIC Transport Errors", func() {
 	It("has a string representation", func() {
 		err := Error(FlowControlError, "foobar")
 		Expect(err.Timeout()).To(BeFalse())
+		Expect(err.IsApplicationError()).To(BeFalse())
 		Expect(err.Error()).To(Equal("FLOW_CONTROL_ERROR: foobar"))
 	})
 
@@ -38,13 +39,17 @@ var _ = Describe("QUIC Transport Errors", func() {
 
 		It("says if an error is a crypto error", func() {
 			Expect(Error(FlowControlError, "").IsCryptoError()).To(BeFalse())
-			Expect(CryptoError(42, "").IsCryptoError()).To(BeTrue())
+			err := CryptoError(42, "")
+			Expect(err.IsCryptoError()).To(BeTrue())
+			Expect(err.IsApplicationError()).To(BeFalse())
+
 		})
 	})
 
 	Context("application errors", func() {
 		It("has a string representation for errors with a message", func() {
 			err := ApplicationError(0x42, "foobar")
+			Expect(err.IsApplicationError()).To(BeTrue())
 			Expect(err.Error()).To(Equal("Application error 0x42: foobar"))
 		})
 

session.go

@@ -1023,14 +1023,14 @@ func (s *session) Close() error {
 }
 
 func (s *session) CloseWithError(code protocol.ApplicationErrorCode, desc string) error {
-	s.closeLocal(qerr.Error(qerr.ErrorCode(code), desc))
+	s.closeLocal(qerr.ApplicationError(qerr.ErrorCode(code), desc))
 	<-s.ctx.Done()
 	return nil
 }
 
 func (s *session) handleCloseError(closeErr closeError) {
 	if closeErr.err == nil {
-		closeErr.err = qerr.NoError
+		closeErr.err = qerr.ApplicationError(0, "")
 	}
 
 	var quicErr *qerr.QuicError
@@ -1250,8 +1250,9 @@ func (s *session) sendConnectionClose(quicErr *qerr.QuicError) ([]byte, error) {
 		reason = quicErr.ErrorMessage
 	}
 	packet, err := s.packer.PackConnectionClose(&wire.ConnectionCloseFrame{
-		ErrorCode:    quicErr.ErrorCode,
-		ReasonPhrase: reason,
+		IsApplicationError: quicErr.IsApplicationError(),
+		ErrorCode:          quicErr.ErrorCode,
+		ReasonPhrase:       reason,
 	})
 	if err != nil {
 		return nil, err

session_test.go

@@ -426,10 +426,16 @@ var _ = Describe("Session", func() {
 		})
 
 		It("shuts down without error", func() {
-			streamManager.EXPECT().CloseWithError(qerr.Error(qerr.NoError, ""))
+			streamManager.EXPECT().CloseWithError(qerr.ApplicationError(0, ""))
 			expectReplaceWithClosed()
 			cryptoSetup.EXPECT().Close()
-			packer.EXPECT().PackConnectionClose(gomock.Any()).Return(&packedPacket{raw: []byte("connection close")}, nil)
+			packer.EXPECT().PackConnectionClose(gomock.Any()).DoAndReturn(func(f *wire.ConnectionCloseFrame) (*packedPacket, error) {
+				Expect(f.IsApplicationError).To(BeTrue())
+				Expect(f.ErrorCode).To(Equal(qerr.NoError))
+				Expect(f.FrameType).To(BeZero())
+				Expect(f.ReasonPhrase).To(BeEmpty())
+				return &packedPacket{raw: []byte("connection close")}, nil
+			})
 			Expect(sess.Close()).To(Succeed())
 			Eventually(areSessionsRunning).Should(BeFalse())
 			Expect(mconn.written).To(HaveLen(1))
@@ -438,7 +444,7 @@ var _ = Describe("Session", func() {
 		})
 
 		It("only closes once", func() {
-			streamManager.EXPECT().CloseWithError(qerr.Error(qerr.NoError, ""))
+			streamManager.EXPECT().CloseWithError(gomock.Any())
 			expectReplaceWithClosed()
 			cryptoSetup.EXPECT().Close()
 			packer.EXPECT().PackConnectionClose(gomock.Any()).Return(&packedPacket{}, nil)
@@ -449,12 +455,17 @@ var _ = Describe("Session", func() {
 			Expect(sess.Context().Done()).To(BeClosed())
 		})
 
-		It("closes streams with proper error", func() {
+		It("closes with an error", func() {
 			testErr := errors.New("test error")
-			streamManager.EXPECT().CloseWithError(qerr.Error(0x1337, testErr.Error()))
+			streamManager.EXPECT().CloseWithError(qerr.ApplicationError(0x1337, testErr.Error()))
 			expectReplaceWithClosed()
 			cryptoSetup.EXPECT().Close()
-			packer.EXPECT().PackConnectionClose(gomock.Any()).Return(&packedPacket{}, nil)
+			packer.EXPECT().PackConnectionClose(gomock.Any()).DoAndReturn(func(f *wire.ConnectionCloseFrame) (*packedPacket, error) {
+				Expect(f.IsApplicationError).To(BeTrue())
+				Expect(f.ErrorCode).To(BeEquivalentTo(0x1337))
+				Expect(f.ReasonPhrase).To(Equal("test error"))
+				return &packedPacket{}, nil
+			})
 			sess.CloseWithError(0x1337, testErr.Error())
 			Eventually(areSessionsRunning).Should(BeFalse())
 			Expect(sess.Context().Done()).To(BeClosed())
@@ -1203,7 +1214,7 @@ var _ = Describe("Session", func() {
 			defer GinkgoRecover()
 			cryptoSetup.EXPECT().RunHandshake().MaxTimes(1)
 			err := sess.run()
-			Expect(err).To(MatchError(qerr.Error(0x1337, testErr.Error())))
+			Expect(err).To(MatchError(qerr.ApplicationError(0x1337, testErr.Error())))
 			close(done)
 		}()
 		streamManager.EXPECT().CloseWithError(gomock.Any())