Better map HTTP status codes to GRPC status codes (#460)

Some HTTP status codes map to specific gRPC status codes as per:
https://github.com/grpc/grpc/blob/master/doc/http-grpc-status-mapping.md

Sources/SwiftGRPCNIO/GRPCError.swift

@@ -165,7 +165,7 @@ extension GRPCClientError: GRPCStatusTransformable {
   public func asGRPCStatus() -> GRPCStatus {
     switch self {
     case .HTTPStatusNotOk(let status):
-      return GRPCStatus(code: .unknown, message: "server returned \(status.code) \(status.reasonPhrase)")
+      return GRPCStatus(code: status.grpcStatusCode, message: "\(status.code): \(status.reasonPhrase)")
 
     case .cancelledByClient:
       return GRPCStatus(code: .cancelled, message: "client cancelled the call")
@@ -202,3 +202,25 @@ extension GRPCCommonError: GRPCStatusTransformable {
     }
   }
 }
+
+extension HTTPResponseStatus {
+  /// The gRPC status code associated with the HTTP status code.
+  ///
+  /// See: https://github.com/grpc/grpc/blob/master/doc/http-grpc-status-mapping.md
+  internal var grpcStatusCode: StatusCode {
+    switch self {
+      case .badRequest:
+        return .internalError
+      case .unauthorized:
+        return .unauthenticated
+      case .forbidden:
+        return .permissionDenied
+      case .notFound:
+        return .unimplemented
+      case .tooManyRequests, .badGateway, .serviceUnavailable, .gatewayTimeout:
+        return .unavailable
+      default:
+        return .unknown
+    }
+  }
+}

Sources/SwiftGRPCNIO/HTTP1ToRawGRPCClientCodec.swift

@@ -89,16 +89,21 @@ extension HTTP1ToRawGRPCClientCodec: ChannelInboundHandler {
       throw GRPCError.client(.invalidState("received headers while in state \(state)"))
     }
 
-    guard head.status == .ok else {
-      throw GRPCError.client(.HTTPStatusNotOk(head.status))
-    }
-
     // Trailers-Only response.
     if head.headers.contains(name: GRPCHeaderName.statusCode) {
       self.state = .expectingBodyOrTrailers
       return try self.processTrailers(context: context, trailers: head.headers)
     }
 
+    // This should be checked *after* the trailers-only response is handled since any status code
+    // and message we already have should take precedence over one we generate from the HTTP status
+    // code and reason.
+    //
+    // Source: https://github.com/grpc/grpc/blob/master/doc/http-grpc-status-mapping.md
+    guard head.status == .ok else {
+      throw GRPCError.client(.HTTPStatusNotOk(head.status))
+    }
+
     let inboundCompression: CompressionMechanism = head.headers[GRPCHeaderName.encoding]
       .first
       .map { CompressionMechanism(rawValue: $0) ?? .unknown } ?? .none

Tests/SwiftGRPCNIOTests/GRPCStatusCodeTests.swift

@@ -0,0 +1,112 @@
+/*
+ * Copyright 2019, gRPC Authors All rights reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+import Foundation
+@testable import SwiftGRPCNIO
+import NIO
+import NIOHTTP1
+import NIOHTTP2
+import XCTest
+
+class GRPCStatusCodeTests: XCTestCase {
+  var channel: EmbeddedChannel!
+  var metadataPromise: EventLoopPromise<HTTPHeaders>!
+  var responsePromise: EventLoopPromise<Echo_EchoResponse>!
+  var statusPromise: EventLoopPromise<GRPCStatus>!
+
+  override func setUp() {
+    super.setUp()
+
+    self.channel = EmbeddedChannel()
+    self.metadataPromise = self.channel.eventLoop.makePromise()
+    self.responsePromise = self.channel.eventLoop.makePromise()
+    self.statusPromise = self.channel.eventLoop.makePromise()
+
+    try! self.channel.pipeline.addHandlers([
+      HTTP1ToRawGRPCClientCodec(),
+      GRPCClientCodec<Echo_EchoRequest, Echo_EchoResponse>(),
+      GRPCClientChannelHandler<Echo_EchoRequest, Echo_EchoResponse>(
+        initialMetadataPromise: self.metadataPromise,
+        statusPromise: self.statusPromise,
+        responseObserver: .succeedPromise(self.responsePromise),
+        errorDelegate: nil)
+    ]).wait()
+  }
+
+  override func tearDown() {
+    self.metadataPromise.fail(GRPCError.client(.cancelledByClient))
+    self.responsePromise.fail(GRPCError.client(.cancelledByClient))
+    self.statusPromise.fail(GRPCError.client(.cancelledByClient))
+  }
+
+  func responseHead(code: HTTPResponseStatus, headers: HTTPHeaders = HTTPHeaders()) -> HTTPClientResponsePart {
+    return .head(HTTPResponseHead(version: HTTPVersion(major: 2, minor: 0), status: code, headers: headers))
+  }
+
+  func testTooManyRequests() throws {
+    XCTAssertNoThrow(try self.channel.writeInbound(self.responseHead(code: .tooManyRequests)))
+    XCTAssertEqual(try statusPromise.futureResult.map { $0.code }.wait(), .unavailable)
+  }
+
+  func testBadGateway() throws {
+    XCTAssertNoThrow(try self.channel.writeInbound(self.responseHead(code: .badGateway)))
+    XCTAssertEqual(try statusPromise.futureResult.map { $0.code }.wait(), .unavailable)
+  }
+
+  func testServiceUnavailable() throws {
+    XCTAssertNoThrow(try self.channel.writeInbound(self.responseHead(code: .serviceUnavailable)))
+    XCTAssertEqual(try statusPromise.futureResult.map { $0.code }.wait(), .unavailable)
+  }
+
+  func testGatewayTimeout() throws {
+    XCTAssertNoThrow(try self.channel.writeInbound(self.responseHead(code: .gatewayTimeout)))
+    XCTAssertEqual(try statusPromise.futureResult.map { $0.code }.wait(), .unavailable)
+  }
+
+  func testBadRequest() throws {
+    XCTAssertNoThrow(try self.channel.writeInbound(self.responseHead(code: .badRequest)))
+    XCTAssertEqual(try statusPromise.futureResult.map { $0.code }.wait(), .internalError)
+  }
+
+  func testUnauthorized() throws {
+    XCTAssertNoThrow(try self.channel.writeInbound(self.responseHead(code: .unauthorized)))
+    XCTAssertEqual(try statusPromise.futureResult.map { $0.code }.wait(), .unauthenticated)
+  }
+
+  func testForbidden() throws {
+    XCTAssertNoThrow(try self.channel.writeInbound(self.responseHead(code: .forbidden)))
+    XCTAssertEqual(try statusPromise.futureResult.map { $0.code }.wait(), .permissionDenied)
+  }
+
+  func testNotFound() throws {
+    XCTAssertNoThrow(try self.channel.writeInbound(self.responseHead(code: .notFound)))
+    XCTAssertEqual(try statusPromise.futureResult.map { $0.code }.wait(), .unimplemented)
+  }
+
+  func testStatusCodeAndMessageAreRespectedForNon200Responses() throws {
+    let statusCode: StatusCode = .doNotUse
+    let statusMessage = "Not the HTTP error phrase"
+
+    var headers = HTTPHeaders()
+    headers.add(name: GRPCHeaderName.statusCode, value: "\(statusCode.rawValue)")
+    headers.add(name: GRPCHeaderName.statusMessage, value: statusMessage)
+
+    XCTAssertNoThrow(try self.channel.writeInbound(self.responseHead(code: .imATeapot, headers: headers)))
+    let status = try statusPromise.futureResult.wait()
+
+    XCTAssertEqual(status.code, statusCode)
+    XCTAssertEqual(status.message, statusMessage)
+  }
+}

Tests/SwiftGRPCNIOTests/XCTestManifests.swift

@@ -70,6 +70,23 @@ extension GRPCSecureInteroperabilityTests {
     ]
 }
 
+extension GRPCStatusCodeTests {
+    // DO NOT MODIFY: This is autogenerated, use:
+    //   `swift test --generate-linuxmain`
+    // to regenerate.
+    static let __allTests__GRPCStatusCodeTests = [
+        ("testBadGateway", testBadGateway),
+        ("testBadRequest", testBadRequest),
+        ("testForbidden", testForbidden),
+        ("testGatewayTimeout", testGatewayTimeout),
+        ("testNotFound", testNotFound),
+        ("testServiceUnavailable", testServiceUnavailable),
+        ("testStatusCodeAndMessageAreRespectedForNon200Responses", testStatusCodeAndMessageAreRespectedForNon200Responses),
+        ("testTooManyRequests", testTooManyRequests),
+        ("testUnauthorized", testUnauthorized),
+    ]
+}
+
 extension GRPCStatusMessageMarshallerTests {
     // DO NOT MODIFY: This is autogenerated, use:
     //   `swift test --generate-linuxmain`
@@ -281,6 +298,7 @@ public func __allTests() -> [XCTestCaseEntry] {
         testCase(GRPCChannelHandlerTests.__allTests__GRPCChannelHandlerTests),
         testCase(GRPCInsecureInteroperabilityTests.__allTests__GRPCInsecureInteroperabilityTests),
         testCase(GRPCSecureInteroperabilityTests.__allTests__GRPCSecureInteroperabilityTests),
+        testCase(GRPCStatusCodeTests.__allTests__GRPCStatusCodeTests),
         testCase(GRPCStatusMessageMarshallerTests.__allTests__GRPCStatusMessageMarshallerTests),
         testCase(HTTP1ToRawGRPCServerCodecTests.__allTests__HTTP1ToRawGRPCServerCodecTests),
         testCase(LengthPrefixedMessageReaderTests.__allTests__LengthPrefixedMessageReaderTests),