Existing and error logs behavior #4656

gsoltis · 2023-04-20T21:35:22Z

Description

Adds an integration test for existing behavior (continue.t)
Fixes behavior of --output-logs=errors-only to match behavior for full in the case of a task that errors
Fixes --continue behavior to report all failed tasks at the end

Testing Instructions

New continue.t integration test. I recommend viewing the changes to that file at each commit to see initial behavior and how it changes.

Fixes #4504
link WEB-895

vercel · 2023-04-20T21:35:32Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated (UTC)
examples-designsystem-docs	🔄 Building (Inspect)	Visit Preview	Apr 24, 2023 9:37pm
examples-kitchensink-blog	🔄 Building (Inspect)	Visit Preview	Apr 24, 2023 9:37pm
examples-svelte-web	🔄 Building (Inspect)	Visit Preview	Apr 24, 2023 9:37pm

8 Ignored Deployments

Name	Status	Preview	Updated (UTC)
examples-basic-web	⬜️ Ignored (Inspect)	Visit Preview	Apr 24, 2023 9:37pm
examples-cra-web	⬜️ Ignored (Inspect)	Visit Preview	Apr 24, 2023 9:37pm
examples-gatsby-web	⬜️ Ignored (Inspect)	Visit Preview	Apr 24, 2023 9:37pm
examples-native-web	⬜️ Ignored (Inspect)	Visit Preview	Apr 24, 2023 9:37pm
examples-nonmonorepo	⬜️ Ignored (Inspect)	Visit Preview	Apr 24, 2023 9:37pm
examples-tailwind-web	⬜️ Ignored (Inspect)	Visit Preview	Apr 24, 2023 9:37pm
examples-vite-web	⬜️ Ignored (Inspect)	Visit Preview	Apr 24, 2023 9:37pm
turbo-site	⬜️ Ignored (Inspect)	Visit Preview	Apr 24, 2023 9:37pm

socket-security · 2023-04-20T21:46:18Z

New dependency changes detected. Learn more about Socket for GitHub ↗︎

🚨 Potential security issues found in this pull request. To accept the risk, merge this PR and you will not be notified again.

Bot Commands

To ignore an alert, reply with a comment starting with @SocketSecurity ignore followed by a space separated list of package-name@version specifiers. e.g. @SocketSecurity ignore foo@1.0.0 bar@* or ignore all packages with @SocketSecurity ignore-all

@SocketSecurity ignore some-lib@0.0.1

🧐 Potential typo squat

Package name is similar to other popular packages and may not be the package you want.

Use care when consuming similarly named packages and ensure that you did not intend to consume a different package. Malicious packages often publish using similar names as existing popular packages.

Package	📎 Did you mean?	Found in
some-lib@0.0.1 (added)	sdome-lib (18 thousand times more downloads)	`turborepo-tests/integration/tests/_fixtures/monorepo_dependency_error/apps/my-app/package.json`, `turborepo-tests/integration/tests/_fixtures/monorepo_dependency_error/apps/other-app/package.json`

Pull request alert summary

Issue	Status
Install scripts	✅ 0 issues
Native code	✅ 0 issues
Bin script shell injection	✅ 0 issues
Unresolved require	✅ 0 issues
Invalid package.json	✅ 0 issues
HTTP dependency	✅ 0 issues
Git dependency	✅ 0 issues
Potential typo squat	⚠️ 1 issue
Known Malware	✅ 0 issues
Telemetry	✅ 0 issues
Protestware/Troll package	✅ 0 issues

📊 Modified Dependency Overview:

➕ Added Package	Capability Access	`+/-` Transitive Count	Publisher
some-lib@0.0.1	eval, network, filesystem	`+6`	d.costa

github-actions · 2023-04-20T22:30:15Z

🟢 CI successful 🟢

Thanks

gsoltis · 2023-04-20T22:35:25Z

The best way to see the behavior changes is to look at the changes to the integration test:
existing behavior
fixing errors-only
adding back summary

mehulkar

Some comments, but pretty much a fix ship.

cli/internal/run/real_run.go

cli/internal/runcache/runcache.go

mehulkar · 2023-04-20T23:46:18Z

turborepo-tests/integration/tests/_fixtures/monorepo_dependency_error/.gitignore

Is it possible to expand either monorepo_one_script_error or basic_monorepo (or some other one that I missed) for this test instead of creating a new one? I would like to prevent fixtures being 1:1 with tests when possible.

Hmm. I need more than one error. I could add a new task to basic_monorepo, although I'm not sure a special case task is preferable to a special case fixture. That being said, I don't feel super strongly. What do you think?

mehulkar · 2023-04-20T23:47:49Z

...orepo-tests/integration/tests/_fixtures/monorepo_dependency_error/apps/some-lib/package.json

not meaningful for the test, but slightly weird to see apps/some-lib instead of the packages/some-lib pattern.

mehulkar · 2023-04-21T20:01:02Z

cli/internal/core/engine.go

-		if strings.Contains(taskID, ROOT_NODE_NAME) {
-			return nil
-		}
+	// Returning an error from the Walk callback cancels downstream tasks, but not unrelated tasks.


Is this the behavior before or after this change?

This is the behavior of the graph library we use, which is unchanged.

mehulkar · 2023-04-21T20:09:02Z

cli/internal/core/engine.go

+		defer errorMu.Unlock()
+		errors = append(errors, err)
+	}
+	unusedErrs := e.TaskGraph.Walk(func(v dag.Vertex) error {


what other errors could we have here? Is it something from a task execution or something from the dag library essentially?

The anonymous function pattern should prevent us from leaking any of our execution errors out. It's possible that something in the dag library can produce an error, but from the dag library's point of view, every node visit should look successful.

I was trying to understand what unusedErrs was about. Since we're recording all our errors with recordErr(), I did not understand what the unusedErrs was or why it could occur

Greg Soltis added 2 commits April 20, 2023 14:32

Existing and error logs behavior

bab4d3b

errors-only matches full logs output for a task error

08db941

turbo-orchestrator bot added team: turbopack labels Apr 20, 2023

Resume printing failed commands at the end of a run

8327560

gsoltis force-pushed the gsoltis/fix_errors_only_continue branch from 7a92597 to 8327560 Compare April 20, 2023 21:58

gsoltis removed the team: turbopack label Apr 20, 2023

turbo-orchestrator bot added the team: turbopack label Apr 20, 2023

gsoltis marked this pull request as ready for review April 20, 2023 22:21

gsoltis requested review from a team as code owners April 20, 2023 22:21

gsoltis requested review from nathanhammond, chris-olszewski and mehulkar and removed request for chris-olszewski April 20, 2023 22:21

Merge branch 'main' into gsoltis/fix_errors_only_continue

2837721

turbo-orchestrator bot removed the team: turbopack label Apr 21, 2023

gsoltis removed the request for review from a team April 21, 2023 17:37

vercel bot deployed to Preview – examples-vite-web April 21, 2023 17:37 View deployment

vercel bot deployed to Preview – examples-tailwind-web April 21, 2023 17:37 View deployment

vercel bot deployed to Preview – examples-basic-web April 21, 2023 17:37 View deployment

vercel bot deployed to Preview – examples-cra-web April 21, 2023 17:37 View deployment

vercel bot deployed to Preview – examples-native-web April 21, 2023 17:37 View deployment

vercel bot deployed to Preview – examples-gatsby-web April 21, 2023 17:37 View deployment

vercel bot deployed to Preview – examples-svelte-web April 21, 2023 17:37 View deployment

vercel bot deployed to Preview – examples-nonmonorepo April 21, 2023 17:37 View deployment

vercel bot deployed to Preview – examples-kitchensink-blog April 21, 2023 17:37 View deployment

vercel bot deployed to Preview – turbo-site April 21, 2023 17:37 View deployment

vercel bot deployed to Preview – examples-designsystem-docs April 21, 2023 17:37 View deployment

mehulkar reviewed Apr 21, 2023

View reviewed changes

Greg Soltis added 2 commits April 24, 2023 14:34

Drop commented-out code

031fdf6

Clarify comment a little

d622cc9

mehulkar approved these changes Apr 25, 2023

View reviewed changes

gsoltis added the pr: automerge Kodiak will merge these automatically after checks pass label Apr 25, 2023

kodiakhq bot merged commit 3b3243a into main Apr 25, 2023

kodiakhq bot deleted the gsoltis/fix_errors_only_continue branch April 25, 2023 21:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Existing and error logs behavior #4656

Existing and error logs behavior #4656

gsoltis commented Apr 20, 2023 •

edited by padmaia

Loading

vercel bot commented Apr 20, 2023 •

edited

Loading

socket-security bot commented Apr 20, 2023

github-actions bot commented Apr 20, 2023 •

edited

Loading

gsoltis commented Apr 20, 2023

mehulkar left a comment

mehulkar Apr 20, 2023

gsoltis Apr 24, 2023

mehulkar Apr 20, 2023

mehulkar Apr 21, 2023

gsoltis Apr 24, 2023

mehulkar Apr 21, 2023

gsoltis Apr 24, 2023

mehulkar Apr 25, 2023 •

edited

Loading

Existing and error logs behavior #4656

Existing and error logs behavior #4656

Conversation

gsoltis commented Apr 20, 2023 • edited by padmaia Loading

Description

Testing Instructions

vercel bot commented Apr 20, 2023 • edited Loading

socket-security bot commented Apr 20, 2023

github-actions bot commented Apr 20, 2023 • edited Loading

🟢 CI successful 🟢

gsoltis commented Apr 20, 2023

mehulkar left a comment

Choose a reason for hiding this comment

mehulkar Apr 20, 2023

Choose a reason for hiding this comment

gsoltis Apr 24, 2023

Choose a reason for hiding this comment

mehulkar Apr 20, 2023

Choose a reason for hiding this comment

mehulkar Apr 21, 2023

Choose a reason for hiding this comment

gsoltis Apr 24, 2023

Choose a reason for hiding this comment

mehulkar Apr 21, 2023

Choose a reason for hiding this comment

gsoltis Apr 24, 2023

Choose a reason for hiding this comment

mehulkar Apr 25, 2023 • edited Loading

Choose a reason for hiding this comment

gsoltis commented Apr 20, 2023 •

edited by padmaia

Loading

vercel bot commented Apr 20, 2023 •

edited

Loading

github-actions bot commented Apr 20, 2023 •

edited

Loading

mehulkar Apr 25, 2023 •

edited

Loading