Browse Source

Add RST_STREAM info to errors (#123)

Motivation:

If a stream is reset by a remote peer the synthesized status doesn't
include the http/2 error code from the RST_STREAM frame which makes
debugging harder.

Modifications:

- Add the http/2 error code to the status message

Result:

Better error messages
George Barnett 5 months ago
parent
commit
e59b4cfc25

+ 2 - 2
Sources/GRPCNIOTransportCore/Client/GRPCClientStreamHandler.swift

@@ -132,8 +132,8 @@ extension GRPCClientStreamHandler {
         context.fireErrorCaught(error)
         context.fireErrorCaught(error)
       }
       }
 
 
-    case .rstStream:
-      self.handleUnexpectedInboundClose(context: context, reason: .streamReset)
+    case .rstStream(let errorCode):
+      self.handleUnexpectedInboundClose(context: context, reason: .streamReset(errorCode))
 
 
     case .ping, .goAway, .priority, .settings, .pushPromise, .windowUpdate,
     case .ping, .goAway, .priority, .settings, .pushPromise, .windowUpdate,
       .alternativeService, .origin:
       .alternativeService, .origin:

+ 46 - 3
Sources/GRPCNIOTransportCore/GRPCStreamStateMachine.swift

@@ -18,6 +18,7 @@ internal import GRPCCore
 internal import NIOCore
 internal import NIOCore
 internal import NIOHPACK
 internal import NIOHPACK
 internal import NIOHTTP1
 internal import NIOHTTP1
