tmc / grpc-websocket-proxy

A proxy to transparently upgrade grpc-gateway streaming endpoints to use websockets
MIT License
553 stars 72 forks source link

Allow usage of the ping and pong handling of Gorilla websocket. #21

Closed TvdBrink closed 4 years ago

TvdBrink commented 4 years ago

With func WithPingControl(interval time.Duration) Option {} it is now possible to specify a ping interval. based on this parameter the ping and pong deadlines are calculated. This allows faster detection of mobile clients with connection lost.

Verelias commented 4 years ago

The proposed structure looks solid, though I would advise maybe splitting up the ping and pong section in two different handlers as the RFC6455 (Websockets) specifies that a pong message may be sent unsolicited which opens up a use-case where a service does not (or rarely) sends out ping frames but receives a lot of unsolicited pong frames to ensure that the service has an accurate understanding of its client's status. @tmc what are your thoughts on this?

TvdBrink commented 4 years ago

@tmc can you review this please.

tmc commented 4 years ago

It doesn't look like there's a way to gracefully stop that goroutine -- can you consider propagating a context object so it can be stoped/canceled cleanly?

TvdBrink commented 4 years ago

@tmc I have added the ctx.Done() select to the goroutine for the graceful stop of the goroutine.

TvdBrink commented 4 years ago

@tmc any update?

tmc commented 4 years ago

LGTM