Skip to content

Commit

Permalink
fix: improve the rendering of build errors when bundling
Browse files Browse the repository at this point in the history
  • Loading branch information
petebacondarwin committed Oct 4, 2024
1 parent 1caa7fd commit 28a4596
Show file tree
Hide file tree
Showing 16 changed files with 153 additions and 98 deletions.
5 changes: 5 additions & 0 deletions .changeset/famous-camels-leave.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"wrangler": patch
---

fix: improve the rendering of build errors when bundling
76 changes: 34 additions & 42 deletions packages/wrangler/src/__tests__/deploy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8940,22 +8940,20 @@ addEventListener('fetch', event => {});`
);

await expect(
runWrangler("deploy index.js --dry-run").catch(
(e) =>
normalizeString(
esbuild
.formatMessagesSync(e?.errors ?? [], { kind: "error" })
.join()
.trim()
).split("This error came from the")[0]
runWrangler("deploy index.js --dry-run").catch((e) =>
normalizeString(
esbuild
.formatMessagesSync(e?.errors ?? [], { kind: "error" })
.join()
.trim()
)
)
).resolves.toMatchInlineSnapshot(`
"X [ERROR] Unexpected external import of \\"cloudflare:sockets\\". Imports are not valid in a Service Worker format Worker.
Did you mean to create a Module Worker?
"X [ERROR] Unexpected external import of \\"cloudflare:sockets\\".
Your worker has no default export, which means it is assumed to be a Service Worker format Worker.
Did you mean to create a ES Module format Worker?
If so, try adding \`export default { ... }\` in your entry-point.
See https://developers.cloudflare.com/workers/reference/migrate-to-module-workers/. [plugin Cloudflare internal imports plugin]
"
See https://developers.cloudflare.com/workers/reference/migrate-to-module-workers/. [plugin cloudflare-internal-imports]"
`);
});

Expand All @@ -8973,24 +8971,20 @@ addEventListener('fetch', event => {});`
);

await expect(
runWrangler("deploy index.js --dry-run").catch(
(e) =>
normalizeString(
esbuild
.formatMessagesSync(e?.errors ?? [], { kind: "error" })
.join()
.trim()
).split("This error came from the")[0]
runWrangler("deploy index.js --dry-run").catch((e) =>
normalizeString(
esbuild
.formatMessagesSync(e?.errors ?? [], { kind: "error" })
.join()
.trim()
)
)
).resolves.toMatchInlineSnapshot(`
"X [ERROR]
Unexpected external import of \\"node:stream\\". Imports are not valid in a Service Worker format Worker.
Did you mean to create a Module Worker?
If so, try adding \`export default { ... }\` in your entry-point.
See https://developers.cloudflare.com/workers/reference/migrate-to-module-workers/.
[plugin nodejs_compat imports plugin]
"
"X [ERROR] Unexpected external import of \\"node:stream\\".
Your worker has no default export, which means it is assumed to be a Service Worker format Worker.
Did you mean to create a ES Module format Worker?
If so, try adding \`export default { ... }\` in your entry-point.
See https://developers.cloudflare.com/workers/reference/migrate-to-module-workers/. [plugin nodejs_compat-imports]"
`);
});

Expand All @@ -9009,22 +9003,20 @@ addEventListener('fetch', event => {});`
);

await expect(
runWrangler("deploy index.js --dry-run").catch(
(e) =>
normalizeString(
esbuild
.formatMessagesSync(e?.errors ?? [], { kind: "error" })
.join()
.trim()
).split("This error came from the")[0]
runWrangler("deploy index.js --dry-run").catch((e) =>
normalizeString(
esbuild
.formatMessagesSync(e?.errors ?? [], { kind: "error" })
.join()
.trim()
)
)
).resolves.toMatchInlineSnapshot(`
"X [ERROR] Unexpected external import of \\"node:stream\\" and \\"node:timers/promises\\". Imports are not valid in a Service Worker format Worker.
Did you mean to create a Module Worker?
"X [ERROR] Unexpected external import of \\"node:stream\\" and \\"node:timers/promises\\".
Your worker has no default export, which means it is assumed to be a Service Worker format Worker.
Did you mean to create a ES Module format Worker?
If so, try adding \`export default { ... }\` in your entry-point.
See https://developers.cloudflare.com/workers/reference/migrate-to-module-workers/. [plugin unenv-cloudflare]
"
See https://developers.cloudflare.com/workers/reference/migrate-to-module-workers/. [plugin hybrid-nodejs_compat]"
`);
});
});
Expand Down
14 changes: 14 additions & 0 deletions packages/wrangler/src/deployment-bundle/build-failures.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,3 +55,17 @@ export function isBuildFailure(err: unknown): err is esbuild.BuildFailure {
"warnings" in err
);
}

