Skip to content

Update commit timeout #121

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.

Sign up for GitHub

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

Merged
marstr merged 2 commits into from
Apr 1, 2026
Merged

Update commit timeout #121

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0fbfd05
Updating dependencies
marstr Apr 1, 2026
d909dab
Have commit command use rootContext
marstr Apr 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions cmd/commit.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,11 +122,8 @@ var commitCmd = &cobra.Command{
return err
}
} else {
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()

var err error
commitTransactionFromFlags.Amount, err = calculateAmount(ctx, ".")
commitTransactionFromFlags.Amount, err = calculateAmount(rootContext, ".")
if err != nil {
Comment on lines 125 to 127

Copilot AI Apr 1, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change won’t actually make calculateAmount respect the global --timeout value, because calculateAmount wraps the provided context with its own hard-coded 2*time.Second timeout. As a result, the operation can still time out at 2s even if rootContext has a larger deadline (and the previous 5s wrapper didn’t matter either). Consider removing the internal fixed timeout or deriving it from the remaining time/deadline of the incoming context.

Copilot uses AI. Check for mistakes.
logrus.Fatalf("Failed to calculate the amount from %q because of the following error: %s", amountDefault, err)
}
Comment on lines 124 to 129

Copilot AI Apr 1, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rootContext is used during Args validation, but it is only initialized by calling RootContext(cmd) (currently done in Run). If Args runs first (typical for Cobra), rootContext will be nil here and calculateAmount will panic when it calls context.WithTimeout on a nil context. Initialize a context in Args via RootContext(cmd) (and if you need cancellation, cancel only a derived child context so you don’t cancel the shared root context).

Copilot uses AI. Check for mistakes.
Expand Down
1 change: 0 additions & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ require (
github.com/spf13/afero v1.1.2 // indirect
github.com/spf13/jwalterweatherman v1.0.0 // indirect
github.com/spf13/pflag v1.0.6 // indirect

Copilot AI Apr 1, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR’s stated purpose is a timeout behavior change, but it also removes the indirect golang.org/x/crypto requirement. If this is just go mod tidy churn, consider reverting/splitting it to keep the review focused; otherwise, please document why the dependency graph change is required for the timeout fix.

Suggested change
github.com/spf13/pflag v1.0.6 // indirect
github.com/spf13/pflag v1.0.6 // indirect
golang.org/x/crypto v0.31.0 // indirect

Copilot uses AI. Check for mistakes.
golang.org/x/crypto v0.45.0 // indirect
golang.org/x/sys v0.38.0 // indirect
golang.org/x/text v0.31.0 // indirect
gopkg.in/yaml.v2 v2.2.8 // indirect
Expand Down
4 changes: 1 addition & 3 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,8 @@ github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/
github.com/ugorji/go/codec v0.0.0-20181204163529-d75b2dcb6bc8/go.mod h1:VFNgLljTbGfSG7qAOspJ7OScBnGdDN/yBr0sguwnwf0=
github.com/xordataexchange/crypt v0.0.3-0.20170626215501-b2862e3d0a77/go.mod h1:aYKd//L2LvnjZzWKhF00oedf4jCCReLcmhLdhm1A27Q=
golang.org/x/crypto v0.0.0-20181203042331-505ab145d0a9/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4=
golang.org/x/crypto v0.45.0 h1:jMBrvKuj23MTlT0bQEOBcAE0mjg8mK9RXFhRH6nyF3Q=
golang.org/x/crypto v0.45.0/go.mod h1:XTGrrkGJve7CYK7J8PEww4aY7gM3qMCElcJQ8n8JdX4=
golang.org/x/sys v0.0.0-20180905080454-ebe1bf3edb33/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20181205085412-a5c9d58dba9a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.38.0 h1:3yZWxaJjBmCWXqhN1qh02AkOnCQ1poK6oF+a7xWL6Gc=
golang.org/x/sys v0.38.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks=
golang.org/x/term v0.37.0 h1:8EGAD0qCmHYZg6J17DvsMy9/wJ7/D/4pV/wfnld5lTU=
Expand Down
Loading