Skip to content

Commit ce25e2c

Browse files
gjcairogjcairo
authored
Flatten RPCError causes if they're also RPCErrors with the same code (#2083)
## Motivation For errors happening deep in the task tree, we'd wrap them in many layers of `RPCError`s. This isn't particularly nice. ## Modifications This PR changes the behaviour of the `RPCError` initialiser to flatten the cause as long as it's an `RPCError` with the same status code as the wrapping error. ## Result Friendlier errors.
1 parent 22d1b57 commit ce25e2c

File tree

2 files changed

+187
-78
lines changed

2 files changed

+187
-78
lines changed

‎Sources/GRPCCore/RPCError.swift‎

Lines changed: 50 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -35,18 +35,62 @@ public struct RPCError: Sendable, Hashable, Error {
3535
/// The original error which led to this error being thrown.
3636
public var cause: (any Error)?
3737

38-
/// Create a new RPC error.
38+
/// Create a new RPC error. If the given `cause` is also an ``RPCError`` sharing the same `code`,
39+
/// then they will be flattened into a single error, by merging the messages and metadata.
3940
///
4041
/// - Parameters:
4142
/// - code: The status code.
4243
/// - message: A message providing additional context about the code.
4344
/// - metadata: Any metadata to attach to the error.
4445
/// - cause: An underlying error which led to this error being thrown.
45-
public init(code: Code, message: String, metadata: Metadata = [:], cause: (any Error)? = nil) {
46-
self.code = code
47-
self.message = message
48-
self.metadata = metadata
49-
self.cause = cause
46+
public init(
47+
code: Code,
48+
message: String,
49+
metadata: Metadata = [:],
50+
cause: (any Error)? = nil
51+
) {
52+
if let rpcErrorCause = cause as? RPCError {
53+
self = .init(
54+
code: code,
55+
message: message,
56+
metadata: metadata,
57+
cause: rpcErrorCause
58+
)
59+
} else {
60+
self.code = code
61+
self.message = message
62+
self.metadata = metadata
63+
self.cause = cause
64+
}
65+
}
66+
67+
/// Create a new RPC error. If the given `cause` shares the same `code`, then it will be flattened
68+
/// into a single error, by merging the messages and metadata.
69+
///
70+
/// - Parameters:
71+
/// - code: The status code.
72+
/// - message: A message providing additional context about the code.
73+
/// - metadata: Any metadata to attach to the error.
74+
/// - cause: An underlying ``RPCError`` which led to this error being thrown.
75+
public init(
76+
code: Code,
77+
message: String,
78+
metadata: Metadata = [:],
79+
cause: RPCError
80+
) {
81+
if cause.code == code {
82+
self.code = code
83+
self.message = message + " \(cause.message)"
84+
var mergedMetadata = metadata
85+
mergedMetadata.add(contentsOf: cause.metadata)
86+
self.metadata = mergedMetadata
87+
self.cause = cause.cause
88+
} else {
89+
self.code = code
90+
self.message = message
91+
self.metadata = metadata
92+
self.cause = cause
93+
}
5094
}
5195

5296
/// Create a new RPC error from the provided ``Status``.

‎Tests/GRPCCoreTests/RPCErrorTests.swift‎

Lines changed: 137 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -14,114 +14,179 @@
1414
* limitations under the License.
1515
*/
1616
import GRPCCore
17-
import XCTest
18-
19-
final class RPCErrorTests: XCTestCase {
20-
private static let statusCodeRawValue: [(RPCError.Code, Int)] = [
21-
(.cancelled, 1),
22-
(.unknown, 2),
23-
(.invalidArgument, 3),
24-
(.deadlineExceeded, 4),
25-
(.notFound, 5),
26-
(.alreadyExists, 6),
27-
(.permissionDenied, 7),
28-
(.resourceExhausted, 8),
29-
(.failedPrecondition, 9),
30-
(.aborted, 10),
31-
(.outOfRange, 11),
32-
(.unimplemented, 12),
33-
(.internalError, 13),
34-
(.unavailable, 14),
35-
(.dataLoss, 15),
36-
(.unauthenticated, 16),
37-
]
17+
import Testing
3818

19+
@Suite("RPCError Tests")
20+
struct RPCErrorTests {
21+
@Test("Custom String Convertible")
3922
func testCustomStringConvertible() {
40-
XCTAssertDescription(RPCError(code: .dataLoss, message: ""), #"dataLoss: """#)
41-
XCTAssertDescription(RPCError(code: .unknown, message: "message"), #"unknown: "message""#)
42-
XCTAssertDescription(RPCError(code: .aborted, message: "message"), #"aborted: "message""#)
23+
#expect(String(describing: RPCError(code: .dataLoss, message: "")) == #"dataLoss: """#)
24+
#expect(
25+
String(describing: RPCError(code: .unknown, message: "message")) == #"unknown: "message""#
26+
)
27+
#expect(
28+
String(describing: RPCError(code: .aborted, message: "message")) == #"aborted: "message""#
29+
)
4330

4431
struct TestError: Error {}
45-
XCTAssertDescription(
46-
RPCError(code: .aborted, message: "message", cause: TestError()),
47-
#"aborted: "message" (cause: "TestError()")"#
32+
#expect(
33+
String(describing: RPCError(code: .aborted, message: "message", cause: TestError()))
34+
== #"aborted: "message" (cause: "TestError()")"#
4835
)
4936
}
5037

38+
@Test("Error from Status")
5139
func testErrorFromStatus() throws {
5240
var status = Status(code: .ok, message: "")
5341
// ok isn't an error
54-
XCTAssertNil(RPCError(status: status))
42+
#expect(RPCError(status: status) == nil)
5543

5644
status.code = .invalidArgument
57-
var error = try XCTUnwrap(RPCError(status: status))
58-
XCTAssertEqual(error.code, .invalidArgument)
59-
XCTAssertEqual(error.message, "")
60-
XCTAssertEqual(error.metadata, [:])
45+
var error = try #require(RPCError(status: status))
46+
#expect(error.code == .invalidArgument)
47+
#expect(error.message == "")
48+
#expect(error.metadata == [:])
6149

6250
status.code = .cancelled
6351
status.message = "an error message"
64-
error = try XCTUnwrap(RPCError(status: status))
65-
XCTAssertEqual(error.code, .cancelled)
66-
XCTAssertEqual(error.message, "an error message")
67-
XCTAssertEqual(error.metadata, [:])
52+
error = try #require(RPCError(status: status))
53+
#expect(error.code == .cancelled)
54+
#expect(error.message == "an error message")
55+
#expect(error.metadata == [:])
6856
}
6957

70-
func testErrorCodeFromStatusCode() throws {
71-
XCTAssertNil(RPCError.Code(Status.Code.ok))
72-
XCTAssertEqual(RPCError.Code(Status.Code.cancelled), .cancelled)
73-
XCTAssertEqual(RPCError.Code(Status.Code.unknown), .unknown)
74-
XCTAssertEqual(RPCError.Code(Status.Code.invalidArgument), .invalidArgument)
75-
XCTAssertEqual(RPCError.Code(Status.Code.deadlineExceeded), .deadlineExceeded)
76-
XCTAssertEqual(RPCError.Code(Status.Code.notFound), .notFound)
77-
XCTAssertEqual(RPCError.Code(Status.Code.alreadyExists), .alreadyExists)
78-
XCTAssertEqual(RPCError.Code(Status.Code.permissionDenied), .permissionDenied)
79-
XCTAssertEqual(RPCError.Code(Status.Code.resourceExhausted), .resourceExhausted)
80-
XCTAssertEqual(RPCError.Code(Status.Code.failedPrecondition), .failedPrecondition)
81-
XCTAssertEqual(RPCError.Code(Status.Code.aborted), .aborted)
82-
XCTAssertEqual(RPCError.Code(Status.Code.outOfRange), .outOfRange)
83-
XCTAssertEqual(RPCError.Code(Status.Code.unimplemented), .unimplemented)
84-
XCTAssertEqual(RPCError.Code(Status.Code.internalError), .internalError)
85-
XCTAssertEqual(RPCError.Code(Status.Code.unavailable), .unavailable)
86-
XCTAssertEqual(RPCError.Code(Status.Code.dataLoss), .dataLoss)
87-
XCTAssertEqual(RPCError.Code(Status.Code.unauthenticated), .unauthenticated)
58+
@Test(
59+
"Error Code from Status Code",
60+
arguments: [
61+
(Status.Code.ok, nil),
62+
(Status.Code.cancelled, RPCError.Code.cancelled),
63+
(Status.Code.unknown, RPCError.Code.unknown),
64+
(Status.Code.invalidArgument, RPCError.Code.invalidArgument),
65+
(Status.Code.deadlineExceeded, RPCError.Code.deadlineExceeded),
66+
(Status.Code.notFound, RPCError.Code.notFound),
67+
(Status.Code.alreadyExists, RPCError.Code.alreadyExists),
68+
(Status.Code.permissionDenied, RPCError.Code.permissionDenied),
69+
(Status.Code.resourceExhausted, RPCError.Code.resourceExhausted),
70+
(Status.Code.failedPrecondition, RPCError.Code.failedPrecondition),
71+
(Status.Code.aborted, RPCError.Code.aborted),
72+
(Status.Code.outOfRange, RPCError.Code.outOfRange),
73+
(Status.Code.unimplemented, RPCError.Code.unimplemented),
74+
(Status.Code.internalError, RPCError.Code.internalError),
75+
(Status.Code.unavailable, RPCError.Code.unavailable),
76+
(Status.Code.dataLoss, RPCError.Code.dataLoss),
77+
(Status.Code.unauthenticated, RPCError.Code.unauthenticated),
78+
]
79+
)
80+
func testErrorCodeFromStatusCode(statusCode: Status.Code, rpcErrorCode: RPCError.Code?) throws {
81+
#expect(RPCError.Code(statusCode) == rpcErrorCode)
8882
}
8983

84+
@Test("Equatable Conformance")
9085
func testEquatableConformance() {
91-
XCTAssertEqual(
92-
RPCError(code: .cancelled, message: ""),
86+
#expect(
9387
RPCError(code: .cancelled, message: "")
88+
== RPCError(code: .cancelled, message: "")
9489
)
9590

96-
XCTAssertEqual(
97-
RPCError(code: .cancelled, message: "message"),
91+
#expect(
9892
RPCError(code: .cancelled, message: "message")
93+
== RPCError(code: .cancelled, message: "message")
9994
)
10095

101-
XCTAssertEqual(
102-
RPCError(code: .cancelled, message: "message", metadata: ["foo": "bar"]),
96+
#expect(
10397
RPCError(code: .cancelled, message: "message", metadata: ["foo": "bar"])
98+
== RPCError(code: .cancelled, message: "message", metadata: ["foo": "bar"])
10499
)
105100

106-
XCTAssertNotEqual(
107-
RPCError(code: .cancelled, message: ""),
101+
#expect(
102+
RPCError(code: .cancelled, message: "")
103+
!= RPCError(code: .cancelled, message: "message")
104+
)
105+
106+
#expect(
108107
RPCError(code: .cancelled, message: "message")
108+
!= RPCError(code: .unknown, message: "message")
109109
)
110110

111-
XCTAssertNotEqual(
112-
RPCError(code: .cancelled, message: "message"),
113-
RPCError(code: .unknown, message: "message")
111+
#expect(
112+
RPCError(code: .cancelled, message: "message", metadata: ["foo": "bar"])
113+
!= RPCError(code: .cancelled, message: "message", metadata: ["foo": "baz"])
114114
)
115+
}
115116

116-
XCTAssertNotEqual(
117-
RPCError(code: .cancelled, message: "message", metadata: ["foo": "bar"]),
118-
RPCError(code: .cancelled, message: "message", metadata: ["foo": "baz"])
117+
@Test(
118+
"Status Code Raw Values",
119+
arguments: [
120+
(RPCError.Code.cancelled, 1),
121+
(.unknown, 2),
122+
(.invalidArgument, 3),
123+
(.deadlineExceeded, 4),
124+
(.notFound, 5),
125+
(.alreadyExists, 6),
126+
(.permissionDenied, 7),
127+
(.resourceExhausted, 8),
128+
(.failedPrecondition, 9),
129+
(.aborted, 10),
130+
(.outOfRange, 11),
131+
(.unimplemented, 12),
132+
(.internalError, 13),
133+
(.unavailable, 14),
134+
(.dataLoss, 15),
135+
(.unauthenticated, 16),
136+
]
137+
)
138+
func testStatusCodeRawValues(statusCode: RPCError.Code, rawValue: Int) {
139+
#expect(statusCode.rawValue == rawValue, "\(statusCode) had unexpected raw value")
140+
}
141+
142+
@Test("Flatten causes with same status code")
143+
func testFlattenCausesWithSameStatusCode() {
144+
let error1 = RPCError(code: .unknown, message: "Error 1.")
145+
let error2 = RPCError(code: .unknown, message: "Error 2.", cause: error1)
146+
let error3 = RPCError(code: .dataLoss, message: "Error 3.", cause: error2)
147+
let error4 = RPCError(code: .aborted, message: "Error 4.", cause: error3)
148+
let error5 = RPCError(
149+
code: .aborted,
150+
message: "Error 5.",
151+
cause: error4
152+
)
153+
154+
let unknownMerged = RPCError(code: .unknown, message: "Error 2. Error 1.")
155+
let dataLossMerged = RPCError(code: .dataLoss, message: "Error 3.", cause: unknownMerged)
156+
let abortedMerged = RPCError(
157+
code: .aborted,
158+
message: "Error 5. Error 4.",
159+
cause: dataLossMerged
119160
)
161+
#expect(error5 == abortedMerged)
120162
}
121163

122-
func testStatusCodeRawValues() {
123-
for (code, expected) in Self.statusCodeRawValue {
124-
XCTAssertEqual(code.rawValue, expected, "\(code) had unexpected raw value")
125-
}
164+
@Test("Causes of errors with different status codes aren't flattened")
165+
func testDifferentStatusCodeAreNotFlattened() throws {
166+
let error1 = RPCError(code: .unknown, message: "Error 1.")
167+
let error2 = RPCError(code: .dataLoss, message: "Error 2.", cause: error1)
168+
let error3 = RPCError(code: .alreadyExists, message: "Error 3.", cause: error2)
169+
let error4 = RPCError(code: .aborted, message: "Error 4.", cause: error3)
170+
let error5 = RPCError(
171+
code: .deadlineExceeded,
172+
message: "Error 5.",
173+
cause: error4
174+
)
175+
176+
#expect(error5.code == .deadlineExceeded)
177+
#expect(error5.message == "Error 5.")
178+
let wrappedError4 = try #require(error5.cause as? RPCError)
179+
#expect(wrappedError4.code == .aborted)
180+
#expect(wrappedError4.message == "Error 4.")
181+
let wrappedError3 = try #require(wrappedError4.cause as? RPCError)
182+
#expect(wrappedError3.code == .alreadyExists)
183+
#expect(wrappedError3.message == "Error 3.")
184+
let wrappedError2 = try #require(wrappedError3.cause as? RPCError)
185+
#expect(wrappedError2.code == .dataLoss)
186+
#expect(wrappedError2.message == "Error 2.")
187+
let wrappedError1 = try #require(wrappedError2.cause as? RPCError)
188+
#expect(wrappedError1.code == .unknown)
189+
#expect(wrappedError1.message == "Error 1.")
190+
#expect(wrappedError1.cause == nil)
126191
}
127192
}

0 commit comments

Comments
 (0)