Skip to content

Commit 0f7c283

Browse files
committed
runner: improve error logging
This clarifies some error messages, and provides the stack trace when taskrunner fails on execution.
1 parent cdce125 commit 0f7c283

5 files changed

Lines changed: 87 additions & 4 deletions

File tree

executor.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -314,7 +314,11 @@ func (e *Executor) ShellRun(ctx context.Context, command string, opts ...shell.R
314314
}
315315
options = append(options, e.shellRunOptions...)
316316
options = append(options, opts...)
317-
return shell.Run(ctx, command, options...)
317+
err := shell.Run(ctx, command, options...)
318+
if err != nil {
319+
return oops.Wrapf(err, "Executor failed to run shell command")
320+
}
321+
return nil
318322
}
319323

320324
func (e *Executor) taskExecution(t *Task) *taskExecution { return e.tasks[t] }
@@ -685,7 +689,7 @@ func (e *Executor) runPass() {
685689

686690
e.runPass()
687691
if err != nil && ctx.Err() != context.Canceled {
688-
return err
692+
return oops.Wrapf(err, "Executor failed to run task")
689693
}
690694

691695
return nil

executor_test.go

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package taskrunner_test
22

33
import (
44
"context"
5+
"errors"
56
"testing"
67
"time"
78

@@ -194,3 +195,63 @@ func TestExecutorInvalidations(t *testing.T) {
194195
mockB.Reset()
195196
}
196197
}
198+
199+
func TestExecutorErrorHandling(t *testing.T) {
200+
config := &config.Config{}
201+
202+
for _, testcase := range []struct {
203+
Name string
204+
Test func(t *testing.T)
205+
}{
206+
{
207+
"shell command error",
208+
func(t *testing.T) {
209+
executor := taskrunner.NewExecutor(config, []*taskrunner.Task{
210+
{
211+
Name: "shell-error",
212+
Run: func(ctx context.Context, shellRun shell.ShellRun) error {
213+
return shellRun(ctx, "invalid_command_that_will_fail")
214+
},
215+
},
216+
})
217+
218+
events := executor.Subscribe()
219+
go func() {
220+
event := consumeUntil(t, events, taskrunner.ExecutorEventKind_TaskFailed)
221+
assert.Contains(t, event.(*taskrunner.TaskFailedEvent).Error.Error(),
222+
"Executor failed to run shell command")
223+
}()
224+
225+
err := executor.Run(context.Background(), []string{"shell-error"}, &taskrunner.Runtime{})
226+
assert.Error(t, err)
227+
assert.Contains(t, err.Error(), "Executor failed to run task")
228+
},
229+
},
230+
{
231+
"task execution error",
232+
func(t *testing.T) {
233+
executor := taskrunner.NewExecutor(config, []*taskrunner.Task{
234+
{
235+
Name: "failing-task",
236+
Run: func(ctx context.Context, shellRun shell.ShellRun) error {
237+
return errors.New("task failed")
238+
},
239+
},
240+
})
241+
242+
events := executor.Subscribe()
243+
go func() {
244+
event := consumeUntil(t, events, taskrunner.ExecutorEventKind_TaskFailed)
245+
assert.Contains(t, event.(*taskrunner.TaskFailedEvent).Error.Error(),
246+
"task failed")
247+
}()
248+
249+
err := executor.Run(context.Background(), []string{"failing-task"}, &taskrunner.Runtime{})
250+
assert.Error(t, err)
251+
assert.Contains(t, err.Error(), "Executor failed to run task")
252+
},
253+
},
254+
} {
255+
t.Run(testcase.Name, testcase.Test)
256+
}
257+
}

runner.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -264,7 +264,7 @@ func Run(options ...RunOption) {
264264
// if it's a well-known executor error, or the underlying task failed AND
265265
// we're not in watch mode.
266266
if oops.Cause(err) == errUndefinedTaskName || !watchMode {
267-
return err
267+
return oops.Wrapf(err, "Runner failed to run task")
268268
}
269269

270270
return nil

shell/shell.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"io"
77
"strings"
88

9+
"github.com/samsarahq/go/oops"
910
"mvdan.cc/sh/interp"
1011
"mvdan.cc/sh/syntax"
1112
)
@@ -48,7 +49,7 @@ func Env(vars map[string]string) RunOption {
4849
func Run(ctx context.Context, command string, opts ...RunOption) error {
4950
p, err := syntax.NewParser().Parse(strings.NewReader(command), "")
5051
if err != nil {
51-
return err
52+
return oops.Wrapf(err, "failed to parse shell command")
5253
}
5354

5455
r, err := interp.New()

shell/shell_test.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,3 +13,20 @@ func TestRunShell(t *testing.T) {
1313
assert.NoError(t, Run(context.Background(), "echo 1", Stdout(&buffer)))
1414
assert.Equal(t, "1\n", buffer.String())
1515
}
16+
17+
func TestRunShellInterpreterError(t *testing.T) {
18+
defer func() {
19+
if r := recover(); r != nil {
20+
err, ok := r.(error)
21+
assert.True(t, ok)
22+
assert.Contains(t, err.Error(), "failed to set up interpreter")
23+
}
24+
}()
25+
Run(context.Background(), "")
26+
}
27+
28+
func TestRunShellParseError(t *testing.T) {
29+
err := Run(context.Background(), "`invalid command")
30+
assert.Error(t, err)
31+
assert.Contains(t, err.Error(), "failed to parse shell command")
32+
}

0 commit comments

Comments
 (0)