Skip to content

Commit 1655bec

Browse files
devin-ai-integration[bot]lrapoport-cfpetebacondarwin
authored
fix: report startup errors before workerd profiling (#10169)
* 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 Co-Authored-By: [email protected] <[email protected]> * 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 Co-Authored-By: [email protected] <[email protected]> * 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 Co-Authored-By: [email protected] <[email protected]> * 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 Co-Authored-By: [email protected] <[email protected]> * 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 Co-Authored-By: [email protected] <[email protected]> * 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 Co-Authored-By: [email protected] <[email protected]> * 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 <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 82c1abc commit 1655bec

File tree

10 files changed

+385
-122
lines changed

10 files changed

+385
-122
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: 76 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,12 @@ import { http, HttpResponse } from "msw";
1111
import dedent from "ts-dedent";
1212
import { vi } from "vitest";
1313
import { findWranglerConfig } from "../config/config-helpers";
14-
import {
15-
printBundleSize,
16-
printOffendingDependencies,
17-
} from "../deployment-bundle/bundle-reporter";
14+
import { printBundleSize } from "../deployment-bundle/bundle-reporter";
1815
import { clearOutputFilePath } from "../output";
1916
import { sniffUserAgent } from "../package-manager";
17+
import { ParseError } from "../parse";
2018
import { writeAuthConfigFile } from "../user";
19+
import { diagnoseScriptSizeError } from "../utils/friendly-validator-errors";
2120
import { mockAccountId, mockApiToken } from "./helpers/mock-account-id";
2221
import { mockAuthDomain } from "./helpers/mock-auth-domain";
2322
import { mockConsoleMethods } from "./helpers/mock-console";
@@ -11178,7 +11177,23 @@ export default{
1117811177
expect(std).toMatchInlineSnapshot(`
1117911178
Object {
1118011179
"debug": "",
11181-
"err": "",
11180+
"err": "X [ERROR] Your Worker failed validation because it exceeded size limits.
11181+
11182+
11183+
A request to the Cloudflare API (/accounts/some-account-id/workers/scripts/test-name/versions)
11184+
failed.
11185+
- workers.api.error.script_too_large [code: 10027]
11186+
Here are the 4 largest dependencies included in your script:
11187+
11188+
- index.js - xx KiB
11189+
- add.wasm - xx KiB
11190+
- dependency.js - xx KiB
11191+
- message.txt - xx KiB
11192+
11193+
If these are unnecessary, consider removing them
11194+
11195+
11196+
",
1118211197
"info": "",
1118311198
"out": "Total Upload: xx KiB / gzip: xx KiB
1118411199
@@ -11190,15 +11205,7 @@ export default{
1119011205
https://github.com/cloudflare/workers-sdk/issues/new/choose
1119111206
1119211207
",
11193-
"warn": "▲ [WARNING] Here are the 4 largest dependencies included in your script:
11194-
11195-
- index.js - xx KiB
11196-
- add.wasm - xx KiB
11197-
- dependency.js - xx KiB
11198-
- message.txt - xx KiB
11199-
If these are unnecessary, consider removing them
11200-
11201-
",
11208+
"warn": "",
1120211209
}
1120311210
`);
1120411211
});
@@ -11239,9 +11246,43 @@ export default{
1123911246
main: "index.js",
1124011247
});
1124111248

11242-
await expect(runWrangler("deploy")).rejects.toThrowError(
11243-
`Your Worker failed validation because it exceeded startup limits.`
11244-
);
11249+
await expect(runWrangler("deploy")).rejects.toThrowError();
11250+
expect(std).toMatchInlineSnapshot(`
11251+
Object {
11252+
"debug": "",
11253+
"err": "X [ERROR] Your Worker failed validation because it exceeded startup limits.
11254+
11255+
11256+
A request to the Cloudflare API (/accounts/some-account-id/workers/scripts/test-name/versions)
11257+
failed.
11258+
- Error: Script startup exceeded CPU time limit. [code: 10021]
11259+
11260+
To ensure fast responses, there are constraints on Worker startup, such as how much CPU it can
11261+
use, or how long it can take. Your Worker has hit one of these startup limits. Try reducing the
11262+
amount of work done during startup (outside the event handler), either by removing code or
11263+
relocating it inside the event handler.
11264+
11265+
Refer to https://developers.cloudflare.com/workers/platform/limits/#worker-startup-time for more
11266+
details
11267+
A CPU Profile of your Worker's startup phase has been written to
11268+
.wrangler/tmp/startup-profile-<HASH>/worker.cpuprofile - load it into the Chrome DevTools profiler
11269+
(or directly in VSCode) to view a flamegraph.
11270+
11271+
",
11272+
"info": "",
11273+
"out": "Total Upload: xx KiB / gzip: xx KiB
11274+
11275+
X [ERROR] A request to the Cloudflare API (/accounts/some-account-id/workers/scripts/test-name/versions) failed.
11276+
11277+
Error: Script startup exceeded CPU time limit. [code: 10021]
11278+
11279+
If you think this is a bug, please open an issue at:
11280+
https://github.com/cloudflare/workers-sdk/issues/new/choose
11281+
11282+
",
11283+
"warn": "",
11284+
}
11285+
`);
1124511286
});
1124611287

