coder / coder/websocket

Document how to have read timeout be larger than idle timeout

Open
#425 3 comments 0 reactions 0 assignees View on GitHub
docs
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.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.