Skip to content

In 2.x - is it possible to add metadata to a ServerReponse? Is it possible in a ServerInterceptor? #2119

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
tweidema opened this issue Nov 15, 2024 · 6 comments
Labels
kind/support Adopter support requests.

Comments

@tweidema
Copy link

What are you trying to achieve?

I would like to add metadata to the response sent back to the client. Preferably in a ServerInterceptor.

What have you tried so far?

Here is my attempt, but metadata is a get-only property

struct grpcServerTracingInterceptor: ServerInterceptor {
    let logger: Logger
    init(logger: Logger) {
        self.logger = logger
    }
    func intercept<Input: Sendable, Output: Sendable>(
        request: StreamingServerRequest<Input>,
        context: ServerContext,
        next: @Sendable (
           _ request: StreamingServerRequest<Input>,
           _ context: ServerContext
        ) async throws -> StreamingServerResponse<Output>
    ) async throws -> StreamingServerResponse<Output> {
        logger.info("Pre-intercept of \(context.descriptor.service):\(context.descriptor.method)")
        let callTime = Date()
        // Forward the request.
        var response = try await next(request, context)

        logger.info("Post-intercept of \(context.descriptor.service):\(context.descriptor.method)")
        let duration = callTime.timeIntervalSinceNow
        response.metadata.addString("gRPC;dur=\(duration)", forKey: "Server-Timing")
        return response
    }
}

The addString (2nd-last line) gives: Cannot use mutating member on immutable value: 'metadata' is a get-only property

@tweidema tweidema added the kind/support Adopter support requests. label Nov 15, 2024
@glbrntt
Copy link
Collaborator

glbrntt commented Nov 15, 2024

metadata is a computed property on StreamingServerResponse, so to mutate it (at the moment) you need to go via accepted and mutate that.

That'll solve your immediate problem, but I think we should add an API to make this more convenient.

@tweidema
Copy link
Author

tweidema commented Nov 15, 2024

@glbrntt Thank you! Could you maybe help someone who does not yet have many months of Swift experience out with a few more details? Here is what I've tried. It compiles, but the client gets "the http/2 server reset the stream", so I'm obviously doing something wrong.
At the end of my ServerInterceptor (after the line "let duration = callTime.timeIntervalSinceNow" from above)

    let oldResult = response.accepted
    let old_res = try oldResult.get()
    var new_metadata = old_res.metadata
    new_metadata.addString("gRPC;dur=\(duration)", forKey: "Server-Timing")
    return StreamingServerResponse<Output>(metadata: new_metadata, producer: old_res.producer)

@glbrntt
Copy link
Collaborator

glbrntt commented Nov 15, 2024

Absolutely :)

Interesting, I suspect that happens because try oldResult.get() throws.

What you should do here is switch over response.accepted, modify the appropriate value for each case, and then assign that back to response.accepted:

var response = try await next(...)

switch response.accepted {
case .success(var contents):
  contents.metadata.addString(...)
  response.accepted = .success(contents)
case .failure(var error):
  error.metadata.addString(...)
  response.accepted = .failure(error)
}

return response

@glbrntt
Copy link
Collaborator

glbrntt commented Nov 15, 2024

I just opened #2120 to provide a convenience setter for this as well.

@tweidema
Copy link
Author

Thank so much. Looks much better than my code :-) But I still get the error.
If I comment out the line

contents.metadata.addString(timingHeader, forKey: "Server-Timing")

the call completes normally. I tried adding a do/catch block around the switch, but vscode says the catch-block is unreachable because no errors are thrown in the do block.
I then tried shaving down both the header key and values and found the cause to any upper-case letters in the server-key.
so

contents.metadata.addString(timingHeader, forKey: "server-timing")

works, but

contents.metadata.addString(timingHeader, forKey: "Server-timing")

gives "the http/2 server reset the stream.

@glbrntt
Copy link
Collaborator

glbrntt commented Nov 15, 2024

Ah, so HTTP/2 header field names must be lowercase. I thought we did that conversion for you, but clearly we don't, so I'll fix that up too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/support Adopter support requests.
Projects
None yet
Development

No branches or pull requests

2 participants