Skip to content

Commit 79ea9be

Browse files
authored
Merge pull request #30 from xraph/fix/client-expired-context
fix(client): honour an already-cancelled context before sending
2 parents b529178 + fdfc5ab commit 79ea9be

1 file changed

Lines changed: 15 additions & 0 deletions

File tree

client/client.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,21 @@ func (c *Client) tryReconnect() {
262262

263263
// request sends a request frame and waits for the correlated response.
264264
func (c *Client) request(ctx context.Context, method string, data any) (*dwp.Frame, error) {
265+
// Checked before anything is marshalled or sent, for two reasons.
266+
// A caller whose deadline has already blown should not still cause a
267+
// side effect on the server, and writeFrame below does not consult
268+
// ctx at all, so without this an expired context still enqueues the
269+
// job. The select at the end of this function cannot stand in for the
270+
// check either: once the response has landed in respCh, both of its
271+
// cases are ready, and Go picks between ready cases uniformly at
272+
// random, so an expired context loses roughly half the time. That is
273+
// what made TestClient_ContextTimeout flake on Linux CI, where the
274+
// read goroutine can deliver the response before this goroutine
275+
// reaches the select.
276+
if err := ctx.Err(); err != nil {
277+
return nil, err
278+
}
279+
265280
frame := &dwp.Frame{
266281
ID: dwp.GenerateFrameID(),
267282
Type: dwp.FrameRequest,

0 commit comments

Comments
 (0)