Merge pull request 'Image builds run their RUN steps on pasta, and every build failure keeps its real error (issue 1703)' (#502) from fix/1703-image-error-shown into main

modules/mesh-build-node/cmd/mesh-build-node/build_test.go

@@ -267,3 +267,38 @@ func TestTheLaunchersAreTheSDKsRule(t *testing.T) {
 		t.Errorf("launchers %v, mesh-sdk's rule %v", got, want)
 	}
 }
+
+// issue 1703: a failed command's reason keeps stderr's end and stdout's start, each bounded, so a long stdout (npm's
+// progress) never pushes npm's error out, and tsc's first error on stdout stays in.
+func TestRanFailedKeepsTheError(t *testing.T) {
+	noise := strings.Repeat("npm progress line\n", 200)
+	cases := map[string]struct {
+		ran     forgekey.Ran
+		want    []string
+		wantNot []string
+	}{
+		"passed":           {forgekey.Ran{Stdout: "ok"}, nil, nil},
+		"npm, long stdout": {forgekey.Ran{Status: 1, Stdout: noise, Stderr: "npm ERR! early\n" + noise + "npm ERR! code E404\n"}, []string{"status 1", "npm ERR! code E404"}, []string{"npm ERR! early"}},
+		"tsc on stdout":    {forgekey.Ran{Status: 2, Stdout: "src/a.ts(1,7): error TS2322: wrong\n" + noise}, []string{"error TS2322"}, nil},
+		"never ran":        {forgekey.Ran{Err: "ENOENT"}, []string{"ENOENT"}, nil},
+	}
+	for name, c := range cases {
+		got := ranFailed(c.ran)
+		if c.want == nil && got != "" {
+			t.Errorf("%s: %q, want none", name, got)
+		}
+		for _, w := range c.want {
+			if !strings.Contains(got, w) {
+				t.Errorf("%s: %q lacks %q", name, got, w)
+			}
+		}
+		for _, w := range c.wantNot {
+			if strings.Contains(got, w) {
+				t.Errorf("%s: %q carries %q", name, got, w)
+			}
+		}
+		if len(got) > failedBytes+100 {
+			t.Errorf("%s: %d bytes, past its bound", name, len(got))
+		}
+	}
+}

modules/mesh-build-node/cmd/mesh-build-node/workspace.go

@@ -180,14 +180,28 @@ func (m *Module) runIn(ctx context.Context, root string, write bool, dir string,
 	return run(ctx, extra, "env", append([]string{"-C", dir, name}, args...)...)
 }
 
-// ranFailed says why a command did not pass, or "" when it did.
+// ranFailed says why a command did not pass, or "" when it did (issue 1703): its exit status, the end of what it wrote
+// on stderr (npm's error is its last lines) and the start of what it wrote on stdout (tsc writes its errors there,
+// first first), each bounded, so a long stdout never pushes stderr's error out of the reason.
 func ranFailed(r forgekey.Ran) string {
 	if r.Status == 0 && r.Err == "" {
 		return ""
 	}
-	out := strings.TrimSpace(r.Stdout + "\n" + r.Stderr)
-	if len(out) > 1500 {
-		out = out[:1500] + " ..."
+	why := fmt.Sprintf("status %d %s", r.Status, r.Err)
+	if e := strings.TrimSpace(r.Stderr); e != "" {
+		if len(e) > failedBytes/2 {
+			e = "... " + e[len(e)-failedBytes/2:]
+		}
+		why += ": stderr: " + e
 	}
-	return fmt.Sprintf("status %d %s: %s", r.Status, r.Err, out)
+	if o := strings.TrimSpace(r.Stdout); o != "" {
+		if len(o) > failedBytes/2 {
+			o = o[:failedBytes/2] + " ..."
+		}
+		why += ": stdout: " + o
+	}
+	return why
 }
+
+// failedBytes bounds what ranFailed carries of a command's output, stderr and stdout half each.
+const failedBytes = 1500