Skip to content

Commit 130855b

Browse files
mrexoxmrexox
authored
chore: timeout cleanup (#1297)
1 parent e6adbba commit 130855b

File tree

13 files changed

+88
-178
lines changed

13 files changed

+88
-178
lines changed

‎internal/config/command.go‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,16 +5,17 @@ import (
55
"errors"
66
"slices"
77
"strings"
8+
"time"
89
)
910

1011
var ErrFilesIncompatible = errors.New("one of your runners contains incompatible file types")
1112

1213
type Command struct {
13-
Run string `json:"run" mapstructure:"run" toml:"run" yaml:"run"`
14-
Files string `json:"files,omitempty" mapstructure:"files" toml:"files,omitempty" yaml:",omitempty"`
15-
Root string `json:"root,omitempty" mapstructure:"root" toml:"root,omitempty" yaml:",omitempty"`
16-
FailText string `json:"fail_text,omitempty" koanf:"fail_text" mapstructure:"fail_text" toml:"fail_text,omitempty" yaml:"fail_text,omitempty"`
17-
Timeout string `json:"timeout,omitempty" mapstructure:"timeout" toml:"timeout,omitempty" yaml:",omitempty"`
14+
Run string `json:"run" mapstructure:"run" toml:"run" yaml:"run"`
15+
Files string `json:"files,omitempty" mapstructure:"files" toml:"files,omitempty" yaml:",omitempty"`
16+
Root string `json:"root,omitempty" mapstructure:"root" toml:"root,omitempty" yaml:",omitempty"`
17+
FailText string `json:"fail_text,omitempty" koanf:"fail_text" mapstructure:"fail_text" toml:"fail_text,omitempty" yaml:"fail_text,omitempty"`
18+
Timeout time.Duration `json:"timeout,omitempty" jsonschema:"type=string,example=15s" mapstructure:"timeout" toml:"timeout,omitempty" yaml:",omitempty"`
1819

1920
Skip any `json:"skip,omitempty" jsonschema:"oneof_type=boolean;array" mapstructure:"skip" toml:"skip,omitempty,inline" yaml:",omitempty"`
2021
Only any `json:"only,omitempty" jsonschema:"oneof_type=boolean;array" mapstructure:"only" toml:"only,omitempty,inline" yaml:",omitempty"`

‎internal/config/command_test.go‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package config
22

33
import (
44
"testing"
5+
"time"
56

67
"github.com/stretchr/testify/assert"
78
)
@@ -44,19 +45,19 @@ func TestCommandsToJobsWithTimeout(t *testing.T) {
4445
commands := map[string]*Command{
4546
"lint": {
4647
Run: "echo lint",
47-
Timeout: "60s",
48+
Timeout: 60 * time.Second,
4849
Priority: 1,
4950
},
5051
"test": {
5152
Run: "echo test",
52-
Timeout: "5m",
53+
Timeout: 5 * time.Minute,
5354
},
5455
}
5556

5657
jobs := CommandsToJobs(commands)
5758

5859
assert.Equal(t, jobs, []*Job{
59-
{Name: "lint", Run: "echo lint", Timeout: "60s"},
60-
{Name: "test", Run: "echo test", Timeout: "5m"},
60+
{Name: "lint", Run: "echo lint", Timeout: 60 * time.Second},
61+
{Name: "test", Run: "echo test", Timeout: 5 * time.Minute},
6162
})
6263
}

‎internal/config/job.go‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,17 @@
11
package config
22

3+
import "time"
4+
35
type Job struct {
4-
Name string `json:"name,omitempty" mapstructure:"name" toml:"name,omitempty" yaml:",omitempty"`
5-
Run string `json:"run,omitempty" jsonschema:"oneof_required=Run a command" mapstructure:"run" toml:"run,omitempty" yaml:",omitempty"`
6-
Script string `json:"script,omitempty" jsonschema:"oneof_required=Run a script" mapstructure:"script" toml:"script,omitempty" yaml:",omitempty"`
7-
Runner string `json:"runner,omitempty" mapstructure:"runner" toml:"runner,omitempty" yaml:",omitempty"`
8-
Args string `json:"args,omitempty" mapstructure:"args" toml:"args,omitempty" yaml:",omitempty"`
9-
Root string `json:"root,omitempty" mapstructure:"root" toml:"root,omitempty" yaml:",omitempty"`
10-
Files string `json:"files,omitempty" mapstructure:"files" toml:"files,omitempty" yaml:",omitempty"`
11-
FailText string `json:"fail_text,omitempty" koanf:"fail_text" mapstructure:"fail_text" toml:"fail_text,omitempty" yaml:"fail_text,omitempty"`
12-
Timeout string `json:"timeout,omitempty" mapstructure:"timeout" toml:"timeout,omitempty" yaml:",omitempty"`
6+
Name string `json:"name,omitempty" mapstructure:"name" toml:"name,omitempty" yaml:",omitempty"`
7+
Run string `json:"run,omitempty" jsonschema:"oneof_required=Run a command" mapstructure:"run" toml:"run,omitempty" yaml:",omitempty"`
8+
Script string `json:"script,omitempty" jsonschema:"oneof_required=Run a script" mapstructure:"script" toml:"script,omitempty" yaml:",omitempty"`
9+
Runner string `json:"runner,omitempty" mapstructure:"runner" toml:"runner,omitempty" yaml:",omitempty"`
10+
Args string `json:"args,omitempty" mapstructure:"args" toml:"args,omitempty" yaml:",omitempty"`
11+
Root string `json:"root,omitempty" mapstructure:"root" toml:"root,omitempty" yaml:",omitempty"`
12+
Files string `json:"files,omitempty" mapstructure:"files" toml:"files,omitempty" yaml:",omitempty"`
13+
FailText string `json:"fail_text,omitempty" koanf:"fail_text" mapstructure:"fail_text" toml:"fail_text,omitempty" yaml:"fail_text,omitempty"`
14+
Timeout time.Duration `json:"timeout,omitempty" jsonschema:"type=string,example=15s" mapstructure:"timeout" toml:"timeout,omitempty" yaml:",omitempty"`
1315

1416
Glob []string `json:"glob,omitempty" jsonschema:"oneof_type=string;array" mapstructure:"glob" toml:"glob,omitempty" yaml:",omitempty"`
1517
Exclude []string `json:"exclude,omitempty" jsonschema:"oneof_type=string;array" mapstructure:"exclude" toml:"exclude,omitempty" yaml:",omitempty"`

‎internal/config/jsonschema.json‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,12 @@
1616
"fail_text": {
1717
"type": "string"
1818
},
19+
"timeout": {
20+
"type": "string",
21+
"examples": [
22+
"15s"
23+
]
24+
},
1925
"skip": {
2026
"oneOf": [
2127
{
@@ -267,6 +273,12 @@
267273
"fail_text": {
268274
"type": "string"
269275
},
276+
"timeout": {
277+
"type": "string",
278+
"examples": [
279+
"15s"
280+
]
281+
},
270282
"glob": {
271283
"oneOf": [
272284
{
@@ -442,6 +454,12 @@
442454
"fail_text": {
443455
"type": "string"
444456
},
457+
"timeout": {
458+
"type": "string",
459+
"examples": [
460+
"15s"
461+
]
462+
},
445463
"interactive": {
446464
"type": "boolean"
447465
},
@@ -456,7 +474,7 @@
456474
"type": "object"
457475
}
458476
},
459-
"$comment": "Last updated on 2026.01.20.",
477+
"$comment": "Last updated on 2026.01.27.",
460478
"properties": {
461479
"min_version": {
462480
"type": "string",

‎internal/config/script.go‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"slices"
66
"strconv"
77
"strings"
8+
"time"
89
"unicode"
910
)
1011

@@ -18,11 +19,11 @@ type Script struct {
1819
Env map[string]string `json:"env,omitempty" mapstructure:"env" toml:"env,omitempty" yaml:",omitempty"`
1920
Priority int `json:"priority,omitempty" mapstructure:"priority" toml:"priority,omitempty" yaml:",omitempty"`
2021

21-
FailText string `json:"fail_text,omitempty" koanf:"fail_text" mapstructure:"fail_text" toml:"fail_text,omitempty" yaml:"fail_text,omitempty"`
22-
Timeout string `json:"timeout,omitempty" mapstructure:"timeout" toml:"timeout,omitempty" yaml:",omitempty"`
23-
Interactive bool `json:"interactive,omitempty" mapstructure:"interactive" toml:"interactive,omitempty" yaml:",omitempty"`
24-
UseStdin bool `json:"use_stdin,omitempty" koanf:"use_stdin" mapstructure:"use_stdin" toml:"use_stdin,omitempty" yaml:"use_stdin,omitempty"`
25-
StageFixed bool `json:"stage_fixed,omitempty" koanf:"stage_fixed" mapstructure:"stage_fixed" toml:"stage_fixed,omitempty" yaml:"stage_fixed,omitempty"`
22+
FailText string `json:"fail_text,omitempty" koanf:"fail_text" mapstructure:"fail_text" toml:"fail_text,omitempty" yaml:"fail_text,omitempty"`
23+
Timeout time.Duration `json:"timeout,omitempty" jsonschema:"type=string,example=15s" mapstructure:"timeout" toml:"timeout,omitempty" yaml:",omitempty"`
24+
Interactive bool `json:"interactive,omitempty" mapstructure:"interactive" toml:"interactive,omitempty" yaml:",omitempty"`
25+
UseStdin bool `json:"use_stdin,omitempty" koanf:"use_stdin" mapstructure:"use_stdin" toml:"use_stdin,omitempty" yaml:"use_stdin,omitempty"`
26+
StageFixed bool `json:"stage_fixed,omitempty" koanf:"stage_fixed" mapstructure:"stage_fixed" toml:"stage_fixed,omitempty" yaml:"stage_fixed,omitempty"`
2627
}
2728

2829
func ScriptsToJobs(scripts map[string]*Script) []*Job {

‎internal/config/script_test.go‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package config
22

33
import (
44
"testing"
5+
"time"
56

67
"github.com/stretchr/testify/assert"
78
)
@@ -44,19 +45,19 @@ func TestScriptsToJobsWithTimeout(t *testing.T) {
4445
scripts := map[string]*Script{
4546
"lint.sh": {
4647
Runner: "bash",
47-
Timeout: "30s",
48+
Timeout: 30 * time.Second,
4849
Priority: 1,
4950
},
5051
"test.sh": {
5152
Runner: "bash",
52-
Timeout: "10m",
53+
Timeout: 10 * time.Minute,
5354
},
5455
}
5556

5657
jobs := ScriptsToJobs(scripts)
5758

5859
assert.Equal(t, jobs, []*Job{
59-
{Name: "lint.sh", Script: "lint.sh", Runner: "bash", Timeout: "30s"},
60-
{Name: "test.sh", Script: "test.sh", Runner: "bash", Timeout: "10m"},
60+
{Name: "lint.sh", Script: "lint.sh", Runner: "bash", Timeout: 30 * time.Second},
61+
{Name: "test.sh", Script: "test.sh", Runner: "bash", Timeout: 10 * time.Minute},
6162
})
6263
}

‎internal/run/controller/exec/executor.go‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ type Options struct {
1010
Root string
1111
Commands []string
1212
Env map[string]string
13-
Timeout string
1413
Interactive, UseStdin bool
1514
}
1615

‎internal/run/controller/exec/executor_test.go‎

Lines changed: 0 additions & 36 deletions
This file was deleted.

‎internal/run/controller/job.go‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -129,22 +129,28 @@ func (c *Controller) runSingleJob(ctx context.Context, scope *scope, id string,
129129

130130
env := maps.Clone(scope.env)
131131
maps.Copy(env, job.Env)
132-
ok, failText := c.run(ctx, strings.Join(append(scope.names, name), " ❯ "), scope.follow, exec.Options{
132+
133+
if job.Timeout > 0 {
134+
var cancel context.CancelFunc
135+
ctx, cancel = context.WithTimeout(ctx, job.Timeout)
136+
defer cancel()
137+
}
138+
err = c.run(ctx, strings.Join(append(scope.names, name), " ❯ "), scope.follow, exec.Options{
133139
Root: filepath.Join(c.git.RootPath, scope.root),
134140
Commands: commands,
135141
Interactive: job.Interactive && !scope.opts.DisableTTY,
136142
UseStdin: job.UseStdin,
137-
Timeout: job.Timeout,
138143
Env: env,
139144
})
140145

141146
executionTime := time.Since(startTime)
142147

143-
if !ok {
144-
if failText == "" {
145-
failText = job.FailText
148+
if err != nil {
149+
if ctx.Err() == context.DeadlineExceeded {
150+
return result.Failure(name, "timeout ("+job.Timeout.String()+")", executionTime)
146151
}
147-
return result.Failure(name, failText, executionTime)
152+
153+
return result.Failure(name, job.FailText, executionTime)
148154
}
149155

150156
if config.HookUsesStagedFiles(scope.hookName) && job.StageFixed && !scope.opts.NoStageFixed {

‎internal/run/controller/run.go‎

Lines changed: 3 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -5,31 +5,16 @@ import (
55
"context"
66
"io"
77
"os"
8-
"time"
98

109
"github.com/evilmartians/lefthook/v2/internal/log"
1110
"github.com/evilmartians/lefthook/v2/internal/run/controller/exec"
1211
"github.com/evilmartians/lefthook/v2/internal/system"
1312
)
1413

15-
func (c *Controller) run(ctx context.Context, name string, follow bool, opts exec.Options) (ok bool, failText string) {
14+
func (c *Controller) run(ctx context.Context, name string, follow bool, opts exec.Options) error {
1615
log.SetName(name)
1716
defer log.UnsetName(name)
1817

19-
// Apply timeout if specified
20-
var timeoutDuration string
21-
if opts.Timeout != "" {
22-
timeout, err := parseDuration(opts.Timeout)
23-
if err != nil {
24-
log.Errorf("invalid timeout format '%s': %s\n", opts.Timeout, err)
25-
return false, ""
26-
}
27-
timeoutDuration = opts.Timeout
28-
var cancel context.CancelFunc
29-
ctx, cancel = context.WithTimeout(ctx, timeout)
30-
defer cancel()
31-
}
32-
3318
// If the command does not explicitly `use_stdin` no input will be provided.
3419
var in io.Reader = system.NullReader
3520
if opts.UseStdin {
@@ -46,27 +31,12 @@ func (c *Controller) run(ctx context.Context, name string, follow bool, opts exe
4631
out = io.Discard
4732
}
4833

49-
err := c.executor.Execute(ctx, opts, in, out)
50-
51-
if err != nil && ctx.Err() == context.DeadlineExceeded {
52-
return false, "timeout (" + timeoutDuration + ")"
53-
}
54-
return err == nil, ""
34+
return c.executor.Execute(ctx, opts, in, out)
5535
}
5636

5737
out := new(bytes.Buffer)
58-
5938
err := c.executor.Execute(ctx, opts, in, out)
60-
6139
log.Execution(name, err, out)
6240

63-
if err != nil && ctx.Err() == context.DeadlineExceeded {
64-
return false, "timeout (" + timeoutDuration + ")"
65-
}
66-
return err == nil, ""
67-
}
68-
69-
// parseDuration parses a duration string (e.g., "60s", "5m", "1h30m").
70-
func parseDuration(duration string) (time.Duration, error) {
71-
return time.ParseDuration(duration)
41+
return err
7242
}

0 commit comments

Comments
 (0)