Make HTTP timeout configurable, raise default to 30s#52
Conversation
The previous 2-second hardcoded timeout in libauth was too short for slow auth-server responses (e.g. ~16s observed at FIT building, see issue z4yx#26). Expose libauth.HttpTimeout and add a --timeout / -t flag and "timeout" config-file field so users can tune it for their network. https://claude.ai/code/session_01Az7YBQcoUBUSZCDQZ23kRc
Reduce the default HTTP timeout from 30 seconds to 2 seconds.
| &cli.StringFlag{Name: "config-file", Aliases: []string{"c"}, Usage: "`path` to your config file, default ~/.auth-thu"}, | ||
| &cli.StringFlag{Name: "hook-success", Usage: "command line to be executed in shell after successful login/out"}, | ||
| &cli.IntFlag{Name: "online-interval", Aliases: []string{"I"}, Usage: "the interval between each keepAlive request (s)", Value: 3}, | ||
| &cli.IntFlag{Name: "timeout", Aliases: []string{"t"}, Usage: "HTTP request timeout in seconds for the auth server", Value: 30}, |
There was a problem hiding this comment.
This default should be change to 2s as well?
There was a problem hiding this comment.
Reduced the HTTP request timeout for the auth server from 30 seconds to 2 seconds.
There was a problem hiding this comment.
Pull request overview
This PR aims to make libauth’s HTTP request timeout configurable from the CLI/config file to better handle slow auth-server responses (per issue #26), rather than relying on a hardcoded short timeout.
Changes:
- Introduces an exported
libauth.HttpTimeoutused bylibauthHTTP clients. - Adds a
timeoutsetting in the CLI config and a--timeout/-tCLI flag to setlibauth.HttpTimeout.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| libauth/requests.go | Replaces hardcoded HTTP client timeouts with a shared HttpTimeout variable. |
| cli/main.go | Adds config/flag plumbing to set libauth.HttpTimeout from user-provided timeout seconds. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // HttpTimeout is the timeout used for all HTTP requests issued by libauth. | ||
| // It can be overridden by the caller (e.g. the CLI) before making requests. | ||
| var HttpTimeout = 2 * time.Second |
| &cli.StringFlag{Name: "config-file", Aliases: []string{"c"}, Usage: "`path` to your config file, default ~/.auth-thu"}, | ||
| &cli.StringFlag{Name: "hook-success", Usage: "command line to be executed in shell after successful login/out"}, | ||
| &cli.IntFlag{Name: "online-interval", Aliases: []string{"I"}, Usage: "the interval between each keepAlive request (s)", Value: 3}, | ||
| &cli.IntFlag{Name: "timeout", Aliases: []string{"t"}, Usage: "HTTP request timeout in seconds for the auth server", Value: 2}, |
| merged.Timeout = c.Int("timeout") | ||
| if !c.IsSet("timeout") && settings.Timeout != 0 { | ||
| merged.Timeout = settings.Timeout | ||
| } | ||
| settings = merged | ||
| if settings.Timeout > 0 { |
There was a problem hiding this comment.
这个感觉有点意义不明，命令行参数允许--timeout 0，但是配置文件的timeout: 0是无效的会被忽略，感觉这样才更迷惑了。
其实完全可以不改，explicit "--timeout 0" will be silently ignored本来也不算什么大问题
|
都在这一个地方讨论吧，不然太分散了
我感觉很奇怪。首先是16秒整，而且稳定，这就说明不是网络的随机波动导致的，而是触发了某种确定性的行为。 总之我感觉这个现象是很奇怪的 |
The previous 2-second hardcoded timeout in libauth was too short for slow auth-server responses (e.g. ~16s observed at FIT building, see issue #26). Expose libauth.HttpTimeout and add a --timeout / -t flag and "timeout" config-file field so users can tune it for their network.
https://claude.ai/code/session_01Az7YBQcoUBUSZCDQZ23kRc