Skip to content
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

SttpClientInterpreter does not propagate headers in the http response for websockets #3959

Open
kamilkloch opened this issue Jul 26, 2024 · 6 comments

Comments

@kamilkloch
Copy link
Contributor

We are using tapir interceptor to enrich server responses with some contextual information (e.g. correlation id):

private val serverOptions = Http4sServerOptions
  .customiseInterceptors[IO]
  .prependInterceptor(LoggingContextInterceptor)
  .options

/** Tapir interceptor which extracts correlation ID from the request headers and puts correlation ID into Http response header. If
  * correlation ID is not present in the HTTP header, a new one is generated.
  */
object LoggingContextInterceptor extends RequestInterceptor[IO] 

On the client side, we wrap tapir interpreter to extract correlation id from the response header and lift the original

def SttpClientInterpreter#toClientThrowErrors: I => F[O]

to

def SttpClientInterpreterWithRandomCorrelationId#toClientThrowErrors: I => ResponseWithContext[F[O]]

/** Container holding
 *   - http response (e.g. tapir high-level case class),
 *   - logging context of the response (e.g correlation id extracted from http headers)
 *
 * Captured logging context allows to match server-side and client-side log (by correlation id).
 */
case class ResponseWithContext[A](ctx: LoggingContext, response: A)

In case of rest calls the wrapped tapir client properly extracts correlation id from the response.

In case of websockets, correlation id header is missing - the underlying SttpClientInterpreter returns a response without the header. A low-level sttp websocket client does receive the correlation id:

***** sttp websocket client
[2024-07-26 14:49:47,353] INFO  [io-compute-8] TapirWebSocketsCorrelationId:37 - [cid=XTM-QVL-HTR] sttp ws request
[2024-07-26 14:49:47,694] INFO  [io-compute-14] LoggingContextInterceptor:44 - RequestMetadata(GET,/ping,List(x-correlation-id: XTM-QVL-HTR, accept-encoding: gzip, deflate, upgrade: websocket, connection: upgrade, sec-websocket-key: C2/p0NQzhCMWpTQO5jXngQ==, sec-websocket-version: 13, origin: http://localhost:8081, host: localhost:8081, accept: */*, user-agent: AHC/2.1)), List(sttp.tapir.server.ServerEndpoint$$anon$3@249b2b39)
[2024-07-26 14:49:47,719] INFO  [io-compute-14] LoggingContextInterceptor:52 - response: Response(ServerResponse(200,Vector(x-correlation-id: XTM-QVL-HTR)))
[2024-07-26 14:49:47,741] INFO  [io-compute-22] TapirWebSocketsCorrelationId:51 - [cid=XTM-QVL-HTR] Sending 1
[2024-07-26 14:49:47,782] INFO  [io-compute-6] TapirWebSocketsCorrelationId:53 - [cid=XTM-QVL-HTR] Received 1
[2024-07-26 14:49:47,795] INFO  [io-compute-19] TapirWebSocketsCorrelationId:80 - [cid=unset] 

***** tapir websocket client
[2024-07-26 14:49:47,827] INFO  [io-compute-2] SttpClientInterpreterWithRandomCorrelationId:25 - [cid=LZE-JTD-WQP] tapir ws request
[2024-07-26 14:49:47,855] INFO  [io-compute-20] LoggingContextInterceptor:44 - RequestMetadata(GET,/ping,List(Connection: Upgrade, Host: localhost:8081, Upgrade: websocket, User-Agent: Java-http-client/21.0.3, Accept-Encoding: gzip, deflate, Sec-WebSocket-Key: sZ/mmEyrbVLoResXK3uMnA==, Sec-WebSocket-Version: 13, x-correlation-id: LZE-JTD-WQP)), List(sttp.tapir.server.ServerEndpoint$$anon$3@249b2b39)
[2024-07-26 14:49:47,856] INFO  [io-compute-20] LoggingContextInterceptor:52 - response: Response(ServerResponse(200,Vector(x-correlation-id: LZE-JTD-WQP)))
[2024-07-26 14:49:47,870] INFO  [io-compute-0] SttpClientInterpreterWithRandomCorrelationId:28 - [cid=unset] Response(sttp.tapir.client.sttp.ws.fs2.WebSocketToFs2Pipe$$Lambda/0x000071f2346ac678@70e0de77,101,,List(),List(),RequestMetadata(GET,ws://localhost:8081/ping,Vector(Accept-Encoding: gzip, deflate, x-correlation-id: LZE-JTD-WQP)))
[2024-07-26 14:49:47,875] INFO  [io-compute-12] TapirWebSocketsCorrelationId:64 - [cid=unset] Sending 1
[2024-07-26 14:49:47,882] INFO  [io-compute-1] TapirWebSocketsCorrelationId:66 - [cid=unset] Received 1

Repo with the code: https://github.com/kamilkloch/tapir-correlation-id

@adamw
Copy link
Member

adamw commented Aug 5, 2024

I'm having some problems trying to reproduce the issue. See my attempt here: https://github.com/softwaremill/tapir/pull/3965/files

Initially I was using HttpClientFs2Backend, and this was indeed failing, because there's no way to obtain response headers in case of web sockets when using Java's HttpClient (at least I can't find a way; see also softwaremill/sttp#2245)

However, with AsyncHttpClientFs2Backend, the test passes.

@kamilkloch
Copy link
Contributor Author

Could you perhaps take a stab at https://github.com/kamilkloch/tapir-correlation-id?
I am aware the code is mode convoluted, but I think it shows that the header is gone.

@adamw
Copy link
Member

adamw commented Aug 6, 2024

Maybe in September. Ideally I'd need a minimized test case, or a really minimized app :)

@kamilkloch
Copy link
Contributor Author

Would you like me to minimize it more? Deal? ;)

@adamw
Copy link
Member

adamw commented Aug 7, 2024

Well if you could minimize it - of course, please do - though both me & @kciesielski are unavailable in August, sorry :)

@kamilkloch
Copy link
Contributor Author

That is great to hear, very well deservered unplugged time for both of you. Will try to minimize.

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

No branches or pull requests

2 participants