Skip to content

Commit ca0d255

Browse files
MrMageMrMage
committed
Ensure that errors in client calls are always provided to the user.
This includes: - Make most completion handlers non-throwing (their errors were for the most part discarded, anyway) - Extract Receive-Streaming into a common protocol - Changing many completion handler interfaces to be called with a `CallResult` instead of `Data?` and `ResultOrRPCError<ReceivedType?>` instead of `(ReceivedType, Error)` - Not throwing on end of stream, but passing `nil` to the completion handler instead - Making `ServerSessionUnary` always return an error when one of its calls fail - Adding test to ensure the proper error behavior.
1 parent 263e66b commit ca0d255

24 files changed

+434
-340
lines changed

‎Sources/Examples/Echo/EchoProvider.swift

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -45,10 +45,9 @@ class EchoProvider: Echo_EchoProvider {
4545
var parts: [String] = []
4646
while true {
4747
do {
48-
let request = try session.receive()
48+
guard let request = try session.receive()
49+
else { break } // End of stream
4950
parts.append(request.text)
50-
} catch ServerError.endOfStream {
51-
break
5251
} catch (let error) {
5352
print("\(error)")
5453
}
@@ -63,7 +62,8 @@ class EchoProvider: Echo_EchoProvider {
6362
var count = 0
6463
while true {
6564
do {
66-
let request = try session.receive()
65+
guard let request = try session.receive()
66+
else { break } // End of stream
6767
var response = Echo_EchoResponse()
6868
response.text = "Swift echo update (\(count)): \(request.text)"
6969
count += 1
@@ -72,8 +72,6 @@ class EchoProvider: Echo_EchoProvider {
7272
print("update error: \(error)")
7373
}
7474
}
75-
} catch ServerError.endOfStream {
76-
break
7775
} catch (let error) {
7876
print("\(error)")
7977
break

‎Sources/Examples/Echo/Generated/echo.grpc.swift

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,9 @@ fileprivate final class Echo_EchoGetCallBase: ClientCallUnaryBase<Echo_EchoReque
3333

3434
internal protocol Echo_EchoExpandCall: ClientCallServerStreaming {
3535
/// Call this to wait for a result. Blocking.
36-
func receive() throws -> Echo_EchoResponse
36+
func receive() throws -> Echo_EchoResponse?
3737
/// Call this to wait for a result. Nonblocking.
38-
func receive(completion: @escaping (Echo_EchoResponse?, ClientError?) -> Void) throws
38+
func receive(completion: @escaping (ResultOrRPCError<Echo_EchoResponse?>) -> Void) throws
3939
}
4040

4141
fileprivate final class Echo_EchoExpandCallBase: ClientCallServerStreamingBase<Echo_EchoRequest, Echo_EchoResponse>, Echo_EchoExpandCall {
@@ -53,7 +53,7 @@ internal protocol Echo_EchoCollectCall: ClientCallClientStreaming {
5353
/// Call this to close the connection and wait for a response. Blocking.
5454
func closeAndReceive() throws -> Echo_EchoResponse
5555
/// Call this to close the connection and wait for a response. Nonblocking.
56-
func closeAndReceive(completion: @escaping (Echo_EchoResponse?, ClientError?) -> Void) throws
56+
func closeAndReceive(completion: @escaping (ResultOrRPCError<Echo_EchoResponse>) -> Void) throws
5757
}
5858

5959
fileprivate final class Echo_EchoCollectCallBase: ClientCallClientStreamingBase<Echo_EchoRequest, Echo_EchoResponse>, Echo_EchoCollectCall {
@@ -68,9 +68,9 @@ class Echo_EchoCollectCallTestStub: ClientCallClientStreamingTestStub<Echo_EchoR
6868

6969
internal protocol Echo_EchoUpdateCall: ClientCallBidirectionalStreaming {
7070
/// Call this to wait for a result. Blocking.
71-
func receive() throws -> Echo_EchoResponse
71+
func receive() throws -> Echo_EchoResponse?
7272
/// Call this to wait for a result. Nonblocking.
73-
func receive(completion: @escaping (Echo_EchoResponse?, ClientError?) -> Void) throws
73+
func receive(completion: @escaping (ResultOrRPCError<Echo_EchoResponse?>) -> Void) throws
7474

7575
/// Call this to send each message in the request stream.
7676
func send(_ message: Echo_EchoRequest, completion: @escaping (Error?) -> Void) throws
@@ -210,8 +210,10 @@ fileprivate final class Echo_EchoExpandSessionBase: ServerSessionServerStreaming
210210
class Echo_EchoExpandSessionTestStub: ServerSessionServerStreamingTestStub<Echo_EchoResponse>, Echo_EchoExpandSession {}
211211

212212
internal protocol Echo_EchoCollectSession: ServerSessionClientStreaming {
213-
/// Receive a message. Blocks until a message is received or the client closes the connection.
214-
func receive() throws -> Echo_EchoRequest
213+
/// Call this to wait for a result. Blocking.
214+
func receive() throws -> Echo_EchoRequest?
215+
/// Call this to wait for a result. Nonblocking.
216+
func receive(completion: @escaping (ResultOrRPCError<Echo_EchoRequest?>) -> Void) throws
215217

216218
/// Send a response and close the connection.
217219
func sendAndClose(_ response: Echo_EchoResponse) throws
@@ -222,8 +224,10 @@ fileprivate final class Echo_EchoCollectSessionBase: ServerSessionClientStreamin
222224
class Echo_EchoCollectSessionTestStub: ServerSessionClientStreamingTestStub<Echo_EchoRequest, Echo_EchoResponse>, Echo_EchoCollectSession {}
223225

224226
internal protocol Echo_EchoUpdateSession: ServerSessionBidirectionalStreaming {
225-
/// Receive a message. Blocks until a message is received or the client closes the connection.
226-
func receive() throws -> Echo_EchoRequest
227+
/// Call this to wait for a result. Blocking.
228+
func receive() throws -> Echo_EchoRequest?
229+
/// Call this to wait for a result. Nonblocking.
230+
func receive(completion: @escaping (ResultOrRPCError<Echo_EchoRequest?>) -> Void) throws
227231

228232
/// Send a message. Nonblocking.
229233
func send(_ response: Echo_EchoResponse, completion: ((Error?) -> Void)?) throws

‎Sources/Examples/Echo/main.swift

Lines changed: 9 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -109,16 +109,13 @@ Group {
109109
callResult = result
110110
sem.signal()
111111
}
112-
var running = true
113-
while running {
114-
do {
115-
let responseMessage = try expandCall.receive()
116-
print("expand received: \(responseMessage.text)")
117-
} catch ClientError.endOfStream {
118-
running = false
119-
}
112+
while true {
113+
guard let responseMessage = try expandCall.receive()
114+
else { break } // End of stream
115+
print("expand received: \(responseMessage.text)")
120116
}
121117
_ = sem.wait()
118+
122119
if let statusCode = callResult?.statusCode {
123120
print("expand completed with code \(statusCode)")
124121
}
@@ -150,6 +147,7 @@ Group {
150147
let responseMessage = try collectCall.closeAndReceive()
151148
print("collect received: \(responseMessage.text)")
152149
_ = sem.wait()
150+
153151
if let statusCode = callResult?.statusCode {
154152
print("collect completed with code \(statusCode)")
155153
}
@@ -182,17 +180,10 @@ Group {
182180
try updateCall.closeSend()
183181

184182
while true {
185-
do {
186-
let responseMessage = try updateCall.receive()
187-
print("update received: \(responseMessage.text)")
188-
} catch ClientError.endOfStream {
189-
break
190-
} catch (let error) {
191-
print("update receive error: \(error)")
192-
break
193-
}
183+
guard let responseMessage = try updateCall.receive()
184+
else { break } // End of stream
185+
print("update received: \(responseMessage.text)")
194186
}
195-
196187
_ = sem.wait()
197188

198189
if let statusCode = callResult?.statusCode {

‎Sources/SwiftGRPC/Core/Call.swift

Lines changed: 25 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -86,34 +86,25 @@ public enum CallError: Error {
8686
}
8787

8888
public struct CallResult: CustomStringConvertible {
89+
public let success: Bool
8990
public let statusCode: StatusCode
9091
public let statusMessage: String?
9192
public let resultData: Data?
9293
public let initialMetadata: Metadata?
9394
public let trailingMetadata: Metadata?
9495

9596
fileprivate init(_ op: OperationGroup) {
96-
if op.success {
97-
if let statusCodeRawValue = op.receivedStatusCode() {
98-
if let statusCode = StatusCode(rawValue: statusCodeRawValue) {
99-
self.statusCode = statusCode
100-
} else {
101-
statusCode = .unknown
102-
}
103-
} else {
104-
statusCode = .ok
105-
}
106-
statusMessage = op.receivedStatusMessage()
107-
resultData = op.receivedMessage()?.data()
108-
initialMetadata = op.receivedInitialMetadata()
109-
trailingMetadata = op.receivedTrailingMetadata()
97+
success = op.success
98+
if let statusCodeRawValue = op.receivedStatusCode(),
99+
let statusCode = StatusCode(rawValue: statusCodeRawValue) {
100+
self.statusCode = statusCode
110101
} else {
111102
statusCode = .unknown
112-
statusMessage = nil
113-
resultData = nil
114-
initialMetadata = nil
115-
trailingMetadata = nil
116103
}
104+
statusMessage = op.receivedStatusMessage()
105+
resultData = op.receivedMessage()?.data()
106+
initialMetadata = op.receivedInitialMetadata()
107+
trailingMetadata = op.receivedTrailingMetadata()
117108
}
118109

119110
public var description: String {
@@ -240,11 +231,18 @@ public class Call {
240231
.receiveStatusOnClient,
241232
]
242233
case .clientStreaming, .bidiStreaming:
243-
operations = [
244-
.sendInitialMetadata(metadata.copy()),
245-
.receiveInitialMetadata,
246-
.receiveStatusOnClient,
247-
]
234+
try perform(OperationGroup(call: self,
235+
operations: [
236+
.sendInitialMetadata(metadata.copy()),
237+
.receiveInitialMetadata
238+
],
239+
completion: nil))
240+
try perform(OperationGroup(call: self,
241+
operations: [.receiveStatusOnClient],
242+
completion: completion != nil
243+
? { op in completion?(CallResult(op)) }
244+
: nil))
245+
return
248246
}
249247
try perform(OperationGroup(call: self,
250248
operations: operations,
@@ -304,27 +302,17 @@ public class Call {
304302

305303
// Receive a message over a streaming connection.
306304
/// - Throws: `CallError` if fails to call.
307-
public func closeAndReceiveMessage(completion: @escaping (Data?) throws -> Void) throws {
305+
public func closeAndReceiveMessage(completion: @escaping (CallResult) -> Void) throws {
308306
try perform(OperationGroup(call: self, operations: [.sendCloseFromClient, .receiveMessage]) { operationGroup in
309-
if operationGroup.success {
310-
if let messageBuffer = operationGroup.receivedMessage() {
311-
try completion(messageBuffer.data())
312-
} else {
313-
try completion(nil) // an empty response signals the end of a connection
314-
}
315-
}
307+
completion(CallResult(operationGroup))
316308
})
317309
}
318310

319311
// Receive a message over a streaming connection.
320312
/// - Throws: `CallError` if fails to call.
321-
public func receiveMessage(completion: @escaping (Data?) throws -> Void) throws {
313+
public func receiveMessage(completion: @escaping (CallResult) -> Void) throws {
322314
try perform(OperationGroup(call: self, operations: [.receiveMessage]) { operationGroup in
323-
if operationGroup.success {
324-
try completion(operationGroup.receivedMessage()?.data())
325-
} else {
326-
try completion(nil)
327-
}
315+
completion(CallResult(operationGroup))
328316
})
329317
}
330318

‎Sources/SwiftGRPC/Core/CompletionQueue.swift

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -113,26 +113,18 @@ class CompletionQueue {
113113
self.operationGroupsMutex.unlock()
114114
if let operationGroup = operationGroup {
115115
// call the operation group completion handler
116-
do {
117-
operationGroup.success = (event.success == 1)
118-
try operationGroup.completion?(operationGroup)
119-
} catch (let callError) {
120-
print("CompletionQueue runToCompletion: grpc error \(callError)")
121-
}
116+
operationGroup.success = (event.success == 1)
117+
operationGroup.completion?(operationGroup)
122118
self.operationGroupsMutex.synchronize {
123119
self.operationGroups[tag] = nil
124120
}
125121
}
126122
break
127123
case GRPC_QUEUE_SHUTDOWN:
128124
running = false
129-
do {
130-
for operationGroup in self.operationGroups.values {
131-
operationGroup.success = false
132-
try operationGroup.completion?(operationGroup)
133-
}
134-
} catch (let callError) {
135-
print("CompletionQueue runToCompletion: grpc error \(callError)")
125+
for operationGroup in self.operationGroups.values {
126+
operationGroup.success = false
127+
operationGroup.completion?(operationGroup)
136128
}
137129
self.operationGroupsMutex.synchronize {
138130
self.operationGroups = [:]

‎Sources/SwiftGRPC/Core/Handler.swift

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ public class Handler {
3030
public let requestMetadata: Metadata
3131

3232
/// A Call object that can be used to respond to the request
33-
private(set) lazy var call: Call = {
33+
public lazy var call: Call = {
3434
Call(underlyingCall: cgrpc_handler_get_call(self.underlyingHandler),
3535
owned: false,
3636
completionQueue: self.completionQueue)
@@ -87,16 +87,16 @@ public class Handler {
8787
/// Receive the message sent with a call
8888
///
8989
public func receiveMessage(initialMetadata: Metadata,
90-
completion: @escaping (Data?) throws -> Void) throws {
90+
completion: @escaping (Data?) -> Void) throws {
9191
let operations = OperationGroup(call: call,
9292
operations: [
9393
.sendInitialMetadata(initialMetadata),
9494
.receiveMessage
9595
]) { operationGroup in
9696
if operationGroup.success {
97-
try completion(operationGroup.receivedMessage()?.data())
97+
completion(operationGroup.receivedMessage()?.data())
9898
} else {
99-
try completion(nil)
99+
completion(nil)
100100
}
101101
}
102102
try call.perform(operations)
@@ -152,11 +152,11 @@ public class Handler {
152152
/// - Parameter initialMetadata: initial metadata to send
153153
/// - Parameter completion: a completion handler to call after the metadata has been sent
154154
public func sendMetadata(initialMetadata: Metadata,
155-
completion: ((Bool) throws -> Void)? = nil) throws {
155+
completion: ((Bool) -> Void)? = nil) throws {
156156
let operations = OperationGroup(call: call,
157157
operations: [.sendInitialMetadata(initialMetadata)],
158158
completion: completion != nil
159-
? { operationGroup in try completion?(operationGroup.success) }
159+
? { operationGroup in completion?(operationGroup.success) }
160160
: nil)
161161
try call.perform(operations)
162162
}
@@ -165,7 +165,7 @@ public class Handler {
165165
///
166166
/// - Parameter completion: a completion handler to call after the message has been received
167167
/// - Returns: a tuple containing status codes and a message (if available)
168-
public func receiveMessage(completion: @escaping (Data?) throws -> Void) throws {
168+
public func receiveMessage(completion: @escaping (CallResult) -> Void) throws {
169169
try call.receiveMessage(completion: completion)
170170
}
171171

@@ -181,10 +181,10 @@ public class Handler {
181181
/// Recognize when the client has closed a request
182182
///
183183
/// - Parameter completion: a completion handler to call after request has been closed
184-
public func receiveClose(completion: @escaping (Bool) throws -> Void) throws {
184+
public func receiveClose(completion: @escaping (Bool) -> Void) throws {
185185
let operations = OperationGroup(call: call,
186186
operations: [.receiveCloseOnServer]) { operationGroup in
187-
try completion(operationGroup.success)
187+
completion(operationGroup.success)
188188
}
189189
try call.perform(operations)
190190
}
@@ -197,7 +197,7 @@ public class Handler {
197197
/// - Parameter completion: a completion handler to call after the status has been sent
198198
public func sendStatus(statusCode: StatusCode,
199199
statusMessage: String,
200-
trailingMetadata: Metadata,
200+
trailingMetadata: Metadata = Metadata(),
201201
completion: ((Bool) -> Void)? = nil) throws {
202202
let operations = OperationGroup(call: call,
203203
operations: [
@@ -210,6 +210,14 @@ public class Handler {
210210
}
211211
try call.perform(operations)
212212
}
213+
214+
public func sendError(_ error: ServerErrorStatus,
215+
completion: ((Bool) -> Void)? = nil) throws {
216+
try sendStatus(statusCode: error.statusCode,
217+
statusMessage: error.statusMessage,
218+
trailingMetadata: error.trailingMetadata,
219+
completion: completion)
220+
}
213221
}
214222

215223
extension Handler: Hashable {

‎Sources/SwiftGRPC/Core/OperationGroup.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ class OperationGroup {
3232
private let call: Call
3333

3434
/// An array of operation objects that are passed into the initializer.
35-
private let operations: [Operation]
35+
let operations: [Operation]
3636

3737
/// An array of observers used to watch the operation
3838
private var underlyingObservers: [UnsafeMutableRawPointer] = []
@@ -41,7 +41,7 @@ class OperationGroup {
4141
let underlyingOperations: UnsafeMutableRawPointer?
4242

4343
/// Completion handler that is called when the group completes
44-
let completion: ((OperationGroup) throws -> Void)?
44+
let completion: ((OperationGroup) -> Void)?
4545

4646
/// Indicates that the OperationGroup completed successfully
4747
var success = false
@@ -81,7 +81,7 @@ class OperationGroup {
8181
/// - Parameter operations: an array of operations
8282
init(call: Call,
8383
operations: [Operation],
84-
completion: ((OperationGroup) throws -> Void)? = nil) {
84+
completion: ((OperationGroup) -> Void)? = nil) {
8585
self.call = call
8686
self.operations = operations
8787
self.completion = completion

0 commit comments

Comments
 (0)