From bb9ff6f9cd69afa3419803dc8fb91c4d9bebb49a Mon Sep 17 00:00:00 2001 From: Mehul Kar Date: Tue, 11 Apr 2023 16:31:06 -0700 Subject: [PATCH] Gather Run Summary even when tasks fail (#4524) --- cli/integration_tests/run_summary/error.t | 114 ++++++++++++++++++ .../run_summary/{run_summary.t => monorepo.t} | 2 - .../run_summary/run_summary_fail.t | 30 ----- .../single-pacakge.t} | 1 - cli/internal/graph/graph.go | 4 +- cli/internal/run/real_run.go | 8 +- cli/internal/runsummary/execution_summary.go | 15 +-- 7 files changed, 129 insertions(+), 45 deletions(-) create mode 100644 cli/integration_tests/run_summary/error.t rename cli/integration_tests/run_summary/{run_summary.t => monorepo.t} (98%) delete mode 100644 cli/integration_tests/run_summary/run_summary_fail.t rename cli/integration_tests/{single_package/run-summary.t => run_summary/single-pacakge.t} (98%) diff --git a/cli/integration_tests/run_summary/error.t b/cli/integration_tests/run_summary/error.t new file mode 100644 index 0000000000000..a064938b64585 --- /dev/null +++ b/cli/integration_tests/run_summary/error.t @@ -0,0 +1,114 @@ +Setup + $ . ${TESTDIR}/../_helpers/setup.sh + $ . ${TESTDIR}/../_helpers/setup_monorepo.sh $(pwd) + + $ rm -rf .turbo/runs + +Use --filter because otherwise we'll get nondeterministic execution summary depending on +whether the other workspaces had finished executing their task yet. We don't care to validate +that behavior in this test. + $ ${TURBO} run maybefails --filter=my-app --summarize > /dev/null 2>&1 + [1] + + $ source "$TESTDIR/../_helpers/run-summary-utils.sh" + $ SUMMARY=$(/bin/ls .turbo/runs/*.json | head -n1) + +Validate that there was a failed task and exitCode is 1 (which is what we get from npm for the failed task) + $ cat $SUMMARY | jq '.execution' + { + "command": "turbo run maybefails --filter=my-app", + "repoPath": "", + "success": 0, + "failed": 1, + "cached": 0, + "attempted": 1, + "startTime": [0-9]+, (re) + "endTime": [0-9]+, (re) + "exitCode": 1 + } + +Validate that we got a full task summary for the failed task with an error in .execution + $ echo $(getSummaryTaskId $SUMMARY "my-app#maybefails") | jq + { + "taskId": "my-app#maybefails", + "task": "maybefails", + "package": "my-app", + "hash": "c11710f9379cb7ea", + "inputs": { + "package.json": "6bcf57fd6ff30d1a6f40ad8d8d08e8b940fc7e3b" + }, + "hashOfExternalDependencies": "ccab0b28617f1f56", + "cache": { + "local": false, + "remote": false + }, + "command": "exit 4", + "cliArguments": [], + "outputs": null, + "excludedOutputs": null, + "logFile": "apps/my-app/.turbo/turbo-maybefails.log", + "directory": "apps/my-app", + "dependencies": [], + "dependents": [], + "resolvedTaskDefinition": { + "outputs": [], + "cache": true, + "dependsOn": [], + "inputs": [], + "outputMode": "full", + "env": [], + "persistent": false + }, + "expandedOutputs": [], + "framework": "", + "environmentVariables": { + "configured": [], + "inferred": [], + "global": [ + "SOME_ENV_VAR=", + "VERCEL_ANALYTICS_ID=" + ] + }, + "execution": { + "startTime": [0-9]+, (re) + "endTime": [0-9]+, (re) + "error": "command .* npm run maybefails exited \(1\)", (re) + "exitCode": 1 + } + } + +With --continue flag. We won't validate the whole thing, just execution +Use --force flag so we can be deterministic about cache hits +Don't use --filter here, so we can validate that both tasks attempted to run + $ rm -rf .turbo/runs + $ ${TURBO} run maybefails --summarize --force --continue > /dev/null 2>&1 + [1] + + $ source "$TESTDIR/../_helpers/run-summary-utils.sh" + $ SUMMARY=$(/bin/ls .turbo/runs/*.json | head -n1) + +success should be 1, and attempted should be 2 + $ cat $SUMMARY | jq '.execution' + { + "command": "turbo run maybefails --continue", + "repoPath": "", + "success": 1, + "failed": 1, + "cached": 0, + "attempted": 2, + "startTime": [0-9]+, (re) + "endTime": [0-9]+, (re) + "exitCode": 1 + } + + $ cat $SUMMARY | jq '.tasks | length' + 2 + +# exitCode is 1, because npm will report all errors with exitCode 1 + $ getSummaryTaskId $SUMMARY "my-app#maybefails" | jq '.execution' + { + "startTime": [0-9]+, (re) + "endTime": [0-9]+, (re) + "error": "command .* npm run maybefails exited \(1\)", (re) + "exitCode": 1 + } diff --git a/cli/integration_tests/run_summary/run_summary.t b/cli/integration_tests/run_summary/monorepo.t similarity index 98% rename from cli/integration_tests/run_summary/run_summary.t rename to cli/integration_tests/run_summary/monorepo.t index b50a9909216e6..5de6b22596b1c 100644 --- a/cli/integration_tests/run_summary/run_summary.t +++ b/cli/integration_tests/run_summary/monorepo.t @@ -59,7 +59,6 @@ Setup { "startTime": [0-9]+, (re) "endTime": [0-9]+, (re) - "error": null, "exitCode": 0 } $ echo $FIRST_APP_BUILD | jq '.cliArguments' @@ -89,7 +88,6 @@ Setup { "startTime": [0-9]+, (re) "endTime": [0-9]+, (re) - "error": null, "exitCode": 0 } diff --git a/cli/integration_tests/run_summary/run_summary_fail.t b/cli/integration_tests/run_summary/run_summary_fail.t deleted file mode 100644 index 18c5189258ffa..0000000000000 --- a/cli/integration_tests/run_summary/run_summary_fail.t +++ /dev/null @@ -1,30 +0,0 @@ -Setup - $ . ${TESTDIR}/../_helpers/setup.sh - $ . ${TESTDIR}/../_helpers/setup_monorepo.sh $(pwd) - - $ rm -rf .turbo/runs - -# Turbo exits early and doesn't generate run summaries on errors, so we need to use --continue for this test. -The maybefails task fails for one workspace but not the other - $ ${TURBO} run maybefails --summarize --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' - { - "startTime": [0-9]+, (re) - "endTime": [0-9]+, (re) - "error": {}, - "exitCode": 1 - } - -# This one has success exit code - $ cat $(/bin/ls .turbo/runs/*.json | head -n1) | jq '.tasks | map(select(.taskId == "util#maybefails")) | .[0].execution' - { - "startTime": [0-9]+, (re) - "endTime": [0-9]+, (re) - "error": null, - "exitCode": 0 - } diff --git a/cli/integration_tests/single_package/run-summary.t b/cli/integration_tests/run_summary/single-pacakge.t similarity index 98% rename from cli/integration_tests/single_package/run-summary.t rename to cli/integration_tests/run_summary/single-pacakge.t index 036becb4261ef..b4621ee42c44d 100644 --- a/cli/integration_tests/single_package/run-summary.t +++ b/cli/integration_tests/run_summary/single-pacakge.t @@ -70,7 +70,6 @@ Check { "startTime": [0-9]+, (re) "endTime": [0-9]+, (re) - "error": null, "exitCode": 0 } $ echo $TASK_SUMMARY | jq '.cliArguments' diff --git a/cli/internal/graph/graph.go b/cli/internal/graph/graph.go index 214df0f50f89d..db68417d24c8d 100644 --- a/cli/internal/graph/graph.go +++ b/cli/internal/graph/graph.go @@ -52,7 +52,7 @@ func (g *CompleteGraph) GetPackageTaskVisitor( taskGraph *dag.AcyclicGraph, getArgs func(taskID string) []string, logger hclog.Logger, - visitor func(ctx gocontext.Context, packageTask *nodes.PackageTask, taskSummary *runsummary.TaskSummary) error, + execFunc func(ctx gocontext.Context, packageTask *nodes.PackageTask, taskSummary *runsummary.TaskSummary) error, ) func(taskID string) error { return func(taskID string) error { packageName, taskName := util.GetPackageTaskFromId(taskID) @@ -140,7 +140,7 @@ func (g *CompleteGraph) GetPackageTaskVisitor( summary.Dependents = descendents } - return visitor(ctx, packageTask, summary) + return execFunc(ctx, packageTask, summary) } } diff --git a/cli/internal/run/real_run.go b/cli/internal/run/real_run.go index b238d754676ff..9e67d58e8730e 100644 --- a/cli/internal/run/real_run.go +++ b/cli/internal/run/real_run.go @@ -98,9 +98,6 @@ func RealRun( taskSummaries := []*runsummary.TaskSummary{} execFunc := func(ctx gocontext.Context, packageTask *nodes.PackageTask, taskSummary *runsummary.TaskSummary) error { taskExecutionSummary, err := ec.exec(ctx, packageTask) - if err != nil { - return err - } // taskExecutionSummary will be nil if the task never executed // (i.e. if the workspace didn't implement the script corresponding to the task) @@ -117,6 +114,11 @@ func RealRun( mu.Unlock() } + // Return the error when there is one + if err != nil { + return err + } + return nil } diff --git a/cli/internal/runsummary/execution_summary.go b/cli/internal/runsummary/execution_summary.go index 77a2c1325f0a2..1071846b495f8 100644 --- a/cli/internal/runsummary/execution_summary.go +++ b/cli/internal/runsummary/execution_summary.go @@ -26,7 +26,7 @@ type executionEvent struct { // Its current status Status executionEventName // Error, only populated for failure statuses - Err error + Err string exitCode *int } @@ -68,7 +68,7 @@ func (en executionEventName) toString() string { type TaskExecutionSummary struct { startAt time.Time // set once status executionEventName // current status, updated during execution - err error // only populated for failure statuses + err string // only populated for failure statuses Duration time.Duration // updated during the task execution exitCode *int // pointer so we can distinguish between 0 and unknown. } @@ -81,10 +81,10 @@ func (ts *TaskExecutionSummary) endTime() time.Time { // We'll use an anonmyous, private struct for this, so it's not confusingly duplicated func (ts *TaskExecutionSummary) MarshalJSON() ([]byte, error) { serializable := struct { - Start int64 `json:"startTime"` - End int64 `json:"endTime"` - Err error `json:"error"` - ExitCode *int `json:"exitCode"` + Start int64 `json:"startTime"` + End int64 `json:"endTime"` + Err string `json:"error,omitempty"` + ExitCode *int `json:"exitCode"` }{ Start: ts.startAt.UnixMilli(), End: ts.endTime().UnixMilli(), @@ -197,8 +197,9 @@ func (es *executionSummary) run(taskID string) (func(outcome executionEventName, // when we assign it to the taskExecutionSummary. exitCode: exitCode, } + if err != nil { - result.Err = fmt.Errorf("running %v failed: %w", taskID, err) + result.Err = err.Error() } // Ignore the return value here