+internal import NIOHTTP2
 
 
 package enum Scheme: String {
 package enum Scheme: String {
   case http
   case http
@@ -613,7 +614,7 @@ struct GRPCStreamStateMachine {
   }
   }
 
 
   enum UnexpectedInboundCloseReason {
   enum UnexpectedInboundCloseReason {
-    case streamReset
+    case streamReset(HTTP2ErrorCode)
     case channelInactive
     case channelInactive
     case errorThrown(any Error)
     case errorThrown(any Error)
   }
   }
@@ -1924,10 +1925,11 @@ extension MethodDescriptor {
 extension RPCError {
 extension RPCError {
   fileprivate init(_ reason: GRPCStreamStateMachine.UnexpectedInboundCloseReason) {
   fileprivate init(_ reason: GRPCStreamStateMachine.UnexpectedInboundCloseReason) {
     switch reason {
     switch reason {
-    case .streamReset:
+    case .streamReset(let errorCode):
       self = RPCError(
       self = RPCError(
         code: .unavailable,
         code: .unavailable,
-        message: "Stream unexpectedly closed: a RST_STREAM frame was received."
+        message:
+          "Stream unexpectedly closed: received RST_STREAM frame (\(errorCode.shortDescription))."
       )
       )
     case .channelInactive:
     case .channelInactive:
       self = RPCError(code: .unavailable, message: "Stream unexpectedly closed.")
       self = RPCError(code: .unavailable, message: "Stream unexpectedly closed.")
@@ -1950,3 +1952,44 @@ extension RPCError {
     self = RPCError(code: .internalError, message: "Invalid state", cause: invalidState)
     self = RPCError(code: .internalError, message: "Invalid state", cause: invalidState)
   }
   }
 }
 }
+
+extension HTTP2ErrorCode {
+  var shortDescription: String {
+    let prefix = "0x" + String(self.networkCode, radix: 16) + ": "
+    let suffix: String
+
+    switch self {
+    case .noError:
+      suffix = "no error"
+    case .protocolError:
+      suffix = "protocol error"
+    case .internalError:
+      suffix = "internal error"
+    case .flowControlError:
+      suffix = "flow control error"
+    case .settingsTimeout:
+      suffix = "settings Timeout"
+    case .streamClosed:
+      suffix = "stream closed"
+    case .frameSizeError:
+      suffix = "frame size error"
+    case .refusedStream:
+      suffix = "fefused stream"
+    case .cancel:
+      suffix = "cancel"
+    case .compressionError:
+      suffix = "compression error"
+    case .connectError:
+      suffix = "connect error"
+    case .enhanceYourCalm:
+      suffix = "enhance your calm"
+    case .inadequateSecurity:
+      suffix = "inadequate security"
+    case .http11Required:
+      suffix = "HTTP/1.1 required"
+    default:
+      suffix = "unknown error"
+    }
+    return prefix + suffix
+  }
+}

+ 2 - 2
Sources/GRPCNIOTransportCore/Server/GRPCServerStreamHandler.swift

@@ -202,8 +202,8 @@ extension GRPCServerStreamHandler {
         context.fireErrorCaught(error)
         context.fireErrorCaught(error)
       }
       }
 
 
-    case .rstStream:
-      self.handleUnexpectedInboundClose(context: context, reason: .streamReset)
+    case .rstStream(let errorCode):
+      self.handleUnexpectedInboundClose(context: context, reason: .streamReset(errorCode))
 
 
     case .ping, .goAway, .priority, .settings, .pushPromise, .windowUpdate,
     case .ping, .goAway, .priority, .settings, .pushPromise, .windowUpdate,
       .alternativeService, .origin:
       .alternativeService, .origin:

+ 1 - 1
Tests/GRPCNIOTransportCoreTests/Client/GRPCClientStreamHandlerTests.swift

@@ -954,7 +954,7 @@ final class GRPCClientStreamHandlerTests: XCTestCase {
       .status(
       .status(
         .init(
         .init(
           code: .unavailable,
           code: .unavailable,
-          message: "Stream unexpectedly closed: a RST_STREAM frame was received."
+          message: "Stream unexpectedly closed: received RST_STREAM frame (0x2: internal error)."
         ),
         ),
         [:]
         [:]
       )
       )

+ 7 - 6
Tests/GRPCNIOTransportCoreTests/GRPCStreamStateMachineTests.swift

@@ -18,6 +18,7 @@ import GRPCCore
 import NIOCore
 import NIOCore
 import NIOEmbedded
 import NIOEmbedded
 import NIOHPACK
 import NIOHPACK
+import NIOHTTP2
 import XCTest
 import XCTest
 
 
 @testable import GRPCNIOTransportCore
 @testable import GRPCNIOTransportCore
@@ -975,10 +976,10 @@ final class GRPCStreamClientStateMachineTests: XCTestCase {
         Status(code: .unavailable, message: "Stream unexpectedly closed.")
         Status(code: .unavailable, message: "Stream unexpectedly closed.")
       ),
       ),
       (
       (
-        GRPCStreamStateMachine.UnexpectedInboundCloseReason.streamReset,
+        GRPCStreamStateMachine.UnexpectedInboundCloseReason.streamReset(.noError),
         Status(
         Status(
           code: .unavailable,
           code: .unavailable,
-          message: "Stream unexpectedly closed: a RST_STREAM frame was received."
+          message: "Stream unexpectedly closed: received RST_STREAM frame (0x0: no error)."
         )
         )
       ),
       ),
       (
       (
@@ -1022,7 +1023,7 @@ final class GRPCStreamClientStateMachineTests: XCTestCase {
   func testUnexpectedCloseWhenServerClosed() throws {
   func testUnexpectedCloseWhenServerClosed() throws {
     let closeReasons = [
     let closeReasons = [
       GRPCStreamStateMachine.UnexpectedInboundCloseReason.channelInactive,
       GRPCStreamStateMachine.UnexpectedInboundCloseReason.channelInactive,
-      .streamReset,
+      .streamReset(.noError),
       .errorThrown(RPCError(code: .deadlineExceeded, message: "Test error")),
       .errorThrown(RPCError(code: .deadlineExceeded, message: "Test error")),
     ]
     ]
     let states = [
     let states = [
@@ -2470,10 +2471,10 @@ final class GRPCStreamServerStateMachineTests: XCTestCase {
         RPCError(code: .unavailable, message: "Stream unexpectedly closed.")
         RPCError(code: .unavailable, message: "Stream unexpectedly closed.")
       ),
       ),
       (
       (
-        GRPCStreamStateMachine.UnexpectedInboundCloseReason.streamReset,
+        GRPCStreamStateMachine.UnexpectedInboundCloseReason.streamReset(.noError),
         RPCError(
         RPCError(
           code: .unavailable,
           code: .unavailable,
-          message: "Stream unexpectedly closed: a RST_STREAM frame was received."
+          message: "Stream unexpectedly closed: received RST_STREAM frame (0x0: no error)."
         )
         )
       ),
       ),
       (
       (
@@ -2514,7 +2515,7 @@ final class GRPCStreamServerStateMachineTests: XCTestCase {
   func testUnexpectedCloseWhenClientClosed() throws {
   func testUnexpectedCloseWhenClientClosed() throws {
     let closeReasons = [
     let closeReasons = [
       GRPCStreamStateMachine.UnexpectedInboundCloseReason.channelInactive,
       GRPCStreamStateMachine.UnexpectedInboundCloseReason.channelInactive,
-      .streamReset,
+      .streamReset(.noError),
       .errorThrown(RPCError(code: .deadlineExceeded, message: "Test error")),
       .errorThrown(RPCError(code: .deadlineExceeded, message: "Test error")),
     ]
     ]
     let states = [
     let states = [

+ 4 - 1
Tests/GRPCNIOTransportCoreTests/Server/GRPCServerStreamHandlerTests.swift

@@ -981,7 +981,10 @@ final class GRPCServerStreamHandlerTests: XCTestCase {
       )
       )
     ) { error in
     ) { error in
       XCTAssertEqual(error.code, .unavailable)
       XCTAssertEqual(error.code, .unavailable)
-      XCTAssertEqual(error.message, "Stream unexpectedly closed: a RST_STREAM frame was received.")
+      XCTAssertEqual(
+        error.message,
+        "Stream unexpectedly closed: received RST_STREAM frame (0x2: internal error)."
+      )
     }
     }
 
 
     // We should now be closed: check we can't write anymore.
     // We should now be closed: check we can't write anymore.