Skip to content

Go: GlideClientConfiguration requestTimeout is using wrong measurement #3931

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

Closed
jbrinkman opened this issue May 21, 2025 · 1 comment · Fixed by #3933
Closed

Go: GlideClientConfiguration requestTimeout is using wrong measurement #3931

jbrinkman opened this issue May 21, 2025 · 1 comment · Fixed by #3933
Assignees
Labels
bug Something isn't working go golang wrapper
Milestone

Comments

@jbrinkman
Copy link
Collaborator

Describe the bug

RequestTimeout field on GlideClientConfiguration was recently updated to use a time.Duration to represent the timeout value. Prior to the change, the value was an int and documented to contain a value representing milliseconds. A time.Duration struct represents time as nanoseconds:

// A Duration represents the elapsed time between two instants
// as an int64 nanosecond count. The representation limits the
// largest representable duration to approximately 290 years.
type Duration int64

const (
	minDuration Duration = -1 << 63
	maxDuration Duration = 1<<63 - 1
)

The Duration value needs to be converted from nanoseconds to milliseconds in the toProtobuf implementation and should be validated in the WithRequestTimeout function to ensure it does not exceed the bounds of the uint32 type so the narrowing cast in toProtobuf does not result in an error.

Expected Behavior

RequestTimeout is expected to represent milliseconds in Glide-core. The time.Duration type should be appropriately constrained and converted to ensure proper operation.

Current Behavior

RequestTimeout value is improperly converted, and has undefined behavior for some Duration values.

Reproduction Steps

See description.

Possible Solution

No response

Additional Information/Context

No response

Client version used

2.0

Engine type and version

Any

OS

Any

Language

Go

Language Version

1.22

Cluster information

No response

Logs

No response

Other information

No response

@jbrinkman jbrinkman added the bug Something isn't working label May 21, 2025
@jbrinkman jbrinkman added this to the 2.0 milestone May 21, 2025
@jbrinkman
Copy link
Collaborator Author

This also affects the connectionTimeout on AdvancedGlideClientConfiguration and should be fixed there as well.

@yipin-chen yipin-chen moved this to In Progress in Valkey-GLIDE - internal May 23, 2025
@yipin-chen yipin-chen moved this from In Progress to Review in Valkey-GLIDE - internal May 23, 2025
@avifenesh avifenesh added the go golang wrapper label May 29, 2025
@github-project-automation github-project-automation bot moved this from Review to Done in Valkey-GLIDE - internal May 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working go golang wrapper
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants