Skip to content

Commit

Permalink
fix(console)!: not all diagnostics are errors (#6975)
Browse files Browse the repository at this point in the history 
The compile function from the @winglang/compiler package throws even if the compilation was successful (e.g. the compiler emitted a warning). As result of that behavior, the console shows a blue screen of death when the compiler emits a warning.

This changeset refactors compile so it doesn't throw anymore, but returns the output directory (if possible) along with the diagnostics and any preflight errors.

Co-authored-by: Cristian Pallarés <[email protected]>
  • Loading branch information
@Chriscbr @skyrpex
Chriscbr and skyrpex authored Aug 2, 2024
1 parent cd71831 commit a8d5057
Show file tree
Hide file tree
Showing 6 changed files with 184 additions and 125 deletions.
63 changes: 38 additions & 25 deletions apps/wing-console/console/server/src/utils/compiler.ts
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import path from "node:path";

import * as wing from "@winglang/compiler";
import { CompileError } from "@winglang/compiler";
import { loadEnvVariables } from "@winglang/sdk/lib/helpers";
import chokidar from "chokidar";
import Emittery from "emittery";
Expand Down Expand Up @@ -54,38 +55,50 @@ export const createCompiler = ({

loadEnvVariables({ cwd: dirname });

try {
isCompiling = true;
await events.emit("compiling");
simfile = await wing.compile(wingfile, {
isCompiling = true;
await events.emit("compiling");

const { outputDir, wingcErrors, preflightError } = await wing.compile(
wingfile,
{
platform,
testing,
preflightLog,
});
await events.emit("compiled", { simfile });
} catch (error) {
// There's no point in showing errors if we're going to recompile anyway.
if (shouldCompileAgain) {
return;
}
},
);

await events.emit(
"error",
new Error(
`Failed to compile.\n\n${await formatWingError(error, wingfile)}`,
{
cause: error,
},
),
);
} finally {
isCompiling = false;
isCompiling = false;
if (shouldCompileAgain) {
shouldCompileAgain = false;
await recompile();
return;
}

if (shouldCompileAgain) {
shouldCompileAgain = false;
await recompile();
let errorString = "";
if (wingcErrors.length > 0) {
errorString += await formatWingError(
new CompileError(wingcErrors),
wingfile,
);
}
if (preflightError) {
errorString += await formatWingError(preflightError, wingfile);
}
if (errorString) {
if (outputDir === undefined) {
// Emit a blue screen of death error if there is no output directory.
await events.emit("error", new Error(errorString));
return;
} else {
// Log the error if there is an output directory.
preflightLog?.(errorString);
}
}

if (outputDir) {
simfile = outputDir;
await events.emit("compiled", { simfile });
}
};

const pathsToWatch = [
Expand Down
4 changes: 2 additions & 2 deletions apps/wing-console/console/server/src/utils/format-wing-error.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ export const formatWingError = async (error: unknown, entryPoint?: string) => {
const result = [];

for (const error of errors) {
const { message, span, annotations, hints } = error;
const { message, span, annotations, hints, severity } = error;
const files: File[] = [];
const labels: Label[] = [];
const cwd = process.cwd();
Expand Down Expand Up @@ -59,7 +59,7 @@ export const formatWingError = async (error: unknown, entryPoint?: string) => {
files,
{
message,
severity: "error",
severity,
labels,
notes: hints.map((hint) => `hint: ${hint}`),
},
Expand Down
172 changes: 92 additions & 80 deletions apps/wing/src/commands/compile.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,94 +86,106 @@ export async function compile(entrypoint?: string, options?: CompileOptions): Pr
}
loadEnvVariables({ cwd: resolve(dirname(entrypoint)) });
const coloring = chalk.supportsColor ? chalk.supportsColor.hasBasic : false;
try {
return await wingCompiler.compile(entrypoint, {
...options,
log,
color: coloring,
platform: options?.platform ?? ["sim"],
});
} catch (error) {
if (error instanceof wingCompiler.CompileError) {
// This is a bug in the user's code. Print the compiler diagnostics.
const diagnostics = error.diagnostics;
const cwd = process.cwd();
const result = [];

for (const diagnostic of diagnostics) {
const { message, span, annotations, hints, severity } = diagnostic;
const files: File[] = [];
const labels: Label[] = [];
const compileOutput = await wingCompiler.compile(entrypoint, {
...options,
log,
color: coloring,
platform: options?.platform ?? ["sim"],
});
if (compileOutput.wingcErrors.length > 0) {
// Print any errors or warnings from the compiler.
const diagnostics = compileOutput.wingcErrors;
const cwd = process.cwd();
const result = [];

for (const diagnostic of diagnostics) {
const { message, span, annotations, hints, severity } = diagnostic;
const files: File[] = [];
const labels: Label[] = [];

// file_id might be "" if the span is synthetic (see #2521)
if (span?.file_id) {
// `span` should only be null if source file couldn't be read etc.
const source = await fsPromise.readFile(span.file_id, "utf8");
const start = span.start_offset;
const end = span.end_offset;
const filePath = relative(cwd, span.file_id);
files.push({ name: filePath, source });
labels.push({
fileId: filePath,
rangeStart: start,
rangeEnd: end,
message: "",
style: "primary",
});
}

for (const annotation of annotations) {
// file_id might be "" if the span is synthetic (see #2521)
if (span?.file_id) {
// `span` should only be null if source file couldn't be read etc.
const source = await fsPromise.readFile(span.file_id, "utf8");
const start = span.start_offset;
const end = span.end_offset;
const filePath = relative(cwd, span.file_id);
files.push({ name: filePath, source });
labels.push({
fileId: filePath,
rangeStart: start,
rangeEnd: end,
message: "",
style: "primary",
});
if (!annotation.span?.file_id) {
continue;
}
const source = await fsPromise.readFile(annotation.span.file_id, "utf8");
const start = annotation.span.start_offset;
const end = annotation.span.end_offset;
const filePath = relative(cwd, annotation.span.file_id);
files.push({ name: filePath, source });
labels.push({
fileId: filePath,
rangeStart: start,
rangeEnd: end,
message: annotation.message,
style: "secondary",
});
}

for (const annotation of annotations) {
// file_id might be "" if the span is synthetic (see #2521)
if (!annotation.span?.file_id) {
continue;
}
const source = await fsPromise.readFile(annotation.span.file_id, "utf8");
const start = annotation.span.start_offset;
const end = annotation.span.end_offset;
const filePath = relative(cwd, annotation.span.file_id);
files.push({ name: filePath, source });
labels.push({
fileId: filePath,
rangeStart: start,
rangeEnd: end,
message: annotation.message,
style: "secondary",
});
}
const diagnosticText = emitDiagnostic(
files,
{
message,
severity,
labels,
notes: hints.map((hint) => `hint: ${hint}`),
},
{
chars: CHARS_ASCII,
},
coloring
);
result.push(diagnosticText);
}

const diagnosticText = emitDiagnostic(
files,
{
message,
severity,
labels,
notes: hints.map((hint) => `hint: ${hint}`),
},
{
chars: CHARS_ASCII,
},
coloring
);
result.push(diagnosticText);
}
if (compileOutput.wingcErrors.map((e) => e.severity).includes("error")) {
throw new Error(result.join("\n").trimEnd());
} else if (error instanceof wingCompiler.PreflightError) {
let output = await prettyPrintError(error.causedBy, {
chalk,
sourceEntrypoint: resolve(entrypoint ?? "."),
});

if (process.env.DEBUG) {
output +=
"\n--------------------------------- ORIGINAL STACK TRACE ---------------------------------\n" +
(error.causedBy.stack ?? "(no stacktrace available)");
}
} else {
console.error(result.join("\n").trimEnd());
}
}

error.causedBy.message = output;
if (compileOutput.preflightError) {
const error = compileOutput.preflightError;
let output = await prettyPrintError(error.causedBy, {
chalk,
sourceEntrypoint: resolve(entrypoint ?? "."),
});

throw error.causedBy;
} else {
throw error;
if (process.env.DEBUG) {
output +=
"\n--------------------------------- ORIGINAL STACK TRACE ---------------------------------\n" +
(error.causedBy.stack ?? "(no stacktrace available)");
}

error.causedBy.message = output;

throw error.causedBy;
}

if (compileOutput.outputDir === undefined) {
// If "outputDir" is undefined, then one or more errors should have been found, so there must be a logical bug.
throw new Error(
"Internal compilation error. Please report this as a bug on the Wing issue tracker."
);
}

return compileOutput.outputDir;
}
22 changes: 16 additions & 6 deletions libs/wingcompiler/src/compile.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,17 @@ import { join, resolve, basename } from "path";
import { stat, mkdtemp } from "fs/promises";
import { tmpdir } from "os";
import { BuiltinPlatform } from "./constants";
import { compile } from "./compile";
import { compile, CompileOptions } from "./compile";

const compileOrFail = async (entrypoint: string, options: CompileOptions) => {
const result = await compile(entrypoint, options);

if (!result.outputDir) {
throw new Error("Compilation failed");
}

return result.outputDir;
};

const exampleDir = resolve("../../examples/tests/valid");
const exampleFilePath = join(exampleDir, "enums.test.w");
Expand All @@ -15,7 +25,7 @@ export async function generateTmpDir() {
describe("compile tests", () => {
test("should produce stable artifacts for tf-aws", async () => {
const targetDir = `${await generateTmpDir()}/target`;
const artifactDir = await compile(exampleFilePath, {
const artifactDir = await compileOrFail(exampleFilePath, {
platform: [BuiltinPlatform.TF_AWS],
targetDir,
});
Expand All @@ -28,7 +38,7 @@ describe("compile tests", () => {

test("should produce temp artifacts for tf-aws testing", async () => {
const targetDir = `${await generateTmpDir()}/target`;
const artifactDir = await compile(exampleFilePath, {
const artifactDir = await compileOrFail(exampleFilePath, {
platform: [BuiltinPlatform.TF_AWS],
targetDir,
testing: true,
Expand All @@ -42,7 +52,7 @@ describe("compile tests", () => {

test("should produce stable artifacts for sim", async () => {
const targetDir = `${await generateTmpDir()}/target`;
const artifactDir = await compile(exampleFilePath, {
const artifactDir = await compileOrFail(exampleFilePath, {
platform: [BuiltinPlatform.SIM],
targetDir,
});
Expand All @@ -55,7 +65,7 @@ describe("compile tests", () => {

test("should produce stable artifacts for sim testing", async () => {
const targetDir = `${await generateTmpDir()}/target`;
const artifactDir = await compile(exampleFilePath, {
const artifactDir = await compileOrFail(exampleFilePath, {
platform: [BuiltinPlatform.SIM],
targetDir,
testing: true,
Expand All @@ -69,7 +79,7 @@ describe("compile tests", () => {

test("should be able to override the target directory", async () => {
const output = `${await generateTmpDir()}/a/b/dir.out`;
const artifactDir = await compile(exampleFilePath, {
const artifactDir = await compileOrFail(exampleFilePath, {
platform: [BuiltinPlatform.SIM],
output,
testing: true,
Expand Down
Loading

0 comments on commit a8d5057

Please sign in to comment.