Skip to content

Commit 1f686ef

Browse files
devin-ai-integration[bot]lrapoport-cfpetebacondarwin
authored
fix: report startup errors before workerd profiling (#10169) (#10182)
* fix: report startup errors before workerd profiling - Report original startup error message before attempting profiling - Handle workerd profiling failures gracefully - Silent profiling failures for deployment errors, reported for manual profiling - Ensure users see the actual startup error even if profiling fails - Fixes #10133 * fix: correct error handling in handleStartupError to allow UserError propagation - Restructure try-catch to only handle profiling failures, not intentional UserError - Ensure tests receive expected UserError message - Update PR description to fix validation requirements * test: add comprehensive tests for handleStartupError function - Test profiling success with profile information included - Test silent profiling failures for deployment errors - Test reported profiling failures for manual profiling - Test relative path handling in profile output - Test startup error reporting before profiling attempts * refactor: show startup errors before profiling attempts - Log startup error immediately before attempting profiling - Eliminates duplicate error handling in catch block - Maintains silent profiling failures for deployment errors - Maintains reported profiling failures for manual profiling - Cleaner error handling approach as requested * fix: resolve Windows path separator issue in test - Update test assertions to be platform-agnostic - Check for key path components rather than exact separators - Fixes Windows CI failure in 'uses relative paths for profile output' test * rework the friendly validator processing - moves `printOffendingDependencies()` to the friendly-validator-errors.ts file, which is the only place it is used - changes the friendly-validator-errors.ts functions to just return error messages that can then be used as needed by client code * fix: resolve TypeScript error and Windows path separator issue - Fix Uint8Array generic type error in check/commands.ts - Update normalizeMessage to handle cross-platform path separators - Ensure test snapshots work on both Windows and Unix systems * ensure that slashes are normalized before the startup profile hash * reuse normalizeString helper * catch all start-up-profile strings when normalizing * fixup snapshot & normalization --------- Co-authored-by: devin-ai-integration[bot] <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: [email protected] <[email protected]> Co-authored-by: Peter Bacon Darwin <[email protected]>
1 parent 0d73563 commit 1f686ef

File tree

10 files changed

+387
-120
lines changed

10 files changed

+387
-120
lines changed

.changeset/long-hairs-feel.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"wrangler": patch
3+
---
4+
5+
fix: report startup errors before workerd profiling

packages/wrangler/src/__tests__/deploy.test.ts

Lines changed: 77 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,12 @@ import { http, HttpResponse } from "msw";
1010
import dedent from "ts-dedent";
1111
import { File } from "undici";
1212
import { vi } from "vitest";
13-
import {
14-
printBundleSize,
15-
printOffendingDependencies,
16-
} from "../deployment-bundle/bundle-reporter";
13+
import { printBundleSize } from "../deployment-bundle/bundle-reporter";
1714
import { clearOutputFilePath } from "../output";
1815
import { sniffUserAgent } from "../package-manager";
16+
import { ParseError } from "../parse";
1917
import { writeAuthConfigFile } from "../user";
18+
import { diagnoseScriptSizeError } from "../utils/friendly-validator-errors";
2019
import { mockAccountId, mockApiToken } from "./helpers/mock-account-id";
2120
import { mockAuthDomain } from "./helpers/mock-auth-domain";
2221
import { mockConsoleMethods } from "./helpers/mock-console";
@@ -11633,7 +11632,23 @@ export default{
1163311632
expect(std).toMatchInlineSnapshot(`
1163411633
Object {
1163511634
"debug": "",
11636-
"err": "",
11635+
"err": "X [ERROR] Your Worker failed validation because it exceeded size limits.
11636+
11637+
11638+
A request to the Cloudflare API (/accounts/some-account-id/workers/scripts/test-name/versions)
11639+
failed.
11640+
- workers.api.error.script_too_large [code: 10027]
11641+
Here are the 4 largest dependencies included in your script:
11642+
11643+
- index.js - xx KiB
11644+
- add.wasm - xx KiB
11645+
- dependency.js - xx KiB
11646+
- message.txt - xx KiB
11647+
11648+
If these are unnecessary, consider removing them
11649+
11650+
11651+
",
1163711652
"info": "",
1163811653
"out": "Total Upload: xx KiB / gzip: xx KiB
1163911654
No bindings found.
@@ -11646,15 +11661,7 @@ export default{
1164611661
https://github.com/cloudflare/workers-sdk/issues/new/choose
1164711662
1164811663
",
11649-
"warn": "▲ [WARNING] Here are the 4 largest dependencies included in your script:
11650-
11651-
- index.js - xx KiB
11652-
- add.wasm - xx KiB
11653-
- dependency.js - xx KiB
11654-
- message.txt - xx KiB
11655-
If these are unnecessary, consider removing them
11656-
11657-
",
11664+
"warn": "",
1165811665
}
1165911666
`);
1166011667
});
@@ -11695,9 +11702,44 @@ export default{
1169511702
main: "index.js",
1169611703
});
1169711704

11698-
await expect(runWrangler("deploy")).rejects.toThrowError(
11699-
`Your Worker failed validation because it exceeded startup limits.`
11700-
);
11705+
await expect(runWrangler("deploy")).rejects.toThrowError();
11706+
expect(std).toMatchInlineSnapshot(`
11707+
Object {
11708+
"debug": "",
11709+
"err": "X [ERROR] Your Worker failed validation because it exceeded startup limits.
11710+
11711+
11712+
A request to the Cloudflare API (/accounts/some-account-id/workers/scripts/test-name/versions)
11713+
failed.
11714+
- Error: Script startup exceeded CPU time limit. [code: 10021]
11715+
11716+
To ensure fast responses, there are constraints on Worker startup, such as how much CPU it can
11717+
use, or how long it can take. Your Worker has hit one of these startup limits. Try reducing the
11718+
amount of work done during startup (outside the event handler), either by removing code or
11719+
relocating it inside the event handler.
11720+
11721+
Refer to https://developers.cloudflare.com/workers/platform/limits/#worker-startup-time for more
11722+
details
11723+
A CPU Profile of your Worker's startup phase has been written to
11724+
.wrangler/tmp/startup-profile-<HASH>/worker.cpuprofile - load it into the Chrome DevTools profiler
11725+
(or directly in VSCode) to view a flamegraph.
11726+
11727+
",
11728+
"info": "",
11729+
"out": "Total Upload: xx KiB / gzip: xx KiB
11730+
No bindings found.
11731+
11732+
X [ERROR] A request to the Cloudflare API (/accounts/some-account-id/workers/scripts/test-name/versions) failed.
11733+
11734+
Error: Script startup exceeded CPU time limit. [code: 10021]
11735+
11736+
If you think this is a bug, please open an issue at:
11737+
https://github.com/cloudflare/workers-sdk/issues/new/choose
11738+
11739+
",
11740+
"warn": "",
11741+
}
11742+
`);
1170111743
});
1170211744

