Skip to content

Commit

Permalink
fix: error swallowing when using continue (vercel#4354)
Browse files Browse the repository at this point in the history
### Description

Fixes regression added in vercel#4254 where we swallowed exit codes when
`--continue` was used to invoke turbo.

Don't love this fix, but it was the most straightforward fix I could
see. @mehulkar unsure if I'm handling the case where an execution
summary doesn't have an exit code correctly.

### Testing Instructions

See added integration test and the changed run summary test which now
has a 1 exit code.
  • Loading branch information
chris-olszewski authored Mar 27, 2023
1 parent 9d027e7 commit 66b690d
Show file tree
Hide file tree
Showing 6 changed files with 72 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ Setup
The maybefails task fails for one workspace but not the other
$ TURBO_RUN_SUMMARY=true ${TURBO} run maybefails --continue > /dev/null
my-app:maybefails: command finished with error, but continuing...
ERROR run failed: command exited (1)
[1]

# ExitCode here is 1, because npm will report all errors with exitCode 1
$ cat $(/bin/ls .turbo/runs/*.json | head -n1) | jq '.tasks | map(select(.taskId == "my-app#maybefails")) | .[0].execution'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
"name": "my-app",
"scripts": {
"okay": "echo 'working'",
"okay2": "echo 'working'",
"error": "exit 2"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@
},
"okay": {
"outputs": []
},
"okay2": {
"dependsOn": ["error"],
"outputs": []
}
}
}
43 changes: 39 additions & 4 deletions cli/integration_tests/monorepo_one_script_error/run.t
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,13 @@ Note that npm reports any failed script as exit code 1, even though we "exit 2"
\xe2\x80\xa2 Packages in scope: my-app (esc)
\xe2\x80\xa2 Running error in 1 packages (esc)
\xe2\x80\xa2 Remote caching disabled (esc)
my-app:okay: cache miss, executing 32478bc54ccf0adb
my-app:okay: cache miss, executing 62ff444b3068c13b
my-app:okay:
my-app:okay: > okay
my-app:okay: > echo 'working'
my-app:okay:
my-app:okay: working
my-app:error: cache miss, executing 1c682ef8cade4542
my-app:error: cache miss, executing 7ec8abd964436064
my-app:error:
my-app:error: > error
my-app:error: > exit 2
Expand All @@ -38,13 +38,13 @@ Make sure error isn't cached
\xe2\x80\xa2 Packages in scope: my-app (esc)
\xe2\x80\xa2 Running error in 1 packages (esc)
\xe2\x80\xa2 Remote caching disabled (esc)
my-app:okay: cache hit, replaying output 32478bc54ccf0adb
my-app:okay: cache hit, replaying output 62ff444b3068c13b
my-app:okay:
my-app:okay: > okay
my-app:okay: > echo 'working'
my-app:okay:
my-app:okay: working
my-app:error: cache miss, executing 1c682ef8cade4542
my-app:error: cache miss, executing 7ec8abd964436064
my-app:error:
my-app:error: > error
my-app:error: > exit 2
Expand All @@ -62,3 +62,38 @@ Make sure error isn't cached

ERROR run failed: command exited (1)
[1]

Make sure error code isn't swallowed with continue
$ ${TURBO} okay2 --continue
\xe2\x80\xa2 Packages in scope: my-app (esc)
\xe2\x80\xa2 Running okay2 in 1 packages (esc)
\xe2\x80\xa2 Remote caching disabled (esc)
my-app:okay: cache hit, replaying output 62ff444b3068c13b
my-app:okay:
my-app:okay: > okay
my-app:okay: > echo 'working'
my-app:okay:
my-app:okay: working
my-app:error: cache miss, executing 7ec8abd964436064
my-app:error:
my-app:error: > error
my-app:error: > exit 2
my-app:error:
my-app:error: npm ERR! Lifecycle script `error` failed with error:
my-app:error: npm ERR! Error: command failed
my-app:error: npm ERR! in workspace: my-app
my-app:error: npm ERR! at location: .*apps/my-app (re)
my-app:error: command finished with error, but continuing...
my-app:okay2: cache miss, executing 6ec9a564c31e8f12
my-app:okay2:
my-app:okay2: > okay2
my-app:okay2: > echo 'working'
my-app:okay2:
my-app:okay2: working

Tasks: 2 successful, 3 total
Cached: 1 cached, 3 total
Time:\s*[\.0-9]+m?s (re)

ERROR run failed: command exited (1)
[1]
16 changes: 16 additions & 0 deletions cli/internal/run/real_run.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,22 @@ func RealRun(
base.UI.Error(err.Error())
}

// When continue on error is enabled don't register failed tasks as errors
// and instead must inspect the task summaries.
if ec.rs.Opts.runOpts.continueOnError {
for _, summary := range runSummary.RunSummary.Tasks {
if childExit := summary.Execution.ExitCode(); childExit != nil {
childExit := *childExit
if childExit < 0 {
childExit = -childExit
}
if childExit > exitCode {
exitCode = childExit
}
}
}
}

runSummary.Close(exitCode, base.RepoRoot)

if exitCode != 0 {
Expand Down
10 changes: 10 additions & 0 deletions cli/internal/runsummary/execution_summary.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,16 @@ func (ts *TaskExecutionSummary) MarshalJSON() ([]byte, error) {
return json.Marshal(&serializable)
}

// ExitCode access exit code nil means no exit code was received
func (ts *TaskExecutionSummary) ExitCode() *int {
var exitCode int
if ts.exitCode == nil {
return nil
}
exitCode = *ts.exitCode
return &exitCode
}

// executionSummary is the state of the entire `turbo run`. Individual task state in `Tasks` field
type executionSummary struct {
// mu guards reads/writes to the `state` field
Expand Down

0 comments on commit 66b690d

Please sign in to comment.