1124711288
describe("unit tests", () => {
@@ -11286,25 +11327,26 @@ export default{
1128611327
"node_modules/k-mod/module.js": { bytesInOutput: 79 },
1128711328
};
1128811329

11289-
printOffendingDependencies(deps);
11290-
expect(std).toMatchInlineSnapshot(`
11291-
Object {
11292-
"debug": "",
11293-
"err": "",
11294-
"info": "",
11295-
"out": "",
11296-
"warn": "▲ [WARNING] Here are the 5 largest dependencies included in your script:
11330+
const message = diagnoseScriptSizeError(
11331+
new ParseError({ text: "too big" }),
11332+
deps
11333+
);
11334+
expect(message).toMatchInlineSnapshot(`
11335+
"Your Worker failed validation because it exceeded size limits.
1129711336
11298-
- node_modules/d-mod/module.js - xx KiB
11299-
- node_modules/g-mod/module.js - xx KiB
11300-
- node_modules/e-mod/module.js - xx KiB
11301-
- node_modules/i-mod/module.js - xx KiB
11302-
- node_modules/j-mod/module.js - xx KiB
11303-
If these are unnecessary, consider removing them
11337+
too big
1130411338
11305-
",
11306-
}
11307-
`);
11339+
Here are the 5 largest dependencies included in your script:
11340+
11341+
- node_modules/d-mod/module.js - 2061.72 KiB
11342+
- node_modules/g-mod/module.js - 77.05 KiB
11343+
- node_modules/e-mod/module.js - 8.02 KiB
11344+
- node_modules/i-mod/module.js - 1.95 KiB
11345+
- node_modules/j-mod/module.js - 0.88 KiB
11346+
11347+
If these are unnecessary, consider removing them
11348+
"
11349+
`);
1130811350
});
1130911351
});
1131011352
});
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: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,11 @@ export function normalizeString(input: string): string {
1313
normalizeErrorMarkers(
1414
replaceByte(
1515
stripTrailingWhitespace(
16-
normalizeSlashes(
17-
normalizeCwd(
18-
normalizeTempDirs(stripTimings(replaceThinSpaces(input)))
16+
stripStartupProfileHash(
17+
normalizeSlashes(
18+
normalizeCwd(
19+
normalizeTempDirs(stripTimings(replaceThinSpaces(input)))
20+
)
1921
)
2022
)
2123
)
@@ -25,6 +27,10 @@ export function normalizeString(input: string): string {
2527
);
2628
}
2729

30+
function stripStartupProfileHash(str: string): string {
31+
return str.replace(/startup-profile-[^/]+/g, "startup-profile-<HASH>");
32+
}
33+
2834
function normalizeTables(str: string): string {
2935
return str
3036
.replaceAll(/+/g, "┌─")

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
@@ -949,12 +949,15 @@ See https://developers.cloudflare.com/workers/platform/compatibility-dates for m
949949
config.tail_consumers
950950
);
951951
}
952-
await helpIfErrorIsSizeOrScriptStartup(
952+
const message = await helpIfErrorIsSizeOrScriptStartup(
953953
err,
954954
dependencies,
955955
workerBundle,
956956
props.projectRoot
957957
);
958+
if (message !== null) {
959+
logger.error(message);
960+
}
958961

959962
// Apply source mapping to validation startup errors if possible
960963
if (

0 commit comments

Comments
 (0)