Move GetSessionID closure into ServerOptions #488
Conversation
|
|
||
| handler := NewStreamableHTTPHandler(func(*http.Request) *Server { return server }, &StreamableHTTPOptions{ | ||
| server := NewServer(testImpl, &ServerOptions{ | ||
| KeepAlive: 10 * time.Millisecond, |
There was a problem hiding this comment.
Why is KeepAlive being set here?
There was a problem hiding this comment.
I see you already resolved this but just for future reference: this is an artifact of moving code around (i.e., the keep alive was there already).
I was wondering about this separately when debugging leaking goroutines and flakyness and it took me some time to realize that the keep alive here is used as a way to make the server close the connection with the client (because it would fail the keep alive after 10ms). This is necessary in this test because of the bug that transport cleanup does not happen even when the client closes the connection itself (which does happen in this test).
When we merge #480 we can actually improve this test and remove the keepalive.
|
Never mind, I did it. |
2 participants
Fixes: #478