Skip to content

Commit f4b2d92

Browse files
committed
interp: clarify that Runner.err is for fatal errors only at this point
And simplify the Runner.Run logic so that we don't shove NewExitStatus errors there right before returning, which is unnecessary confusion.
1 parent d9296b4 commit f4b2d92

File tree

3 files changed

+21
-20
lines changed

3 files changed

+21
-20
lines changed

interp/api.go

+8-7
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ type Runner struct {
131131

132132
noErrExit bool
133133

134-
err error // current shell exit code or fatal error
134+
fatalErr error // current fatal error, e.g. from a handler
135135
returning bool // whether the current function `return`ed
136136
exiting bool // whether the current shell `exit`ed
137137

@@ -828,7 +828,7 @@ func (r *Runner) Run(ctx context.Context, node syntax.Node) error {
828828
r.Reset()
829829
}
830830
r.fillExpandConfig(ctx)
831-
r.err = nil
831+
r.fatalErr = nil
832832
r.returning = false
833833
r.exiting = false
834834
r.filename = ""
@@ -846,13 +846,14 @@ func (r *Runner) Run(ctx context.Context, node syntax.Node) error {
846846
default:
847847
return fmt.Errorf("node can only be File, Stmt, or Command: %T", node)
848848
}
849-
if r.exit != 0 {
850-
r.setErr(NewExitStatus(uint8(r.exit)))
849+
maps.Insert(r.Vars, r.writeEnv.Each)
850+
if r.fatalErr != nil {
851+
return r.fatalErr
851852
}
852-
if r.Vars != nil {
853-
maps.Insert(r.Vars, r.writeEnv.Each)
853+
if r.exit != 0 {
854+
return NewExitStatus(uint8(r.exit))
854855
}
855-
return r.err
856+
return nil
856857
}
857858

858859
// Exited reports whether the last Run call should exit an entire shell. This

interp/builtin.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,7 @@ func (r *Runner) builtinCode(ctx context.Context, pos syntax.Pos, name string, a
210210
var err error
211211
pwd, err = filepath.EvalSymlinks(pwd)
212212
if err != nil {
213-
r.setErr(err)
213+
r.setFatalErr(err) // perhaps overly dramatic?
214214
return 1
215215
}
216216
}

interp/runner.go

+12-12
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ func (r *Runner) fillExpandConfig(ctx context.Context) {
6565
r2.stdout = w
6666
r2.stmts(ctx, cs.Stmts)
6767
r.lastExpandExit = r2.exit
68-
return r2.err
68+
return r2.fatalErr
6969
},
7070
ProcSubst: func(ps *syntax.ProcSubst) (string, error) {
7171
if runtime.GOOS == "windows" {
@@ -261,9 +261,9 @@ func (r *Runner) handlerCtx(ctx context.Context) context.Context {
261261
return context.WithValue(ctx, handlerCtxKey{}, hc)
262262
}
263263

264-
func (r *Runner) setErr(err error) {
265-
if r.err == nil {
266-
r.err = err
264+
func (r *Runner) setFatalErr(err error) {
265+
if r.fatalErr == nil {
266+
r.fatalErr = err
267267
}
268268
}
269269

@@ -280,11 +280,11 @@ func (r *Runner) errf(format string, a ...any) {
280280
}
281281

282282
func (r *Runner) stop(ctx context.Context) bool {
283-
if r.err != nil || r.returning || r.exiting {
283+
if r.fatalErr != nil || r.returning || r.exiting {
284284
return true
285285
}
286286
if err := ctx.Err(); err != nil {
287-
r.err = err
287+
r.fatalErr = err
288288
return true
289289
}
290290
if r.opts[optNoExec] {
@@ -367,7 +367,7 @@ func (r *Runner) cmd(ctx context.Context, cm syntax.Command) {
367367
r2 := r.subshell(false)
368368
r2.stmts(ctx, cm.Stmts)
369369
r.exit = r2.exit
370-
r.setErr(r2.err)
370+
r.setFatalErr(r2.fatalErr)
371371
case *syntax.CallExpr:
372372
// Use a new slice, to not modify the slice in the alias map.
373373
var args []*syntax.Word
@@ -464,7 +464,7 @@ func (r *Runner) cmd(ctx context.Context, cm syntax.Command) {
464464
case syntax.Pipe, syntax.PipeAll:
465465
pr, pw, err := os.Pipe()
466466
if err != nil {
467-
r.setErr(err)
467+
r.setFatalErr(err) // not being able to create a pipe is rare but critical
468468
return
469469
}
470470
r2 := r.subshell(false)
@@ -489,7 +489,7 @@ func (r *Runner) cmd(ctx context.Context, cm syntax.Command) {
489489
r.exit = r2.exit
490490
r.exiting = r2.exiting
491491
}
492-
r.setErr(r2.err)
492+
r.setFatalErr(r2.fatalErr)
493493
}
494494
case *syntax.IfClause:
495495
oldNoErrExit := r.noErrExit
@@ -994,7 +994,7 @@ func (r *Runner) call(ctx context.Context, pos syntax.Pos, args []string) {
994994
args, err = r.callHandler(r.handlerCtx(ctx), args)
995995
if err != nil {
996996
// handler's custom fatal error
997-
r.setErr(err)
997+
r.setFatalErr(err)
998998
return
999999
}
10001000
}
@@ -1034,7 +1034,7 @@ func (r *Runner) exec(ctx context.Context, args []string) {
10341034
r.exit = int(status)
10351035
} else {
10361036
// handler's custom fatal error
1037-
r.setErr(err)
1037+
r.setFatalErr(err)
10381038
r.exit = 1
10391039
}
10401040
} else {
@@ -1066,7 +1066,7 @@ func (r *Runner) open(ctx context.Context, path string, flags int, mode os.FileM
10661066
r.errf("%v\n", err)
10671067
}
10681068
default: // handler's custom fatal error
1069-
r.setErr(err)
1069+
r.setFatalErr(err)
10701070
}
10711071
return nil, err
10721072
}

0 commit comments

Comments
 (0)