/**
* Returns true if the error has a cause that is like an esbuild BuildFailure object.
*/
export function isBuildFailureFromCause(
err: unknown
): err is { cause: esbuild.BuildFailure } {
return (
typeof err === "object" &&
err !== null &&
"cause" in err &&
isBuildFailure(err.cause)
);
}
16 changes: 15 additions & 1 deletion packages/wrangler/src/deployment-bundle/bundle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -467,7 +467,11 @@ export async function bundleWorker(
await ctx.watch();
result = await initialBuildResultPromise;
if (result.errors.length > 0) {
throw new UserError("Failed to build");
throw new BuildFailure(
"Initial build failed.",
result.errors,
result.warnings
);
}

stop = async function () {
Expand Down Expand Up @@ -535,3 +539,13 @@ export async function bundleWorker(
},
};
}

class BuildFailure extends Error {
constructor(
message: string,
readonly errors: esbuild.Message[],
readonly warnings: esbuild.Message[]
) {
super(message);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ export function esbuildAliasExternalPlugin(
aliases: Record<string, string>
): Plugin {
return {
name: "external alias imports plugin",
name: "external-alias-imports",
setup(build) {
build.onResolve({ filter: /.*/g }, (args) => {
// If it's the entrypoint, let it be as is
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import type { Plugin } from "esbuild";
* An esbuild plugin that will mark `node:async_hooks` imports as external.
*/
export const asyncLocalStoragePlugin: Plugin = {
name: "Mark async local storage imports as external plugin",
name: "async-local-storage-imports",
setup(pluginBuild) {
pluginBuild.onResolve({ filter: /^node:async_hooks(\/|$)/ }, () => {
return { external: true };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import type { Plugin } from "esbuild";
* An esbuild plugin that will mark any `cloudflare:...` imports as external.
*/
export const cloudflareInternalPlugin: Plugin = {
name: "Cloudflare internal imports plugin",
name: "cloudflare-internal-imports",
setup(pluginBuild) {
const paths = new Set();
pluginBuild.onStart(() => paths.clear());
Expand All @@ -23,14 +23,19 @@ export const cloudflareInternalPlugin: Plugin = {
.map((p) => `"${p}"`)
.sort()
);
throw new Error(
dedent`
Unexpected external import of ${pathList}. Imports are not valid in a Service Worker format Worker.
Did you mean to create a Module Worker?
If so, try adding \`export default { ... }\` in your entry-point.
See https://developers.cloudflare.com/workers/reference/migrate-to-module-workers/.
`
);
return {
errors: [
{
text: dedent`
Unexpected external import of ${pathList}.
Your worker has no default export, which means it is assumed to be a Service Worker format Worker.
Did you mean to create a ES Module format Worker?
If so, try adding \`export default { ... }\` in your entry-point.
See https://developers.cloudflare.com/workers/reference/migrate-to-module-workers/.
`,
},
],
};
}
});
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export function configProviderPlugin(
config: Record<string, Record<string, unknown>>
): Plugin {
return {
name: "middleware config provider plugin",
name: "middleware-config-provider",
setup(build) {
build.onResolve({ filter: /^config:/ }, (args) => ({
path: args.path,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ const REQUIRED_NODE_BUILT_IN_NAMESPACE = "node-built-in-modules";
export const nodejsHybridPlugin: () => Plugin = () => {
const { alias, inject, external } = env(nodeless, cloudflare);
return {
name: "unenv-cloudflare",
name: "hybrid-nodejs_compat",
setup(build) {
errorOnServiceWorkerFormat(build);
handleRequireCallsToNodeJSBuiltins(build);
Expand Down Expand Up @@ -42,14 +42,19 @@ function errorOnServiceWorkerFormat(build: PluginBuild) {
.map((p) => `"${p}"`)
.sort()
);
throw new Error(
dedent`
Unexpected external import of ${pathList}. Imports are not valid in a Service Worker format Worker.
Did you mean to create a Module Worker?
If so, try adding \`export default { ... }\` in your entry-point.
See https://developers.cloudflare.com/workers/reference/migrate-to-module-workers/.
`
);
return {
errors: [
{
text: dedent`
Unexpected external import of ${pathList}.
Your worker has no default export, which means it is assumed to be a Service Worker format Worker.
Did you mean to create a ES Module format Worker?
If so, try adding \`export default { ... }\` in your entry-point.
See https://developers.cloudflare.com/workers/reference/migrate-to-module-workers/.
`,
},
],
};
}
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,22 +20,27 @@ export function logBuildOutput(
onStart?.();
});
build.onEnd(async ({ errors, warnings }) => {
if (errors.length > 0) {
if (nodejsCompatMode !== "legacy") {
rewriteNodeCompatBuildFailure(errors, nodejsCompatMode);
}
logBuildFailure(errors, warnings);
return;
}

if (warnings.length > 0) {
logBuildWarnings(warnings);
}

if (!bundled) {
// First bundle, no need to update bundle
// First bundle, no need to update bundle or log errors
bundled = true;

// But we still want to log warnings as these are not repeated in first-time build failures
if (warnings.length > 0) {
logBuildWarnings(warnings);
}
} else {
if (errors.length > 0) {
if (nodejsCompatMode !== "legacy") {
rewriteNodeCompatBuildFailure(errors, nodejsCompatMode);
}
logBuildFailure(errors, warnings);
return;
}

if (warnings.length > 0) {
logBuildWarnings(warnings);
}

await updateBundle?.();
}
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import type { Plugin } from "esbuild";
export const nodejsCompatPlugin: (silenceWarnings: boolean) => Plugin = (
silenceWarnings
) => ({
name: "nodejs_compat imports plugin",
name: "nodejs_compat-imports",
setup(pluginBuild) {
// Infinite loop detection
const seen = new Set<string>();
Expand Down Expand Up @@ -71,20 +71,19 @@ export const nodejsCompatPlugin: (silenceWarnings: boolean) => Plugin = (
.map((p) => `"${p}"`)
.sort()
);
throw new Error(`
Unexpected external import of ${paths}. Imports are not valid in a Service Worker format Worker.
Did you mean to create a Module Worker?
If so, try adding \`export default { ... }\` in your entry-point.
See https://developers.cloudflare.com/workers/reference/migrate-to-module-workers/.
`);
const errors = Array.from(warnedPackaged.entries()).map(
([path, importers]) =>
`Unexpected import "${path}" which is not valid in a Service Worker format Worker. Are you missing \`export default { ... }\` from your Worker?\n` +
"Imported from:\n" +
toList(importers, pluginBuild.initialOptions.absWorkingDir) +
"\n"
);
throw new Error(errors.join(""));
return {
errors: [
{
text: dedent`
Unexpected external import of ${paths}.
Your worker has no default export, which means it is assumed to be a Service Worker format Worker.
Did you mean to create a ES Module format Worker?
If so, try adding \`export default { ... }\` in your entry-point.
See https://developers.cloudflare.com/workers/reference/migrate-to-module-workers/.
`,
},
],
};
}
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import type { Plugin } from "esbuild";
* An esbuild plugin that will export URL from the url polyfill
*/
export const standardURLPlugin: () => Plugin = () => ({
name: "standard URL plugin",
name: "standard-URL",
setup(pluginBuild) {
pluginBuild.onResolve({ filter: /^node:url$|^url$/ }, ({ importer }) => {
if (importer === "standard-url-plugin") {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ export default async function guessWorkerFormat(
metafile: true,
bundle: false,
write: false,
logLevel: "silent",
...(tsconfig && { tsconfig }),
});

Expand Down
9 changes: 7 additions & 2 deletions packages/wrangler/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,10 @@ import { d1 } from "./d1";
import { deleteHandler, deleteOptions } from "./delete";
import { deployHandler, deployOptions } from "./deploy";
import { isAuthenticationError } from "./deploy/deploy";
import { isBuildFailure } from "./deployment-bundle/build-failures";
import {
isBuildFailure,
isBuildFailureFromCause,
} from "./deployment-bundle/build-failures";
import {
commonDeploymentCMDSetup,
deployments,
Expand Down Expand Up @@ -904,7 +907,9 @@ export async function main(argv: string[]): Promise<void> {
} else if (isBuildFailure(e)) {
mayReport = false;
logBuildFailure(e.errors, e.warnings);
logger.error(e.message);
} else if (isBuildFailureFromCause(e)) {
mayReport = false;
logBuildFailure(e.cause.errors, e.cause.warnings);
} else {
let loggableException = e;
if (
Expand Down
10 changes: 7 additions & 3 deletions packages/wrangler/src/logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -190,9 +190,13 @@ export function logBuildWarnings(warnings: Message[]) {
* style esbuild would.
*/
export function logBuildFailure(errors: Message[], warnings: Message[]) {
const logs = formatMessagesSync(errors, { kind: "error", color: true });
for (const log of logs) {
console.error(log);
if (errors.length > 0) {
const logs = formatMessagesSync(errors, { kind: "error", color: true });
const errorStr = errors.length > 1 ? "errors" : "error";
logger.error(
`Build failed with ${errors.length} ${errorStr}:\n` + logs.join("\n")
);
}

logBuildWarnings(warnings);
}
Loading

0 comments on commit 28a4596

Please sign in to comment.