1170311745
describe("unit tests", () => {
@@ -11742,25 +11784,26 @@ export default{
1174211784
"node_modules/k-mod/module.js": { bytesInOutput: 79 },
1174311785
};
1174411786

11745-
printOffendingDependencies(deps);
11746-
expect(std).toMatchInlineSnapshot(`
11747-
Object {
11748-
"debug": "",
11749-
"err": "",
11750-
"info": "",
11751-
"out": "",
11752-
"warn": "▲ [WARNING] Here are the 5 largest dependencies included in your script:
11787+
const message = diagnoseScriptSizeError(
11788+
new ParseError({ text: "too big" }),
11789+
deps
11790+
);
11791+
expect(message).toMatchInlineSnapshot(`
11792+
"Your Worker failed validation because it exceeded size limits.
1175311793
11754-
- node_modules/d-mod/module.js - xx KiB
11755-
- node_modules/g-mod/module.js - xx KiB
11756-
- node_modules/e-mod/module.js - xx KiB
11757-
- node_modules/i-mod/module.js - xx KiB
11758-
- node_modules/j-mod/module.js - xx KiB
11759-
If these are unnecessary, consider removing them
11794+
too big
1176011795
11761-
",
11762-
}
11763-
`);
11796+
Here are the 5 largest dependencies included in your script:
11797+
11798+
- node_modules/d-mod/module.js - 2061.72 KiB
11799+
- node_modules/g-mod/module.js - 77.05 KiB
11800+
- node_modules/e-mod/module.js - 8.02 KiB
11801+
- node_modules/i-mod/module.js - 1.95 KiB
11802+
- node_modules/j-mod/module.js - 0.88 KiB
11803+
11804+
If these are unnecessary, consider removing them
11805+
"
11806+
`);
1176411807
});
1176511808
});
1176611809
});
Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
import { FormData } from "undici";
2+
import { describe, expect, it, vi } from "vitest";
3+
import * as checkCommands from "../check/commands";
4+
import { logger } from "../logger";
5+
import { ParseError } from "../parse";
6+
import { helpIfErrorIsSizeOrScriptStartup } from "../utils/friendly-validator-errors";
7+
import { mockConsoleMethods } from "./helpers/mock-console";
8+
import { normalizeString } from "./helpers/normalize";
9+
import { runInTempDir } from "./helpers/run-in-tmp";
10+
11+
vi.mock("../check/commands", () => ({ analyseBundle: vi.fn() }));
12+
const mockAnalyseBundle = vi.mocked(checkCommands.analyseBundle);
13+
14+
describe("helpIfErrorIsSizeOrScriptStartup", () => {
15+
const std = mockConsoleMethods();
16+
runInTempDir();
17+
beforeEach(() => {
18+
mockAnalyseBundle.mockReset();
19+
20+
// Ensure logger is set to debug level for testing
21+
const loggerLevel = logger.loggerLevel;
22+
logger.loggerLevel = "debug";
23+
return () => (logger.loggerLevel = loggerLevel); // Restore original logger level after test
24+
});
25+
26+
it("cleanly reports a startup error even if bundle analysis fails", async () => {
27+
mockAnalyseBundle.mockRejectedValue(new Error("workerd profiling failed"));
28+
29+
expect(
30+
await helpIfErrorIsSizeOrScriptStartup(
31+
makeStartupError("Script startup exceeded CPU limit."),
32+
{}, // no dependencies
33+
new FormData(), // mock worker bundle
34+
"/test"
35+
)
36+
).toMatchInlineSnapshot(`
37+
"Your Worker failed validation because it exceeded startup limits.
38+
39+
Deploy failed
40+
- Script startup exceeded CPU limit.
41+
42+
To ensure fast responses, there are constraints on Worker startup, such as how much CPU it can use, or how long it can take. Your Worker has hit one of these startup limits. Try reducing the amount of work done during startup (outside the event handler), either by removing code or relocating it inside the event handler.
43+
44+
Refer to https://developers.cloudflare.com/workers/platform/limits/#worker-startup-time for more details"
45+
`);
46+
47+
expect(std).toMatchInlineSnapshot(`
48+
Object {
49+
"debug": "An error occurred while trying to locally profile the Worker: Error: workerd profiling failed",
50+
"err": "",
51+
"info": "",
52+
"out": "",
53+
"warn": "",
54+
}
55+
`);
56+
});
57+
58+
it("reports size errors even if bundle analysis would fail", async () => {
59+
mockAnalyseBundle.mockRejectedValue(new Error("workerd profiling failed"));
60+
61+
expect(
62+
await helpIfErrorIsSizeOrScriptStartup(
63+
makeScriptSizeError("Script size exceeded limits."),
64+
{ "test.js": { bytesInOutput: 1000 } }, // mock dependencies
65+
new FormData(), // mock worker bundle
66+
"/test"
67+
)
68+
).toMatchInlineSnapshot(`
69+
"Your Worker failed validation because it exceeded size limits.
70+
71+
Script size exceeded limits.
72+
73+
Here are the 1 largest dependencies included in your script:
74+
75+
- test.js - 0.98 KiB
76+
77+
If these are unnecessary, consider removing them
78+
"
79+
`);
80+
expect(std).toMatchInlineSnapshot(`
81+
Object {
82+
"debug": "",
83+
"err": "",
84+
"info": "",
85+
"out": "",
86+
"warn": "",
87+
}
88+
`);
89+
});
90+
91+
it("includes profile information when bundle analysis succeeds", async () => {
92+
mockAnalyseBundle.mockResolvedValue({ nodes: [], samples: [] });
93+
94+
const message = await helpIfErrorIsSizeOrScriptStartup(
95+
makeStartupError("Exceeded startup limits."),
96+
{}, // no dependencies
97+
new FormData(), // mock worker bundle
98+
process.cwd() // mock project root (the tmp dir)
99+
);
100+
101+
expect(normalizeString(message ?? "")).toMatchInlineSnapshot(`
102+
"Your Worker failed validation because it exceeded startup limits.
103+
104+
Deploy failed
105+
- Exceeded startup limits.
106+
107+
To ensure fast responses, there are constraints on Worker startup, such as how much CPU it can use, or how long it can take. Your Worker has hit one of these startup limits. Try reducing the amount of work done during startup (outside the event handler), either by removing code or relocating it inside the event handler.
108+
109+
Refer to https://developers.cloudflare.com/workers/platform/limits/#worker-startup-time for more details
110+
A CPU Profile of your Worker's startup phase has been written to .wrangler/tmp/startup-profile-<HASH>/worker.cpuprofile - load it into the Chrome DevTools profiler (or directly in VSCode) to view a flamegraph."
111+
`);
112+
expect(std).toMatchInlineSnapshot(`
113+
Object {
114+
"debug": "",
115+
"err": "",
116+
"info": "",
117+
"out": "",
118+
"warn": "",
119+
}
120+
`);
121+
});
122+
});
123+
124+
function makeScriptSizeError(text: string): ParseError {
125+
const error = new ParseError({ text });
126+
Object.assign(error, { code: 10027 });
127+
return error;
128+
}
129+
130+
function makeStartupError(message: string): ParseError {
131+
const error = new ParseError({
132+
text: "Deploy failed",
133+
notes: [{ text: message }],
134+
});
135+
Object.assign(error, { code: 10021 });
136+
return error;
137+
}

