Skip to content

Commit 0a554f9

Browse files
authored
Mark more errors as UserError (#10769)
* Mark all containers-shared errors as UserErrors * D1 & Miniflare * fix lint * Update changeset to clarify UserError usage Clarify the purpose of marking errors as UserError. * fix lint * properly handle abort signals
1 parent ed04ed3 commit 0a554f9

File tree

18 files changed

+215
-79
lines changed

18 files changed

+215
-79
lines changed

.changeset/popular-moons-knock.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
"@cloudflare/containers-shared": patch
3+
"miniflare": patch
4+
"wrangler": patch
5+
---
6+
7+
Mark more errors as `UserError` to disable Sentry reporting

fixtures/ratelimit-app/tsconfig.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
{
22
"compilerOptions": {
3-
"target": "ES2020",
3+
"target": "esnext",
44
"esModuleInterop": true,
55
"module": "CommonJS",
6-
"lib": ["ES2020"],
6+
"lib": ["esnext"],
77
"types": ["node"],
88
"skipLibCheck": true,
99
"moduleResolution": "node",

fixtures/worker-ts/wrangler.jsonc

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,6 @@
11
{
2+
"$schema": "node_modules/wrangler/config-schema.json",
23
"name": "worker-ts",
34
"main": "src/index.ts",
45
"compatibility_date": "2023-05-04",
5-
"hyperdrive": [
6-
{
7-
"binding": "H",
8-
"id": "hello",
9-
},
10-
],
116
}

packages/containers-shared/src/build.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { spawn } from "child_process";
22
import { readFileSync } from "fs";
3+
import { UserError } from "./error";
34
import type {
45
BuildArgs,
56
ContainerDevOptions,
@@ -70,7 +71,7 @@ export function dockerBuild(
7071
resolve();
7172
} else if (!errorHandled) {
7273
errorHandled = true;
73-
reject(new Error(`Docker build exited with code: ${code}`));
74+
reject(new UserError(`Docker build exited with code: ${code}`));
7475
}
7576
});
7677
child.on("error", (err) => {
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
/**
2+
* This is used to provide telemetry with a sanitised error
3+
* message that could not have any user-identifying information.
4+
* Set to `true` to duplicate `message`.
5+
* */
6+
type TelemetryMessage = {
7+
telemetryMessage?: string | true;
8+
};
9+
10+
/**
11+
* Base class for errors where the user has done something wrong. These are not
12+
* reported to Sentry. API errors are intentionally *not* `UserError`s, and are
13+
* reported to Sentry. This will help us understand which API errors need better
14+
* messaging.
15+
*/
16+
export class UserError extends Error {
17+
telemetryMessage: string | undefined;
18+
constructor(
19+
message?: string | undefined,
20+
options?:
21+
| ({
22+
cause?: unknown;
23+
} & TelemetryMessage)
24+
| undefined
25+
) {
26+
super(message, options);
27+
// Restore prototype chain:
28+
// https://www.typescriptlang.org/docs/handbook/release-notes/typescript-2-2.html#support-for-newtarget
29+
Object.setPrototypeOf(this, new.target.prototype);
30+
this.telemetryMessage =
31+
options?.telemetryMessage === true ? message : options?.telemetryMessage;
32+
}
33+
}

packages/containers-shared/src/images.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { buildImage } from "./build";
2+
import { UserError } from "./error";
23
import {
34
getCloudflareContainerRegistry,
45
isCloudflareRegistryLink,
@@ -73,7 +74,7 @@ export async function prepareContainerImagesForDev(args: {
7374
} = args;
7475
let aborted = false;
7576
if (process.platform === "win32") {
76-
throw new Error(
77+
throw new UserError(
7778
"Local development with containers is currently not supported on Windows. You should use WSL instead. You can also set `enable_containers` to false if you do not need to develop the container part of your application."
7879
);
7980
}
@@ -95,7 +96,7 @@ export async function prepareContainerImagesForDev(args: {
9596
});
9697
} else {
9798
if (!isCloudflareRegistryLink(options.image_uri)) {
98-
throw new Error(
99+
throw new UserError(
99100
`Image "${options.image_uri}" is a registry link but does not point to the Cloudflare container registry.\n` +
100101
`To use an existing image from another repository, see https://developers.cloudflare.com/containers/image-management/#using-existing-images`
101102
);

packages/containers-shared/src/inspect.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { spawn } from "child_process";
2+
import { UserError } from "./error";
23

34
export async function dockerImageInspect(
45
dockerPath: string,
@@ -22,7 +23,7 @@ export async function dockerImageInspect(
2223
proc.on("close", (code) => {
2324
if (code !== 0) {
2425
return reject(
25-
new Error(`failed inspecting image locally: ${stderr.trim()}`)
26+
new UserError(`failed inspecting image locally: ${stderr.trim()}`)
2627
);
2728
}
2829
resolve(stdout.trim());

packages/containers-shared/src/login.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { spawn } from "node:child_process";
22
import { ImageRegistriesService, ImageRegistryPermissions } from "./client";
3+
import { UserError } from "./error";
34
import { getCloudflareContainerRegistry } from "./knobs";
45

56
/**
@@ -44,7 +45,7 @@ export async function dockerLoginManagedRegistry(pathToDocker: string) {
4445
if (code === 0) {
4546
resolve();
4647
} else {
47-
reject(new Error(`Login failed with code: ${code}`));
48+
reject(new UserError(`Login failed with code: ${code}`));
4849
}
4950
});
5051
});

packages/containers-shared/src/utils.ts

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { execFileSync, spawn } from "child_process";
22
import { randomUUID } from "crypto";
33
import { existsSync, statSync } from "fs";
44
import path from "path";
5+
import { UserError } from "./error";
56
import { dockerImageInspect } from "./inspect";
67
import type { ContainerDevOptions } from "./types";
78
import type { StdioOptions } from "child_process";
@@ -38,13 +39,13 @@ export const runDockerCmd = (
3839
resolve({ aborted });
3940
} else if (!errorHandled) {
4041
errorHandled = true;
41-
reject(new Error(`Docker command exited with code: ${code}`));
42+
reject(new UserError(`Docker command exited with code: ${code}`));
4243
}
4344
});
4445
child.on("error", (err) => {
4546
if (!errorHandled) {
4647
errorHandled = true;
47-
reject(new Error(`Docker command failed: ${err.message}`));
48+
reject(new UserError(`Docker command failed: ${err.message}`));
4849
}
4950
});
5051
return {
@@ -66,7 +67,7 @@ export const runDockerCmdWithOutput = (dockerPath: string, args: string[]) => {
6667
const stdout = execFileSync(dockerPath, args, { encoding: "utf8" });
6768
return stdout.trim();
6869
} catch (error) {
69-
throw new Error(
70+
throw new UserError(
7071
`Failed running docker command: ${(error as Error).message}. Command: ${dockerPath} ${args.join(" ")}`
7172
);
7273
}
@@ -81,7 +82,7 @@ export const verifyDockerInstalled = async (
8182
await runDockerCmd(dockerPath, ["info"], ["inherit", "pipe", "pipe"]);
8283
} catch {
8384
// We assume this command is unlikely to fail for reasons other than the Docker daemon not running, or the Docker CLI not being installed or in the PATH.
84-
throw new Error(
85+
throw new UserError(
8586
`The Docker CLI could not be launched. Please ensure that the Docker CLI is installed and the daemon is running.\n` +
8687
`Other container tooling that is compatible with the Docker CLI and engine may work, but is not yet guaranteed to do so. You can specify an executable with the environment variable WRANGLER_DOCKER_BIN and a socket with DOCKER_HOST.` +
8788
`${isDev ? "\nTo suppress this error if you do not intend on triggering any container instances, set dev.enable_containers to false in your Wrangler config or passing in --enable-containers=false." : ""}`
@@ -103,7 +104,7 @@ export const isDockerfile = (
103104
const maybeDockerfile = path.resolve(baseDir, image);
104105
if (existsSync(maybeDockerfile)) {
105106
if (isDir(maybeDockerfile)) {
106-
throw new Error(
107+
throw new UserError(
107108
`${image} is a directory, you should specify a path to the Dockerfile`
108109
);
109110
}
@@ -116,22 +117,22 @@ export const isDockerfile = (
116117
new URL(`https://${image}`);
117118
} catch (e) {
118119
if (e instanceof Error) {
119-
throw new Error(errorPrefix + e.message);
120+
throw new UserError(errorPrefix + e.message);
120121
}
121122
throw e;
122123
}
123124
const imageParts = image.split("/");
124125

125126
if (!imageParts[imageParts.length - 1]?.includes(":")) {
126-
throw new Error(
127+
throw new UserError(
127128
errorPrefix +
128129
`If this is an image registry path, it needs to include at least a tag ':' (e.g: docker.io/httpd:1)`
129130
);
130131
}
131132

132133
// validate URL
133134
if (image.includes("://")) {
134-
throw new Error(
135+
throw new UserError(
135136
errorPrefix +
136137
`Image reference should not include the protocol part (e.g: docker.io/httpd:1, not https://docker.io/httpd:1)`
137138
);
@@ -218,7 +219,7 @@ export async function checkExposedPorts(
218219
formatString: "{{ len .Config.ExposedPorts }}",
219220
});
220221
if (output === "0") {
221-
throw new Error(
222+
throw new UserError(
222223
`The container "${options.class_name}" does not expose any ports. In your Dockerfile, please expose any ports you intend to connect to.\n` +
223224
"For additional information please see: https://developers.cloudflare.com/containers/local-dev/#exposing-ports.\n"
224225
);

packages/containers-shared/tests/docker-context.test.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { execFileSync } from "child_process";
22
import { afterEach, describe, expect, it, vi } from "vitest";
3+
import { UserError } from "../src/error";
34
import { resolveDockerHost } from "../src/utils";
45

56
vi.mock("node:child_process");
@@ -41,7 +42,7 @@ describe.skipIf(process.platform !== "linux" && process.env.CI === "true")(
4142

4243
it("should fall back to platform default when context fails", () => {
4344
vi.mocked(execFileSync).mockImplementation(() => {
44-
throw new Error("Docker command failed");
45+
throw new UserError("Docker command failed");
4546
});
4647

4748
const result = resolveDockerHost("/no/op/docker");

0 commit comments

Comments
 (0)