From bd815158876662d4f7d2a4ac1b3979e00d21654e Mon Sep 17 00:00:00 2001 From: Mads Hartmann Date: Fri, 16 Sep 2022 13:03:04 +0000 Subject: [PATCH] Ensure error message for failed slices stay within the slice --- .werft/build.ts | 9 +++++++-- .werft/jobs/build/validate-changes.ts | 17 ++++++----------- .werft/util/werft.ts | 25 +++++++++++++++++++++---- 3 files changed, 34 insertions(+), 17 deletions(-) diff --git a/.werft/build.ts b/.werft/build.ts index e3b546eae88017..9d16083645735b 100644 --- a/.werft/build.ts +++ b/.werft/build.ts @@ -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"; @@ -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) => { diff --git a/.werft/jobs/build/validate-changes.ts b/.werft/jobs/build/validate-changes.ts index 4bb8eee87e5635..1f5d9c20e4db50 100644 --- a/.werft/jobs/build/validate-changes.ts +++ b/.werft/jobs/build/validate-changes.ts @@ -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. diff --git a/.werft/util/werft.ts b/.werft/util/werft.ts index 612c55f9227c22..bd127d62b26432 100644 --- a/.werft/util/werft.ts +++ b/.werft/util/werft.ts @@ -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 */ @@ -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) { @@ -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); } /**