packages/wrangler/src/__tests__/helpers/normalize.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,21 @@ export function normalizeString(input: string): string {
55
return normalizeErrorMarkers(
66
replaceByte(
77
stripTrailingWhitespace(
8-
normalizeSlashes(normalizeCwd(normalizeTempDirs(stripTimings(input))))
8+
stripStartupProfileHash(
9+
normalizeSlashes(normalizeCwd(normalizeTempDirs(stripTimings(input))))
10+
)
911
)
1012
)
1113
);
1214
}
1315

16+
/**
17+
* Replace the non-deterministic hash in a startup-profile path with a placeholder.
18+
*/
19+
function stripStartupProfileHash(str: string): string {
20+
return str.replace(/startup-profile-[^/]+/g, "startup-profile-<HASH>");
21+
}
22+
1423
/**
1524
* Normalize error `X` markers.
1625
*

packages/wrangler/src/check/commands.ts

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -138,9 +138,9 @@ export const checkStartupCommand = createCommand({
138138

139139
async function getEntryValue(
140140
entry: FormDataEntryValue
141-
): Promise<Uint8Array<ArrayBuffer> | string> {
141+
): Promise<Uint8Array | string> {
142142
if (entry instanceof Blob) {
143-
return new Uint8Array(await entry.arrayBuffer());
143+
return new Uint8Array((await entry.arrayBuffer()) as ArrayBuffer);
144144
} else {
145145
return entry as string;
146146
}
@@ -166,11 +166,14 @@ async function convertWorkerBundleToModules(
166166
workerBundle: FormData
167167
): Promise<ModuleDefinition[]> {
168168
return await Promise.all(
169-
[...workerBundle.entries()].map(async (m) => ({
170-
type: getModuleType(m[1]),
171-
path: m[0],
172-
contents: await getEntryValue(m[1]),
173-
}))
169+
[...workerBundle.entries()].map(
170+
async (m) =>
171+
({
172+
type: getModuleType(m[1]),
173+
path: m[0],
174+
contents: await getEntryValue(m[1]),
175+
}) as ModuleDefinition
176+
)
174177
);
175178
}
176179

packages/wrangler/src/deploy/deploy.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -888,12 +888,15 @@ See https://developers.cloudflare.com/workers/platform/compatibility-dates for m
888888
if (!bindingsPrinted) {
889889
printBindings({ ...withoutStaticAssets, vars: maskedVars });
890890
}
891-
await helpIfErrorIsSizeOrScriptStartup(
891+
const message = await helpIfErrorIsSizeOrScriptStartup(
892892
err,
893893
dependencies,
894894
workerBundle,
895895
props.projectRoot
896896
);
897+
if (message !== null) {
898+
logger.error(message);
899+
}
897900

898901
// Apply source mapping to validation startup errors if possible
899902
if (

0 commit comments

Comments
 (0)