Propagate context to server RPC handlers #2059
Conversation
Motivation: The interceptors API has a context which, at the moment, only includes the name of the RPC. This information is generally useful so should be propagated to the server handler too. Information on the context can also be extended later to include things like the identity of the remote peer, or info about the state of the RPC. Modifications: - Rename 'ServerInterceptorContext' to 'ServerContext' - Make the transport the source of the context and have that provide it to the 'listen' method. Propagate this through the server stack to the generated stubs. - Update code generator to include the context - Update generated code - Update services Results: RPC handlers have a context provided by a transport
| @@ -171,7 +171,7 @@ final class InProcessClientTransportTests: XCTestCase { | |||
| } | |||
|
|
|||
| group.addTask { | |||
| try await server.listen { stream in | |||
There was a problem hiding this comment.
Aren't we getting a warning because we're not using context here? Should we _ it, or should we assert the contained method descriptor?
There was a problem hiding this comment.
Nope, no warning for not using it. I don't mind whether we change it to _ or leave it as context. Let me know what you prefer.
I'll add a separate test checking we get the descriptor in the context.
There was a problem hiding this comment.
Ah, that's fine, I'm okay with the name. I thought warnings were emitted in this case :D
| @@ -41,7 +41,7 @@ final class InProcessServerTransportTests: XCTestCase { | |||
|
|
|||
| try await withThrowingTaskGroup(of: Void.self) { group in | |||
| group.addTask { | |||
| try await transport.listen { stream in | |||
There was a problem hiding this comment.
Aren't we getting a warning because we're not using context here? Should we _ it, or should we assert the contained method descriptor?
| @@ -71,7 +71,7 @@ final class InProcessServerTransportTests: XCTestCase { | |||
|
|
|||
| try transport.acceptStream(firstStream) | |||
|
|
|||
| try await transport.listen { stream in | |||
There was a problem hiding this comment.
Aren't we getting a warning because we're not using context here? Should we _ it, or should we assert the contained method descriptor?
| @@ -121,7 +121,7 @@ struct ClientRPCExecutorTestHarness { | |||
| ) async throws { | |||
| try await withThrowingTaskGroup(of: Void.self) { group in | |||
| group.addTask { | |||
| try await self.serverTransport.listen { stream in | |||
There was a problem hiding this comment.
Aren't we getting a warning because we're not using context here?
Motivation:
The interceptors API has a context which, at the moment, only includes
the name of the RPC. This information is generally useful so should be
propagated to the server handler too. Information on the context can
also be extended later to include things like the identity of the remote
peer, or info about the state of the RPC.
Modifications:
to the 'listen' method. Propagate this through the server stack to the
generated stubs.
Results:
RPC handlers have a context provided by a transport