Skip to content

Commit 4cf5802

Browse files
committed
Provide errors to failed client promises rather than a status
Motivation: If an RPC fails for some reason we will convert the error into an appropriate `GRPCStatus`. This status is used to succeed the status promise and fail any other promises (initial and trailing metadata, and if applicable, the response promise). Without looking at logs it's harder to determine the exact nature of the error. Modifications: - Fail the initial metadata, trailing metadata and response promise with the error that the status was derived from. Result: More helpful errors.
1 parent 4be0ed1 commit 4cf5802

File tree

4 files changed

+40
-9
lines changed

4 files changed

+40
-9
lines changed

Sources/GRPC/ClientCalls/ClientCallTransport.swift

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -391,7 +391,7 @@ extension ChannelTransport {
391391
}
392392

393393
// Update our state: we're closing.
394-
self.close(withStatus: errorStatus)
394+
self.close(error: error, status: errorStatus)
395395
promise?.fail(errorStatus)
396396

397397
case .closed:
@@ -402,7 +402,7 @@ extension ChannelTransport {
402402
/// Close the call, if it's not yet closed with the given status.
403403
///
404404
/// Must be called from the event loop.
405-
private func close(withStatus status: GRPCStatus) {
405+
private func close(error: Error, status: GRPCStatus) {
406406
self.eventLoop.preconditionInEventLoop()
407407

408408
switch self.state {
@@ -416,7 +416,7 @@ extension ChannelTransport {
416416
self.scheduledTimeout = nil
417417

418418
// Fail any outstanding promises.
419-
self.responseContainer.fail(with: status)
419+
self.responseContainer.fail(with: error, status: status)
420420

421421
// Fail any buffered writes.
422422
while !self.requestBuffer.isEmpty {
@@ -439,7 +439,7 @@ extension ChannelTransport {
439439
self.scheduledTimeout = nil
440440

441441
// Fail any outstanding promises.
442-
self.responseContainer.fail(with: status)
442+
self.responseContainer.fail(with: error, status: status)
443443

444444
// Close the channel.
445445
channel.close(mode: .all, promise: nil)
@@ -502,7 +502,7 @@ extension ChannelTransport: ClientCallInbound {
502502
// We're not really failing the status here; in some cases the server may fast fail, in which
503503
// case we'll only see trailing metadata and status: we should fail the initial metadata and
504504
// response in that case.
505-
self.responseContainer.fail(with: status)
505+
self.responseContainer.fail(with: status, status: status)
506506
}
507507

508508
case .closed:

Sources/GRPC/ClientCalls/ResponsePartContainer.swift

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,16 +37,16 @@ internal struct ResponsePartContainer<Response: GRPCPayload> {
3737

3838
/// Fail all promises - except for the status promise - with the given error status. Succeed the
3939
/// status promise.
40-
mutating func fail(with status: GRPCStatus) {
41-
self.lazyInitialMetadataPromise.fail(status)
40+
mutating func fail(with error: Error, status: GRPCStatus) {
41+
self.lazyInitialMetadataPromise.fail(error)
4242

4343
switch self.responseHandler {
4444
case .unary(let response):
45-
response.fail(status)
45+
response.fail(error)
4646
case .stream:
4747
()
4848
}
49-
self.lazyTrailingMetadataPromise.fail(status)
49+
self.lazyTrailingMetadataPromise.fail(error)
5050
// We always succeed the status.
5151
self.lazyStatusPromise.succeed(status)
5252
}

Tests/GRPCTests/ChannelTransportTests.swift

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -340,6 +340,36 @@ class ChannelTransportTests: GRPCTestCase {
340340
// Promise should fail.
341341
XCTAssertThrowsError(try requestHeadPromise.futureResult.wait())
342342
}
343+
344+
func testErrorsAreNotAlwaysStatus() throws {
345+
let channel = EmbeddedChannel()
346+
let responsePromise = channel.eventLoop.makePromise(of: Response.self)
347+
let container = ResponsePartContainer<Response>(
348+
eventLoop: channel.eventLoop,
349+
unaryResponsePromise: responsePromise
350+
)
351+
352+
let transport = self.makeEmbeddedTransport(channel: channel, container: container)
353+
transport.activate(stream: channel)
354+
355+
// Send an error
356+
transport.receiveError(GRPCError.RPCCancelledByClient())
357+
358+
XCTAssertThrowsError(try transport.responseContainer.lazyInitialMetadataPromise.getFutureResult().wait()) { error in
359+
XCTAssertTrue(error is GRPCError.RPCCancelledByClient)
360+
}
361+
362+
XCTAssertThrowsError(try transport.responseContainer.lazyTrailingMetadataPromise.getFutureResult().wait()) { error in
363+
XCTAssertTrue(error is GRPCError.RPCCancelledByClient)
364+
}
365+
366+
XCTAssertThrowsError(try responsePromise.futureResult.wait()) { error in
367+
XCTAssertTrue(error is GRPCError.RPCCancelledByClient)
368+
}
369+
370+
// Status never fails.
371+
XCTAssertNoThrow(try transport.responseContainer.lazyStatusPromise.getFutureResult().wait())
372+
}
343373
}
344374

345375
extension _GRPCClientRequestPart {

Tests/GRPCTests/XCTestManifests.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ extension ChannelTransportTests {
2222
("testBufferedWritesAreFailedOnClose", testBufferedWritesAreFailedOnClose),
2323
("testChannelBecomesInactive", testChannelBecomesInactive),
2424
("testChannelError", testChannelError),
25+
("testErrorsAreNotAlwaysStatus", testErrorsAreNotAlwaysStatus),
2526
("testInboundMethodsAfterShutdown", testInboundMethodsAfterShutdown),
2627
("testOutboundMethodsAfterShutdown", testOutboundMethodsAfterShutdown),
2728
("testTimeoutAfterActivating", testTimeoutAfterActivating),

0 commit comments

Comments
 (0)