Document how to have read timeout be larger than idle timeout
- Dominant language
- Go
- Stars
- 5.5k
- Forks
- 372
- PR merge metrics
- No merged PRs in 30d
Description
Hi! Thanks for this excellent library.
I have a use case where I want the server to wait up to 10 seconds for the client to start sending data. If the server doesn't get a new Reader within that time, it closes the connection and bails.
However, if the client *does* start sending data, then they have a much longer time limit to finish sending. Say 2 minutes. As far as I can tell, there's not a good way to accomplish this with the current API design.
I saw this issue: https://github.com/nhooyr/websocket/issues/87
Which works IFF the idle timeout is larger than the read timeout.
This is because the `Conn.msgReader` holds on to the context passed in during `Reader()` Example:
```
ctx, cancel := context.WithTimeout(rootCtx, 10 *second)
defer cancel()
_, reader, err := ws.Reader(ctx)
if err != nil {
// Err handling....
log.Error(err)
return
}
// If reading data takes longer than 10 seconds, the timeout above will fire, and the context will be cancelled
// Killing the connnection
data, err := io.ReadAll(reader)
if err != nil {
// Err handling....
log.Error(err)
return
}
```
I can potentially work around this. Instead of using `context.WithTimeout`, I could just use `context.WithCancel`. Then have a `time.AfterFunc()`, which uses atomics to check if we got the reader already. In which case, don't cancel. Example:
```
ctx, cancel := context.WithCancel(rootCtx)
defer cancel()
gotReader := atomic.Bool{}
time.AfterFunc(10*time.Second, func() {
if !gotReader.Load() {
cancel()
}
})
_, reader, err := ws.Reader(ctx)
if err != nil {
// Err handling....
log.Error(err)
return
}
gotReader.Store(true)
time.AfterFunc(2*time.Minute, func() {
cancel()
})
data, err := io.ReadAll(reader)
if err != nil {
// Err handling....
log.Error(err)
return
}
```
I'm not sure what the best way to modify the API would be. You're unfortunately stuck with the io.Reader interface, without the ability to add a context. Thoughts?
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.