fix(timeout): accept Go duration strings and report timeouts clearly (#12)#16
Merged
Merged
Conversation
The --timeout/-t flag parsed its value as `time.ParseDuration(value + "s")`,
unconditionally appending "s". That only worked for bare-second numbers and
mishandled the duration strings the flag is meant to take:
--timeout 1m -> ParseDuration("1ms") = 1 millisecond (silently wrong!)
--timeout 500ms -> error ("500mss")
--timeout 5s -> error ("5ss")
Add a parseTimeout helper that accepts both forms: a bare number is treated
as seconds (keeping the documented `--timeout <seconds>` default), and any
other value is parsed as a Go duration string (5s, 1m, 500ms, 1m30s).
Negative and unparseable values now produce a clear error.
Also surface a readable message when a request actually times out, instead
of the raw `context deadline exceeded` transport error:
request timed out after 30s (increase or disable with --timeout)
Adds CLI-parser tests (incl. a regression test that "1m" == 1 minute) and
client tests covering a firing timeout and a request that completes in time.
Updates the --timeout help text. No change to the default (30s) or to the
saved-request path.
Closes #12
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes #12.
The
--timeout/-tflag parsed its value withtime.ParseDuration(value + "s"), unconditionally appendings. That only worked for bare-second numbers and mishandled the duration strings the issue explicitly asks to support:--timeout 3030s✓30s✓ (unchanged)--timeout 1m1ms— 1 millisecond, silently wrong1m✓--timeout 5s5ss)5s✓--timeout 500ms500mss)500ms✓--timeout notadurationinvalid timeout "notaduration": want a number of seconds or a duration like 5s, 1m, 500msChanges
parseTimeouthelper (main.go): a bare number is treated as seconds (keeps the documented--timeout <seconds>default), anything else is parsed as a Go duration string (5s,1m,500ms,1m30s). Negative / unparseable values now error clearly.client/client.go): when a request actually times out, surfacerequest timed out after 30s (increase or disable with --timeout)instead of the rawcontext deadline exceededtransport error.--timeouthelp text.run) path.Maps to the issue's acceptance criteria:
--timeoutflag accepts duration stringsType of change
How Has This Been Tested?
main_test.go, incl. a regression test that1m== 1 minute, not 1ms) and client timeout behaviour (client/timeout_test.go, a firing timeout + a request that completes in time).go test ./...locally — all packages pass.kurl --timeout 500ms …→ workskurl --timeout 1ms https://example.com→request timed out after 1ms (increase or disable with --timeout)kurl --timeout notaduration …→ clear validation errorChecklist
gofmt,go vetclean)