Browse Source

Merge pull request #228 from MrMage/simplify-request-handlers

Streamline the interface of the server-side response handling code
Tim Burks 7 years ago
parent
commit
2a1a067ad5

+ 6 - 8
Sources/Examples/Echo/EchoProvider.swift

@@ -26,7 +26,7 @@ class EchoProvider: Echo_EchoProvider {
   }
 
   // expand splits a request into words and returns each word in a separate message.
-  func expand(request: Echo_EchoRequest, session: Echo_EchoExpandSession) throws {
+  func expand(request: Echo_EchoRequest, session: Echo_EchoExpandSession) throws -> ServerStatus? {
     let parts = request.text.components(separatedBy: " ")
     for (i, part) in parts.enumerated() {
       var response = Echo_EchoResponse()
@@ -37,12 +37,11 @@ class EchoProvider: Echo_EchoProvider {
         }
       }
     }
-    session.waitForSendOperationsToFinish()
-    try session.close(withStatus: .ok, completion: nil)
+    return .ok
   }
 
   // collect collects a sequence of messages and returns them concatenated when the caller closes.
-  func collect(session: Echo_EchoCollectSession) throws {
+  func collect(session: Echo_EchoCollectSession) throws -> Echo_EchoResponse? {
     var parts: [String] = []
     while true {
       do {
@@ -56,11 +55,11 @@ class EchoProvider: Echo_EchoProvider {
     }
     var response = Echo_EchoResponse()
     response.text = "Swift echo collect: " + parts.joined(separator: " ")
-    try session.sendAndClose(response: response, status: .ok, completion: nil)
+    return response
   }
 
   // update streams back messages as they are received in an input stream.
-  func update(session: Echo_EchoUpdateSession) throws {
+  func update(session: Echo_EchoUpdateSession) throws -> ServerStatus? {
     var count = 0
     while true {
       do {
@@ -79,7 +78,6 @@ class EchoProvider: Echo_EchoProvider {
         break
       }
     }
-    session.waitForSendOperationsToFinish()
-    try session.close(withStatus: .ok, completion: nil)
+    return .ok
   }
 }

+ 19 - 17
Sources/Examples/Echo/Generated/echo.grpc.swift

@@ -211,11 +211,13 @@ class Echo_EchoServiceTestStub: ServiceClientTestStubBase, Echo_EchoService {
 }
 
 /// To build a server, implement a class that conforms to this protocol.
+/// If one of the methods returning `ServerStatus?` returns nil,
+/// it is expected that you have already returned a status to the client by means of `session.close`.
 internal protocol Echo_EchoProvider {
   func get(request: Echo_EchoRequest, session: Echo_EchoGetSession) throws -> Echo_EchoResponse
-  func expand(request: Echo_EchoRequest, session: Echo_EchoExpandSession) throws
-  func collect(session: Echo_EchoCollectSession) throws
-  func update(session: Echo_EchoUpdateSession) throws
+  func expand(request: Echo_EchoRequest, session: Echo_EchoExpandSession) throws -> ServerStatus?
+  func collect(session: Echo_EchoCollectSession) throws -> Echo_EchoResponse?
+  func update(session: Echo_EchoUpdateSession) throws -> ServerStatus?
 }
 
 internal protocol Echo_EchoGetSession: ServerSessionUnary {}
@@ -231,7 +233,8 @@ internal protocol Echo_EchoExpandSession: ServerSessionServerStreaming {
   func _send(_ message: Echo_EchoResponse, timeout: DispatchTime) throws
 
   /// Close the connection and send the status. Non-blocking.
-  /// You MUST call this method once you are done processing the request.
+  /// This method should be called if and only if your request handler returns a nil value instead of a server status;
+  /// otherwise SwiftGRPC will take care of sending the status for you.
   func close(withStatus status: ServerStatus, completion: (() -> Void)?) throws
 }
 
@@ -250,7 +253,8 @@ internal protocol Echo_EchoCollectSession: ServerSessionClientStreaming {
   /// Call this to wait for a result. Nonblocking.
   func receive(completion: @escaping (ResultOrRPCError<Echo_EchoRequest?>) -> Void) throws
 
-  /// You MUST call one of these two methods once you are done processing the request.
+  /// Exactly one of these two methods should be called if and only if your request handler returns nil;
+  /// otherwise SwiftGRPC will take care of sending the response and status for you.
   /// Close the connection and send a single result. Non-blocking.
   func sendAndClose(response: Echo_EchoResponse, status: ServerStatus, completion: (() -> Void)?) throws
   /// Close the connection and send an error. Non-blocking.
@@ -280,7 +284,8 @@ internal protocol Echo_EchoUpdateSession: ServerSessionBidirectionalStreaming {
   func _send(_ message: Echo_EchoResponse, timeout: DispatchTime) throws
 
   /// Close the connection and send the status. Non-blocking.
-  /// You MUST call this method once you are done processing the request.
+  /// This method should be called if and only if your request handler returns a nil value instead of a server status;
+  /// otherwise SwiftGRPC will take care of sending the status for you.
   func close(withStatus status: ServerStatus, completion: (() -> Void)?) throws
 }
 
@@ -318,36 +323,33 @@ internal final class Echo_EchoServer: ServiceServer {
     super.init(address: address, certificateString: certificateString, keyString: keyString)
   }
 
-  /// Start the server.
-  internal override func handleMethod(_ method: String, handler: Handler) throws -> Bool {
+  /// Determines and calls the appropriate request handler, depending on the request's method.
+  /// Throws `HandleMethodError.unknownMethod` for methods not handled by this service.
+  internal override func handleMethod(_ method: String, handler: Handler) throws -> ServerStatus? {
     let provider = self.provider
     switch method {
     case "/echo.Echo/Get":
-      try Echo_EchoGetSessionBase(
+      return try Echo_EchoGetSessionBase(
         handler: handler,
         providerBlock: { try provider.get(request: $0, session: $1 as! Echo_EchoGetSessionBase) })
           .run()
-      return true
     case "/echo.Echo/Expand":
-      try Echo_EchoExpandSessionBase(
+      return try Echo_EchoExpandSessionBase(
         handler: handler,
         providerBlock: { try provider.expand(request: $0, session: $1 as! Echo_EchoExpandSessionBase) })
           .run()
-      return true
     case "/echo.Echo/Collect":
-      try Echo_EchoCollectSessionBase(
+      return try Echo_EchoCollectSessionBase(
         handler: handler,
         providerBlock: { try provider.collect(session: $0 as! Echo_EchoCollectSessionBase) })
           .run()
-      return true
     case "/echo.Echo/Update":
-      try Echo_EchoUpdateSessionBase(
+      return try Echo_EchoUpdateSessionBase(
         handler: handler,
         providerBlock: { try provider.update(session: $0 as! Echo_EchoUpdateSessionBase) })
           .run()
-      return true
     default:
-      return false
+      throw HandleMethodError.unknownMethod
     }
   }
 }

+ 12 - 7
Sources/SwiftGRPC/Core/CompletionQueue.swift

@@ -67,7 +67,9 @@ class CompletionQueue {
   /// Mutex for synchronizing access to operationGroups
   private let operationGroupsMutex: Mutex = Mutex()
   
-  private var hasBeenShutdown = false
+  private var _hasBeenShutdown = false
+  
+  public var hasBeenShutdown: Bool { return operationGroupsMutex.synchronize { _hasBeenShutdown } }
 
   /// Initializes a CompletionQueue
   ///
@@ -80,7 +82,7 @@ class CompletionQueue {
   
   deinit {
     operationGroupsMutex.synchronize {
-      hasBeenShutdown = true
+      _hasBeenShutdown = true
     }
     cgrpc_completion_queue_shutdown(underlyingCompletionQueue)
     cgrpc_completion_queue_drain(underlyingCompletionQueue)
@@ -101,7 +103,7 @@ class CompletionQueue {
   /// - Parameter operationGroup: the operation group to handle.
   func register(_ operationGroup: OperationGroup, onSuccess: () throws -> Void) throws {
     try operationGroupsMutex.synchronize {
-      guard !hasBeenShutdown
+      guard !_hasBeenShutdown
         else { throw CallError.completionQueueShutdown }
       operationGroups[operationGroup.tag] = operationGroup
       try onSuccess()
@@ -113,7 +115,11 @@ class CompletionQueue {
   /// - Parameter completion: a completion handler that is called when the queue stops running
   func runToCompletion(completion: (() -> Void)?) {
     // run the completion queue on a new background thread
-    let spinloopThreadQueue = DispatchQueue(label: "SwiftGRPC.CompletionQueue.runToCompletion.spinloopThread")
+    var threadLabel = "SwiftGRPC.CompletionQueue.runToCompletion.spinloopThread"
+    if let name = self.name {
+      threadLabel.append(" (\(name))")
+    }
+    let spinloopThreadQueue = DispatchQueue(label: threadLabel)
     spinloopThreadQueue.async {
       spinloop: while true {
         let event = cgrpc_completion_queue_get_next_event(self.underlyingCompletionQueue, 600)
@@ -147,7 +153,6 @@ class CompletionQueue {
         case GRPC_QUEUE_TIMEOUT:
           continue spinloop
         default:
-          print("CompletionQueue.runToCompletion error: unknown event type \(event.type)")
           break spinloop
         }
       }
@@ -165,9 +170,9 @@ class CompletionQueue {
   func shutdown() {
     var needsShutdown = false
     operationGroupsMutex.synchronize {
-      if !hasBeenShutdown {
+      if !_hasBeenShutdown {
         needsShutdown = true
-        hasBeenShutdown = true
+        _hasBeenShutdown = true
       }
     }
     if needsShutdown {

+ 31 - 0
Sources/SwiftGRPC/Runtime/ServerSession.swift

@@ -57,6 +57,37 @@ open class ServerSessionBase: ServerSession {
   
   public func cancel() {
     call.cancel()
+    handler.shutdown()
+  }
+  
+  func sendInitialMetadataAndWait() throws {
+    let sendMetadataSignal = DispatchSemaphore(value: 0)
+    var success = false
+    try handler.sendMetadata(initialMetadata: initialMetadata) {
+      success = $0
+      sendMetadataSignal.signal()
+    }
+    sendMetadataSignal.wait()
+    
+    if !success {
+      throw ServerStatus.sendingInitialMetadataFailed
+    }
+  }
+  
+  func receiveRequestAndWait() throws -> Data {
+    let sendMetadataSignal = DispatchSemaphore(value: 0)
+    var requestData: Data?
+    try handler.receiveMessage(initialMetadata: initialMetadata) {
+      requestData = $0
+      sendMetadataSignal.signal()
+    }
+    sendMetadataSignal.wait()
+    
+    if let requestData = requestData {
+      return requestData
+    } else {
+      throw ServerStatus.noRequestData
+    }
   }
 }
 

+ 9 - 29
Sources/SwiftGRPC/Runtime/ServerSessionBidirectionalStreaming.swift

@@ -26,7 +26,7 @@ open class ServerSessionBidirectionalStreamingBase<InputType: Message, OutputTyp
   public typealias ReceivedType = InputType
   public typealias SentType = OutputType
   
-  public typealias ProviderBlock = (ServerSessionBidirectionalStreamingBase) throws -> Void
+  public typealias ProviderBlock = (ServerSessionBidirectionalStreamingBase) throws -> ServerStatus?
   private var providerBlock: ProviderBlock
 
   public init(handler: Handler, providerBlock: @escaping ProviderBlock) {
@@ -34,34 +34,14 @@ open class ServerSessionBidirectionalStreamingBase<InputType: Message, OutputTyp
     super.init(handler: handler)
   }
   
-  public func run() throws {
-    let sendMetadataSignal = DispatchSemaphore(value: 0)
-    var success = false
-    try handler.sendMetadata(initialMetadata: initialMetadata) {
-      success = $0
-      sendMetadataSignal.signal()
-    }
-    sendMetadataSignal.wait()
-    
-    var responseStatus: ServerStatus?
-    if success {
-      do {
-        try self.providerBlock(self)
-      } catch {
-        responseStatus = (error as? ServerStatus) ?? .processingError
-      }
-    } else {
-      print("ServerSessionBidirectionalStreamingBase.run sending initial metadata failed")
-      responseStatus = .sendingInitialMetadataFailed
-    }
-    
-    if let responseStatus = responseStatus {
-      // Error encountered, notify the client.
-      do {
-        try self.handler.sendStatus(responseStatus)
-      } catch {
-        print("ServerSessionBidirectionalStreamingBase.run error sending status: \(error)")
-      }
+  public func run() throws -> ServerStatus? {
+    try sendInitialMetadataAndWait()
+    do {
+      return try self.providerBlock(self)
+    } catch {
+      // Errors thrown by `providerBlock` should be logged in that method;
+      // we return the error as a status code to avoid `ServiceServer` logging this as a "really unexpected" error.
+      return (error as? ServerStatus) ?? .processingError
     }
   }
 }

+ 16 - 26
Sources/SwiftGRPC/Runtime/ServerSessionClientStreaming.swift

@@ -23,7 +23,7 @@ public protocol ServerSessionClientStreaming: ServerSession {}
 open class ServerSessionClientStreamingBase<InputType: Message, OutputType: Message>: ServerSessionBase, ServerSessionClientStreaming, StreamReceiving {
   public typealias ReceivedType = InputType
   
-  public typealias ProviderBlock = (ServerSessionClientStreamingBase) throws -> Void
+  public typealias ProviderBlock = (ServerSessionClientStreamingBase) throws -> OutputType?
   private var providerBlock: ProviderBlock
 
   public init(handler: Handler, providerBlock: @escaping ProviderBlock) {
@@ -40,35 +40,25 @@ open class ServerSessionClientStreamingBase<InputType: Message, OutputType: Mess
     try handler.sendStatus(status, completion: completion)
   }
   
-  public func run() throws {
-    let sendMetadataSignal = DispatchSemaphore(value: 0)
-    var success = false
-    try handler.sendMetadata(initialMetadata: initialMetadata) {
-      success = $0
-      sendMetadataSignal.signal()
-    }
-    sendMetadataSignal.wait()
+  public func run() throws -> ServerStatus? {
+    try sendInitialMetadataAndWait()
     
-    var responseStatus: ServerStatus?
-    if success {
-      do {
-        try self.providerBlock(self)
-      } catch {
-        responseStatus = (error as? ServerStatus) ?? .processingError
+    let responseMessage: OutputType
+    do {
+      guard let handlerResponse = try self.providerBlock(self) else {
+        // This indicates that the provider blocks has taken responsibility for sending a response and status, so do
+        // nothing.
+        return nil
       }
-    } else {
-      print("ServerSessionClientStreamingBase.run sending initial metadata failed")
-      responseStatus = .sendingInitialMetadataFailed
+      responseMessage = handlerResponse
+    } catch {
+      // Errors thrown by `providerBlock` should be logged in that method;
+      // we return the error as a status code to avoid `ServiceServer` logging this as a "really unexpected" error.
+      return (error as? ServerStatus) ?? .processingError
     }
     
-    if let responseStatus = responseStatus {
-      // Error encountered, notify the client.
-      do {
-        try self.handler.sendStatus(responseStatus)
-      } catch {
-        print("ServerSessionClientStreamingBase.run error sending status: \(error)")
-      }
-    }
+    try self.sendAndClose(response: responseMessage)
+    return nil  // The status will already be sent by `sendAndClose` above.
   }
 }
 

+ 10 - 30
Sources/SwiftGRPC/Runtime/ServerSessionServerStreaming.swift

@@ -25,7 +25,7 @@ public protocol ServerSessionServerStreaming: ServerSession {
 open class ServerSessionServerStreamingBase<InputType: Message, OutputType: Message>: ServerSessionBase, ServerSessionServerStreaming, StreamSending {
   public typealias SentType = OutputType
   
-  public typealias ProviderBlock = (InputType, ServerSessionServerStreamingBase) throws -> Void
+  public typealias ProviderBlock = (InputType, ServerSessionServerStreamingBase) throws -> ServerStatus?
   private var providerBlock: ProviderBlock
 
   public init(handler: Handler, providerBlock: @escaping ProviderBlock) {
@@ -33,35 +33,15 @@ open class ServerSessionServerStreamingBase<InputType: Message, OutputType: Mess
     super.init(handler: handler)
   }
   
-  public func run() throws {
-    let sendMetadataSignal = DispatchSemaphore(value: 0)
-    var requestData: Data?
-    try handler.receiveMessage(initialMetadata: initialMetadata) {
-      requestData = $0
-      sendMetadataSignal.signal()
-    }
-    sendMetadataSignal.wait()
-    
-    var responseStatus: ServerStatus?
-    if let requestData = requestData {
-      do {
-        let requestMessage = try InputType(serializedData: requestData)
-        try self.providerBlock(requestMessage, self)
-      } catch {
-        responseStatus = (error as? ServerStatus) ?? .processingError
-      }
-    } else {
-      print("ServerSessionServerStreamingBase.run no request data")
-      responseStatus = .noRequestData
-    }
-    
-    if let responseStatus = responseStatus {
-      // Error encountered, notify the client.
-      do {
-        try self.handler.sendStatus(responseStatus)
-      } catch {
-        print("ServerSessionServerStreamingBase.run error sending status: \(error)")
-      }
+  public func run() throws -> ServerStatus? {
+    let requestData = try receiveRequestAndWait()
+    let requestMessage = try InputType(serializedData: requestData)
+    do {
+      return try self.providerBlock(requestMessage, self)
+    } catch {
+      // Errors thrown by `providerBlock` should be logged in that method;
+      // we return the error as a status code to avoid `ServiceServer` logging this as a "really unexpected" error.
+      return (error as? ServerStatus) ?? .processingError
     }
   }
 }

+ 21 - 37
Sources/SwiftGRPC/Runtime/ServerSessionUnary.swift

@@ -31,47 +31,31 @@ open class ServerSessionUnaryBase<InputType: Message, OutputType: Message>: Serv
     super.init(handler: handler)
   }
   
-  public func run() throws {
-    let sendMetadataSignal = DispatchSemaphore(value: 0)
-    var requestData: Data?
-    try handler.receiveMessage(initialMetadata: initialMetadata) {
-      requestData = $0
-      sendMetadataSignal.signal()
-    }
-    sendMetadataSignal.wait()
-    
-    let responseStatus: ServerStatus
-    if let requestData = requestData {
-      do {
-        let requestMessage = try InputType(serializedData: requestData)
-        let responseMessage = try self.providerBlock(requestMessage, self)
-        
-        let sendResponseSignal = DispatchSemaphore(value: 0)
-        var sendResponseError: Error?
-        try self.handler.call.sendMessage(data: responseMessage.serializedData()) {
-          sendResponseError = $0
-          sendResponseSignal.signal()
-        }
-        sendResponseSignal.wait()
-        if let sendResponseError = sendResponseError {
-          print("ServerSessionUnaryBase.run error sending response: \(sendResponseError)")
-          throw sendResponseError
-        }
-        
-        responseStatus = .ok
-      } catch {
-        responseStatus = (error as? ServerStatus) ?? .processingError
-      }
-    } else {
-      print("ServerSessionUnaryBase.run no request data")
-      responseStatus = .noRequestData
-    }
+  public func run() throws -> ServerStatus? {
+    let requestData = try receiveRequestAndWait()
+    let requestMessage = try InputType(serializedData: requestData)
     
+    let responseMessage: OutputType
     do {
-      try self.handler.sendStatus(responseStatus)
+      responseMessage = try self.providerBlock(requestMessage, self)
     } catch {
-      print("ServerSessionUnaryBase.run error sending status: \(error)")
+      // Errors thrown by `providerBlock` should be logged in that method;
+      // we return the error as a status code to avoid `ServiceServer` logging this as a "really unexpected" error.
+      return (error as? ServerStatus) ?? .processingError
     }
+    
+    let sendResponseSignal = DispatchSemaphore(value: 0)
+    var sendResponseError: Error?
+    try self.handler.call.sendMessage(data: responseMessage.serializedData()) {
+      sendResponseError = $0
+      sendResponseSignal.signal()
+    }
+    sendResponseSignal.wait()
+    if let sendResponseError = sendResponseError {
+      throw sendResponseError
+    }
+    
+    return .ok
   }
 }
 

+ 28 - 6
Sources/SwiftGRPC/Runtime/ServiceServer.swift

@@ -48,9 +48,13 @@ open class ServiceServer {
     server = Server(address: address, key: key, certs: certificate)
   }
 
+  public enum HandleMethodError: Error {
+    case unknownMethod
+  }
+  
   /// Handle the given method. Needs to be overridden by actual implementations.
   /// Returns whether the method was actually handled.
-  open func handleMethod(_ method: String, handler: Handler) throws -> Bool { fatalError("needs to be overridden") }
+  open func handleMethod(_ method: String, handler: Handler) throws -> ServerStatus? { fatalError("needs to be overridden") }
 
   /// Start the server.
   public func start() {
@@ -71,8 +75,17 @@ open class ServiceServer {
       }
       
       do {
-        if !(try strongSelf.handleMethod(unwrappedMethod, handler: handler)) {
-          do {
+        do {
+          if let responseStatus = try strongSelf.handleMethod(unwrappedMethod, handler: handler),
+            !handler.completionQueue.hasBeenShutdown {
+            // The handler wants us to send the status for them; do that.
+            // But first, ensure that all outgoing messages have been enqueued, to avoid ending the stream prematurely:
+            handler.call.messageQueueEmpty.wait()
+            try handler.sendStatus(responseStatus)
+          }
+        } catch _ as HandleMethodError {
+          if !handler.completionQueue.hasBeenShutdown {
+            // The method is not implemented by the service - send a status saying so.
             try handler.call.perform(OperationGroup(
               call: handler.call,
               operations: [
@@ -82,12 +95,21 @@ open class ServiceServer {
             ]) { _ in
               handler.shutdown()
             })
-          } catch {
-            print("ServiceServer.start error sending status for unknown method: \(error)")
           }
         }
       } catch {
-        print("Server error: \(error)")
+        // The individual sessions' `run` methods (which are called by `self.handleMethod`) only throw errors if
+        // they encountered an error that has not also been "seen" by the actual request handler implementation.
+        // Therefore, this error is "really unexpected" and  should be logged here - there's nowhere else to log it otherwise.
+        print("ServiceServer unexpected error handling method '\(unwrappedMethod)': \(error)")
+        do {
+          if !handler.completionQueue.hasBeenShutdown {
+            try handler.sendStatus((error as? ServerStatus) ?? .processingError)
+          }
+        } catch {
+          print("ServiceServer unexpected error handling method '\(unwrappedMethod)'; sending status failed as well: \(error)")
+          handler.shutdown()
+        }
       }
     }
   }

+ 15 - 11
Sources/protoc-gen-swiftgrpc/Generator-Server.swift

@@ -41,6 +41,8 @@ extension Generator {
 
   private func printServerProtocol() {
     println("/// To build a server, implement a class that conforms to this protocol.")
+    println("/// If one of the methods returning `ServerStatus?` returns nil,")
+    println("/// it is expected that you have already returned a status to the client by means of `session.close`.")
     println("\(access) protocol \(providerName) {")
     indent()
     for method in service.methods {
@@ -49,11 +51,11 @@ extension Generator {
       case .unary:
         println("func \(methodFunctionName)(request: \(methodInputName), session: \(methodSessionName)) throws -> \(methodOutputName)")
       case .serverStreaming:
-        println("func \(methodFunctionName)(request: \(methodInputName), session: \(methodSessionName)) throws")
+        println("func \(methodFunctionName)(request: \(methodInputName), session: \(methodSessionName)) throws -> ServerStatus?")
       case .clientStreaming:
-        println("func \(methodFunctionName)(session: \(methodSessionName)) throws")
+        println("func \(methodFunctionName)(session: \(methodSessionName)) throws -> \(methodOutputName)?")
       case .bidirectionalStreaming:
-        println("func \(methodFunctionName)(session: \(methodSessionName)) throws")
+        println("func \(methodFunctionName)(session: \(methodSessionName)) throws -> ServerStatus?")
       }
     }
     outdent()
@@ -88,8 +90,9 @@ extension Generator {
     outdent()
     println("}")
     println()
-    println("/// Start the server.")
-    println("\(access) override func handleMethod(_ method: String, handler: Handler) throws -> Bool {")
+    println("/// Determines and calls the appropriate request handler, depending on the request's method.")
+    println("/// Throws `HandleMethodError.unknownMethod` for methods not handled by this service.")
+    println("\(access) override func handleMethod(_ method: String, handler: Handler) throws -> ServerStatus? {")
     indent()
     println("let provider = self.provider")
     println("switch method {")
@@ -99,7 +102,7 @@ extension Generator {
       indent()
       switch streamingType(method) {
       case .unary, .serverStreaming:
-        println("try \(methodSessionName)Base(")
+        println("return try \(methodSessionName)Base(")
         indent()
         println("handler: handler,")
         println("providerBlock: { try provider.\(methodFunctionName)(request: $0, session: $1 as! \(methodSessionName)Base) })")
@@ -108,7 +111,7 @@ extension Generator {
         outdent()
         outdent()
       default:
-        println("try \(methodSessionName)Base(")
+        println("return try \(methodSessionName)Base(")
         indent()
         println("handler: handler,")
         println("providerBlock: { try provider.\(methodFunctionName)(session: $0 as! \(methodSessionName)Base) })")
@@ -117,12 +120,11 @@ extension Generator {
         outdent()
         outdent()
       }
-      println("return true")
       outdent()
     }
     println("default:")
     indent()
-    println("return false")
+    println("throw HandleMethodError.unknownMethod")
     outdent()
     println("}")
     outdent()
@@ -143,7 +145,8 @@ extension Generator {
   }
   
   private func printServerMethodSendAndClose(sentType: String) {
-    println("/// You MUST call one of these two methods once you are done processing the request.")
+    println("/// Exactly one of these two methods should be called if and only if your request handler returns nil;")
+    println("/// otherwise SwiftGRPC will take care of sending the response and status for you.")
     println("/// Close the connection and send a single result. Non-blocking.")
     println("func sendAndClose(response: \(sentType), status: ServerStatus, completion: (() -> Void)?) throws")
     println("/// Close the connection and send an error. Non-blocking.")
@@ -172,7 +175,8 @@ extension Generator {
 
   private func printServerMethodClose() {
     println("/// Close the connection and send the status. Non-blocking.")
-    println("/// You MUST call this method once you are done processing the request.")
+    println("/// This method should be called if and only if your request handler returns a nil value instead of a server status;")
+    println("/// otherwise SwiftGRPC will take care of sending the status for you.")
     println("func close(withStatus status: ServerStatus, completion: (() -> Void)?) throws")
   }
   

+ 3 - 3
Tests/SwiftGRPCTests/ChannelArgumentTests.swift

@@ -26,15 +26,15 @@ fileprivate class ChannelArgumentTestProvider: Echo_EchoProvider {
     return Echo_EchoResponse(text: (session as! ServerSessionBase).handler.requestMetadata["user-agent"]!)
   }
   
-  func expand(request: Echo_EchoRequest, session: Echo_EchoExpandSession) throws {
+  func expand(request: Echo_EchoRequest, session: Echo_EchoExpandSession) throws -> ServerStatus? {
     fatalError("not implemented")
   }
   
-  func collect(session: Echo_EchoCollectSession) throws {
+  func collect(session: Echo_EchoCollectSession) throws -> Echo_EchoResponse? {
     fatalError("not implemented")
   }
   
-  func update(session: Echo_EchoUpdateSession) throws {
+  func update(session: Echo_EchoUpdateSession) throws -> ServerStatus? {
     fatalError("not implemented")
   }
 }

+ 4 - 3
Tests/SwiftGRPCTests/CompletionQueueTests.swift

@@ -25,18 +25,19 @@ fileprivate class ClosingProvider: Echo_EchoProvider {
     return Echo_EchoResponse()
   }
   
-  func expand(request: Echo_EchoRequest, session: Echo_EchoExpandSession) throws {
+  func expand(request: Echo_EchoRequest, session: Echo_EchoExpandSession) throws -> ServerStatus? {
     let closeSem = DispatchSemaphore(value: 0)
     try! session.close(withStatus: .ok) {
       closeSem.signal()
     }
     XCTAssertThrowsError(try session.send(Echo_EchoResponse()))
     doneExpectation.fulfill()
+    return nil
   }
   
-  func collect(session: Echo_EchoCollectSession) throws { }
+  func collect(session: Echo_EchoCollectSession) throws -> Echo_EchoResponse? { fatalError("not implemented") }
   
-  func update(session: Echo_EchoUpdateSession) throws { }
+  func update(session: Echo_EchoUpdateSession) throws -> ServerStatus? { fatalError("not implemented") }
 }
 
 class CompletionQueueTests: BasicEchoTestCase {

+ 6 - 4
Tests/SwiftGRPCTests/ServerCancellingTests.swift

@@ -24,19 +24,21 @@ fileprivate class CancellingProvider: Echo_EchoProvider {
     return Echo_EchoResponse()
   }
   
-  func expand(request: Echo_EchoRequest, session: Echo_EchoExpandSession) throws {
+  func expand(request: Echo_EchoRequest, session: Echo_EchoExpandSession) throws -> ServerStatus? {
     session.cancel()
     XCTAssertThrowsError(try session.send(Echo_EchoResponse()))
+    return nil
   }
   
-  func collect(session: Echo_EchoCollectSession) throws {
+  func collect(session: Echo_EchoCollectSession) throws -> Echo_EchoResponse? {
     session.cancel()
-    try! session.sendAndClose(response: Echo_EchoResponse(), status: .ok, completion: nil)
+    return Echo_EchoResponse()
   }
   
-  func update(session: Echo_EchoUpdateSession) throws {
+  func update(session: Echo_EchoUpdateSession) throws -> ServerStatus? {
     session.cancel()
     XCTAssertThrowsError(try session.send(Echo_EchoResponse()))
+    return nil
   }
 }
 

+ 3 - 9
Tests/SwiftGRPCTests/ServerTestExample.swift

@@ -61,18 +61,13 @@ extension ServerTestExample {
     let session = Echo_EchoCollectSessionTestStub()
     session.inputs = ["foo", "bar", "baz"].map { Echo_EchoRequest(text: $0) }
     
-    XCTAssertNoThrow(try provider.collect(session: session))
-    
-    XCTAssertEqual(.ok, session.status!.code)
-    XCTAssertEqual(Echo_EchoResponse(text: "Swift echo collect: foo bar baz"),
-                   session.output)
+    XCTAssertEqual(Echo_EchoResponse(text: "Swift echo collect: foo bar baz"), try provider.collect(session: session)!)
   }
   
   func testServerStreaming() {
     let session = Echo_EchoExpandSessionTestStub()
-    XCTAssertNoThrow(try provider.expand(request: Echo_EchoRequest(text: "foo bar baz"), session: session))
+    XCTAssertEqual(.ok, try provider.expand(request: Echo_EchoRequest(text: "foo bar baz"), session: session)!.code)
     
-    XCTAssertEqual(.ok, session.status!.code)
     XCTAssertEqual(["foo", "bar", "baz"].enumerated()
       .map { Echo_EchoResponse(text: "Swift echo expand (\($0)): \($1)") },
                    session.outputs)
@@ -82,9 +77,8 @@ extension ServerTestExample {
     let inputStrings = ["foo", "bar", "baz"]
     let session = Echo_EchoUpdateSessionTestStub()
     session.inputs = inputStrings.map { Echo_EchoRequest(text: $0) }
-    XCTAssertNoThrow(try provider.update(session: session))
+    XCTAssertEqual(.ok, try! provider.update(session: session)!.code)
     
-    XCTAssertEqual(.ok, session.status!.code)
     XCTAssertEqual(inputStrings.enumerated()
       .map { Echo_EchoResponse(text: "Swift echo update (\($0)): \($1)") },
                    session.outputs)

+ 3 - 3
Tests/SwiftGRPCTests/ServerThrowingTests.swift

@@ -25,15 +25,15 @@ fileprivate class StatusThrowingProvider: Echo_EchoProvider {
     throw testStatus
   }
   
-  func expand(request: Echo_EchoRequest, session: Echo_EchoExpandSession) throws {
+  func expand(request: Echo_EchoRequest, session: Echo_EchoExpandSession) throws -> ServerStatus? {
     throw testStatus
   }
   
-  func collect(session: Echo_EchoCollectSession) throws {
+  func collect(session: Echo_EchoCollectSession) throws -> Echo_EchoResponse? {
     throw testStatus
   }
   
-  func update(session: Echo_EchoUpdateSession) throws {
+  func update(session: Echo_EchoUpdateSession) throws -> ServerStatus? {
     throw testStatus
   }
 }

+ 6 - 3
Tests/SwiftGRPCTests/ServerTimeoutTests.swift

@@ -24,16 +24,19 @@ fileprivate class TimingOutEchoProvider: Echo_EchoProvider {
     return Echo_EchoResponse()
   }
   
-  func expand(request: Echo_EchoRequest, session: Echo_EchoExpandSession) throws {
+  func expand(request: Echo_EchoRequest, session: Echo_EchoExpandSession) throws -> ServerStatus? {
     Thread.sleep(forTimeInterval: 0.2)
+    return .ok
   }
   
-  func collect(session: Echo_EchoCollectSession) throws {
+  func collect(session: Echo_EchoCollectSession) throws -> Echo_EchoResponse? {
     Thread.sleep(forTimeInterval: 0.2)
+    return Echo_EchoResponse()
   }
   
-  func update(session: Echo_EchoUpdateSession) throws {
+  func update(session: Echo_EchoUpdateSession) throws -> ServerStatus? {
     Thread.sleep(forTimeInterval: 0.2)
+    return .ok
   }
 }
 

+ 10 - 5
Tests/SwiftGRPCTests/ServiceClientTests.swift

@@ -18,25 +18,30 @@ import Foundation
 import XCTest
 
 class ServiceClientTests: BasicEchoTestCase {
-  private lazy var sharedChannel = Channel(address: address, secure: false)
+  private var sharedChannel: Channel?
 
   override func setUp() {
     super.setUp()
     sharedChannel = Channel(address: address, secure: false)
   }
+  
+  override func tearDown() {
+    sharedChannel = nil
+    super.tearDown()
+  }
 
   func testSharingChannelBetweenClientsUnaryAsync() {
     let firstCallExpectation = expectation(description: "First call completes successfully")
     let secondCallExpectation = expectation(description: "Second call completes successfully")
 
     do {
-      let client1 = Echo_EchoServiceClient(channel: sharedChannel)
+      let client1 = Echo_EchoServiceClient(channel: sharedChannel!)
       try _ = client1.get(Echo_EchoRequest(text: "foo")) { _, callResult in
         XCTAssertEqual(.ok, callResult.statusCode)
         firstCallExpectation.fulfill()
       }
 
-      let client2 = Echo_EchoServiceClient(channel: sharedChannel)
+      let client2 = Echo_EchoServiceClient(channel: sharedChannel!)
       try _ = client2.get(Echo_EchoRequest(text: "foo")) { _, callResult in
         XCTAssertEqual(.ok, callResult.statusCode)
         secondCallExpectation.fulfill()
@@ -50,7 +55,7 @@ class ServiceClientTests: BasicEchoTestCase {
 
   func testSharedChannelStillWorksAfterFirstUnaryClientCompletes() {
     do {
-      let client1 = Echo_EchoServiceClient(channel: sharedChannel)
+      let client1 = Echo_EchoServiceClient(channel: sharedChannel!)
       let response1 = try client1.get(Echo_EchoRequest(text: "foo")).text
       XCTAssertEqual("Swift echo get: foo", response1)
     } catch let error {
@@ -58,7 +63,7 @@ class ServiceClientTests: BasicEchoTestCase {
     }
 
     do {
-      let client2 = Echo_EchoServiceClient(channel: sharedChannel)
+      let client2 = Echo_EchoServiceClient(channel: sharedChannel!)
       let response2 = try client2.get(Echo_EchoRequest(text: "foo")).text
       XCTAssertEqual("Swift echo get: foo", response2)
     } catch let error {