-
Notifications
You must be signed in to change notification settings - Fork 60
fix: use wrapped context's err when <-ctx.Done() case is executed #463
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -14,6 +14,8 @@ Versioning](http://semver.org/spec/v2.0.0.html) except to the first release. | |||||
|
||||||
### Fixed | ||||||
|
||||||
- Use wrapped context err when <-ctx.Done() case is executed | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please, add a link to the issue in the changelog entry.
Suggested change
|
||||||
|
||||||
## [v2.4.0] - 2025-07-11 | ||||||
|
||||||
This release focuses on adding schema/user/session operations, synchronous transaction | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -984,7 +984,10 @@ func (conn *Connection) newFuture(req Request) (fut *Future) { | |
if ctx != nil { | ||
select { | ||
case <-ctx.Done(): | ||
fut.SetError(fmt.Errorf("context is done (request ID %d)", fut.requestId)) | ||
fut.SetError(fmt.Errorf( | ||
"context is done (request ID %d): %w", | ||
fut.requestId, context.Cause(ctx), | ||
)) | ||
Comment on lines
+987
to
+990
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please, add a test that we could check the context error existence with |
||
shard.rmut.Unlock() | ||
return | ||
default: | ||
|
@@ -1026,7 +1029,10 @@ func (conn *Connection) contextWatchdog(fut *Future, ctx context.Context) { | |
case <-fut.done: | ||
return | ||
default: | ||
conn.cancelFuture(fut, fmt.Errorf("context is done (request ID %d)", fut.requestId)) | ||
conn.cancelFuture(fut, fmt.Errorf( | ||
"context is done (request ID %d): %w", | ||
fut.requestId, context.Cause(ctx), | ||
)) | ||
} | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,7 +16,7 @@ import ( | |
type Tuple struct { | ||
// Instruct msgpack to pack this struct as array, so no custom packer | ||
// is needed. | ||
_msgpack struct{} `msgpack:",asArray"` //nolint: structcheck,unused | ||
_msgpack struct{} `msgpack:",asArray"` // nolint: structcheck,unused | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please, revert the unnecessary change. |
||
Id uint | ||
Msg string | ||
Name string | ||
|
@@ -167,7 +167,7 @@ func ExamplePingRequest_Context() { | |
fmt.Println("Ping Error", regexp.MustCompile("[0-9]+").ReplaceAllString(err.Error(), "N")) | ||
// Output: | ||
// Ping Resp data [] | ||
// Ping Error context is done (request ID N) | ||
// Ping Error context is done (request ID N): context deadline exceeded | ||
} | ||
|
||
func ExampleSelectRequest() { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -48,7 +48,7 @@ type Member struct { | |
Val uint | ||
} | ||
|
||
var contextDoneErrRegexp = regexp.MustCompile(`^context is done \(request ID [0-9]+\)$`) | ||
var contextDoneErrRegexp = regexp.MustCompile(`^context is done \(request ID [0-9]+\):.+$`) | ||
|
||
func (m *Member) EncodeMsgpack(e *msgpack.Encoder) error { | ||
if err := e.EncodeArrayLen(2); err != nil { | ||
|
@@ -89,8 +89,8 @@ var indexNo = uint32(0) | |
var indexName = "primary" | ||
var opts = Opts{ | ||
Timeout: 5 * time.Second, | ||
//Concurrency: 32, | ||
//RateLimit: 4*1024, | ||
// Concurrency: 32, | ||
// RateLimit: 4*1024, | ||
Comment on lines
+92
to
+93
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please, revert the unnecessary change. |
||
} | ||
|
||
const N = 500 | ||
|
@@ -888,7 +888,7 @@ func TestFutureMultipleGetTypedWithError(t *testing.T) { | |
} | ||
} | ||
|
||
/////////////////// | ||
// ///////////////// | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please, revert the unnecessary change. |
||
|
||
func TestClient(t *testing.T) { | ||
var err error | ||
|
@@ -2716,7 +2716,7 @@ func TestCallRequest(t *testing.T) { | |
func TestClientRequestObjectsWithNilContext(t *testing.T) { | ||
conn := test_helpers.ConnectWithValidation(t, dialer, opts) | ||
defer conn.Close() | ||
req := NewPingRequest().Context(nil) //nolint | ||
req := NewPingRequest().Context(nil) // nolint | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please, revert the unnecessary change. |
||
data, err := conn.Do(req).Get() | ||
if err != nil { | ||
t.Fatalf("Failed to Ping: %s", err) | ||
|
@@ -3269,7 +3269,7 @@ func TestClientIdRequestObjectWithNilContext(t *testing.T) { | |
req := NewIdRequest(ProtocolInfo{ | ||
Version: ProtocolVersion(1), | ||
Features: []iproto.Feature{iproto.IPROTO_FEATURE_STREAMS}, | ||
}).Context(nil) //nolint | ||
}).Context(nil) // nolint | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please, revert the unnecessary change. |
||
data, err := conn.Do(req).Get() | ||
require.Nilf(t, err, "No errors on Id request execution") | ||
require.NotNilf(t, data, "Response data not empty") | ||
|
@@ -3293,7 +3293,7 @@ func TestClientIdRequestObjectWithPassedCanceledContext(t *testing.T) { | |
req := NewIdRequest(ProtocolInfo{ | ||
Version: ProtocolVersion(1), | ||
Features: []iproto.Feature{iproto.IPROTO_FEATURE_STREAMS}, | ||
}).Context(ctx) //nolint | ||
}).Context(ctx) // nolint | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please, revert the unnecessary change. |
||
cancel() | ||
resp, err := conn.Do(req).Get() | ||
require.Nilf(t, resp, "Response is empty") | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please, move it into
Changed
orAdded
section of the CHANGELOG.md. It does not fixes anything, but looks like an improvment.