Skip to content

Commit bc6bc0d

Browse files
fixup: Treat the user handler throwing GRPCStatus.ok as an error
Signed-off-by: Si Beaumont <[email protected]>
1 parent 75b161a commit bc6bc0d

File tree

2 files changed

+16
-4
lines changed

2 files changed

+16
-4
lines changed

Sources/GRPC/AsyncAwaitSupport/GRPCAsyncServerHandler.swift

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -362,7 +362,18 @@ internal final class AsyncServerHandler<
362362
}
363363

364364
// Call the user function.
365-
try await self.userHandler(requestStream, responseStreamWriter, context)
365+
do {
366+
try await self.userHandler(requestStream, responseStreamWriter, context)
367+
} catch {
368+
// Throwing GRPCStatus.ok is considered to be invalid.
369+
if (error as? GRPCStatus)?.isOk ?? false {
370+
throw GRPCStatus(
371+
code: .unknown,
372+
message: "Handler threw GRPCStatus error with code .ok"
373+
)
374+
}
375+
throw error
376+
}
366377

367378
// Check for cancellation after the user function has returned so we don't return OK in the
368379
// event the RPC was cancelled. This is probably overkill because the `handleError(_:)`

Tests/GRPCTests/GRPCAsyncServerHandlerTests.swift

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -287,7 +287,7 @@ class AsyncServerHandlerTests: ServerHandlerTestCaseBase {
287287
await assertThat(self.recorder.trailers, .is([:]))
288288
} }
289289

290-
func testHandlerThrowsGRPCStatusOK() { XCTAsyncTest {
290+
func testHandlerThrowsGRPCStatusOKResultsInUnknownStatus() { XCTAsyncTest {
291291
// Create a user function that immediately throws GRPCStatus.ok.
292292
let handler = self.makeHandler { _, _, _ in
293293
throw GRPCStatus.ok
@@ -299,9 +299,10 @@ class AsyncServerHandlerTests: ServerHandlerTestCaseBase {
299299
// Wait for user handler to finish (it's gonna throw immediately).
300300
await assertThat(await handler.task?.value, .notNil())
301301

302-
// Check the status is OK.
303-
await assertThat(self.recorder.status, .notNil(.hasCode(.ok)))
302+
// Check the status is `.unknown`.
303+
await assertThat(self.recorder.status, .notNil(.hasCode(.unknown)))
304304
} }
305+
305306
}
306307

307308
#endif

0 commit comments

Comments
 (0)