Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure error message for failed slices stay within the slice #13042

Merged
merged 1 commit into from
Sep 16, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions .werft/build.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import * as fs from "fs";
import { SpanStatusCode } from "@opentelemetry/api";
import { Werft } from "./util/werft";
import { FailedSliceError, Werft } from "./util/werft";
import { reportBuildFailureInSlack } from "./util/slack";
import * as Tracing from "./observability/tracing";
import * as VM from "./vm/vm";
Expand All @@ -27,7 +27,12 @@ Tracing.initialize()
message: err,
});

console.log("Error", err);
if (err instanceof FailedSliceError) {
// This error was produced using werft.fail which means that we
// already handled it "gracefully"
} else {
console.log("Error", err);
}

if (context.Repository.ref === "refs/heads/main") {
reportBuildFailureInSlack(context, err).catch((error: Error) => {
Expand Down
17 changes: 6 additions & 11 deletions .werft/jobs/build/validate-changes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,12 @@ import { JobConfig } from "./job-config";

export async function validateChanges(werft: Werft, config: JobConfig) {
werft.phase("validate-changes", "validating changes");
try {
await Promise.all([
branchNameCheck(werft, config),
preCommitCheck(werft),
typecheckWerftJobs(werft),
leewayVet(werft),
]);
} catch (err) {
werft.fail("validate-changes", err);
}
werft.done("validate-changes");
await Promise.all([
branchNameCheck(werft, config),
preCommitCheck(werft),
typecheckWerftJobs(werft),
leewayVet(werft),
]);
}

// Branch names cannot be longer than 45 characters.
Expand Down
25 changes: 21 additions & 4 deletions .werft/util/werft.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,14 @@ import { exec } from "./shell";

let werft: Werft;

export class FailedSliceError extends Error {
constructor(message: string) {
super(message);
this.name = "FailedSliceError";
Object.setPrototypeOf(this, FailedSliceError.prototype);
}
}

/**
* For backwards compatibility with existing code we expose a global Werft instance
*/
Expand Down Expand Up @@ -75,7 +83,7 @@ export class Werft {
/**
* Use this when you intend to fail the werft job
*/
public fail(slice, err) {
public fail(slice: string, err: string | Error) {
const span = this.sliceSpans[slice];

if (span) {
Expand All @@ -92,12 +100,21 @@ export class Werft {
}
span.setStatus({
code: SpanStatusCode.ERROR,
message: err,
message: err.toString(),
});
});

console.log(`[${slice}|FAIL] ${err}`);
throw err;
// In case the error message is a multi-line string we want to ensure that we contain
// the error message within the slice (otherwise they'll be moved to the "default" slice of the phase)
err.toString()
.split("\n")
.forEach((line: string) => console.log(`[${slice}] ${line}`));

// The UI shows the last log of the slice which might not make a lot of sense
// for multi-line error messages, so instead we tell the user to expand the slice.
console.log(`[${slice}] Failed. Expand to see why`);
console.log(`[${slice}|FAIL]`);
throw new FailedSliceError(slice);
}

/**
Expand Down