Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: better errors for workers commands in pages projects #6996

Merged
merged 7 commits into from
Oct 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/nine-cougars-repeat.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"wrangler": patch
---

fix: improve error messaging when accidentally using Workers commands in Pages project

If we detect a Workers command used with a Pages project (i.e. wrangler.toml contains `pages_output_build_dir`), error with Pages version of command rather than "missing entry-point" etc.
11 changes: 11 additions & 0 deletions packages/wrangler/src/__tests__/delete.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,17 @@ describe("delete", () => {
`);
});

it("should error helpfully if pages_build_output_dir is set", async () => {
writeWranglerToml({ pages_build_output_dir: "dist", name: "test" });
await expect(
runWrangler("delete")
).rejects.toThrowErrorMatchingInlineSnapshot(
`
[Error: It looks like you've run a Workers-specific command in a Pages project.
For Pages, please run \`wrangler pages project delete\` instead.]
`
);
});
describe("force deletes", () => {
it("should prompt for extra confirmation when service is depended on and use force", async () => {
mockConfirm({
Expand Down
15 changes: 15 additions & 0 deletions packages/wrangler/src/__tests__/deploy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,21 @@ describe("deploy", () => {
`);
});

it("should error helpfully if pages_build_output_dir is set in wrangler.toml", async () => {
writeWranglerToml({
pages_build_output_dir: "public",
name: "test-name",
});
await expect(
runWrangler("deploy")
).rejects.toThrowErrorMatchingInlineSnapshot(
`
[Error: It looks like you've run a Workers-specific command in a Pages project.
For Pages, please run \`wrangler pages deploy\` instead.]
`
);
});

describe("output additional script information", () => {
it("for first party workers, it should print worker information at log level", async () => {
setIsTTY(false);
Expand Down
10 changes: 10 additions & 0 deletions packages/wrangler/src/__tests__/dev.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1886,6 +1886,16 @@ describe.sequential("wrangler dev", () => {
);
});
});

it("should error helpfully if pages_build_output_dir is set", async () => {
writeWranglerToml({ pages_build_output_dir: "dist", name: "test" });
await expect(runWrangler("dev")).rejects.toThrowErrorMatchingInlineSnapshot(
`
[Error: It looks like you've run a Workers-specific command in a Pages project.
For Pages, please run \`wrangler pages dev\` instead.]
`
);
});
});

function mockGetZones(domain: string, zones: { id: string }[] = []) {
Expand Down
76 changes: 76 additions & 0 deletions packages/wrangler/src/__tests__/secret.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,25 @@ describe("wrangler secret", () => {
);
}

it("should error helpfully if pages_build_output_dir is set", async () => {
fs.writeFileSync(
"wrangler.toml",
TOML.stringify({
pages_build_output_dir: "public",
name: "script-name",
}),
"utf-8"
);
await expect(
runWrangler("secret put secret-name")
).rejects.toThrowErrorMatchingInlineSnapshot(
`
[Error: It looks like you've run a Workers-specific command in a Pages project.
For Pages, please run \`wrangler pages secret put\` instead.]
`
);
});

describe("interactive", () => {
beforeEach(() => {
setIsTTY(true);
Expand Down Expand Up @@ -389,6 +408,25 @@ describe("wrangler secret", () => {
);
}

it("should error helpfully if pages_build_output_dir is set", async () => {
fs.writeFileSync(
"wrangler.toml",
TOML.stringify({
pages_build_output_dir: "public",
name: "script-name",
}),
"utf-8"
);
await expect(
runWrangler("secret delete secret-name")
).rejects.toThrowErrorMatchingInlineSnapshot(
`
[Error: It looks like you've run a Workers-specific command in a Pages project.
For Pages, please run \`wrangler pages secret delete\` instead.]
`
);
});

it("should delete a secret", async () => {
mockDeleteRequest({ scriptName: "script-name", secretName: "the-key" });
mockConfirm({
Expand Down Expand Up @@ -499,6 +537,25 @@ describe("wrangler secret", () => {
);
}

it("should error helpfully if pages_build_output_dir is set", async () => {
fs.writeFileSync(
"wrangler.toml",
TOML.stringify({
pages_build_output_dir: "public",
name: "script-name",
}),
"utf-8"
);
await expect(
runWrangler("secret list")
).rejects.toThrowErrorMatchingInlineSnapshot(
`
[Error: It looks like you've run a Workers-specific command in a Pages project.
For Pages, please run \`wrangler pages secret list\` instead.]
`
);
});

it("should list secrets", async () => {
mockListRequest({ scriptName: "script-name" });
await runWrangler("secret list --name script-name");
Expand Down Expand Up @@ -565,6 +622,25 @@ describe("wrangler secret", () => {
});

describe("bulk", () => {
it("should error helpfully if pages_build_output_dir is set", async () => {
fs.writeFileSync(
"wrangler.toml",
TOML.stringify({
pages_build_output_dir: "public",
name: "script-name",
}),
"utf-8"
);
await expect(
runWrangler("secret bulk")
).rejects.toThrowErrorMatchingInlineSnapshot(
`
[Error: It looks like you've run a Workers-specific command in a Pages project.
For Pages, please run \`wrangler pages secret bulk\` instead.]
`
);
});

it("should fail secret bulk w/ no pipe or JSON input", async () => {
vi.spyOn(readline, "createInterface").mockImplementation(
() => null as unknown as Interface
Expand Down
16 changes: 16 additions & 0 deletions packages/wrangler/src/__tests__/tail.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { MockWebSocket } from "./helpers/mock-web-socket";
import { createFetchResult, msw, mswSucessScriptHandlers } from "./helpers/msw";
import { runInTempDir } from "./helpers/run-in-tmp";
import { runWrangler } from "./helpers/run-wrangler";
import { writeWranglerToml } from "./helpers/write-wrangler-toml";
import type {
AlarmEvent,
EmailEvent,
Expand Down Expand Up @@ -843,6 +844,21 @@ describe("tail", () => {
await api.closeHelper();
});
});

it("should error helpfully if pages_build_output_dir is set in wrangler.toml", async () => {
writeWranglerToml({
pages_build_output_dir: "public",
name: "test-name",
});
await expect(
runWrangler("tail")
).rejects.toThrowErrorMatchingInlineSnapshot(
`
[Error: It looks like you've run a Workers-specific command in a Pages project.
For Pages, please run \`wrangler pages deployment tail\` instead.]
`
);
});
});

/* helpers */
Expand Down
6 changes: 6 additions & 0 deletions packages/wrangler/src/delete.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,12 @@ export async function deleteHandler(args: DeleteArgs) {
const configPath =
args.config || (args.script && findWranglerToml(path.dirname(args.script)));
const config = readConfig(configPath, args);
if (config.pages_build_output_dir) {
throw new UserError(
"It looks like you've run a Workers-specific command in a Pages project.\n" +
"For Pages, please run `wrangler pages project delete` instead."
);
}
await metrics.sendMetricsEvent(
"delete worker script",
{},
Expand Down
7 changes: 7 additions & 0 deletions packages/wrangler/src/deploy/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,13 @@ export async function deployHandler(args: DeployArgs) {
args.config || (args.script && findWranglerToml(path.dirname(args.script)));
const projectRoot = configPath && path.dirname(configPath);
const config = readConfig(configPath, args);
if (config.pages_build_output_dir) {
throw new UserError(
"It looks like you've run a Workers-specific command in a Pages project.\n" +
"For Pages, please run `wrangler pages deploy` instead."
);
}

const entry = await getEntry(args, config, "deploy");

if (args.public) {
Expand Down
6 changes: 6 additions & 0 deletions packages/wrangler/src/deployment-bundle/entry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,12 @@ export async function getEntry(
) {
paths = resolveEntryWithAssets();
} else {
if (config.pages_build_output_dir && command === "dev") {
throw new UserError(
"It looks like you've run a Workers-specific command in a Pages project.\n" +
"For Pages, please run `wrangler pages dev` instead."
);
}
throw new UserError(
`Missing entry-point: The entry-point should be specified via the command line (e.g. \`wrangler ${command} path/to/script\`) or the \`main\` config field.`
);
Expand Down
24 changes: 24 additions & 0 deletions packages/wrangler/src/secret/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,12 @@ export const secret = (secretYargs: CommonYargsArgv) => {
async (args) => {
await printWranglerBanner();
const config = readConfig(args.config, args);
if (config.pages_build_output_dir) {
throw new UserError(
"It looks like you've run a Workers-specific command in a Pages project.\n" +
"For Pages, please run `wrangler pages secret put` instead."
);
}

const scriptName = getLegacyScriptName(args, config);
if (!scriptName) {
Expand Down Expand Up @@ -243,6 +249,12 @@ export const secret = (secretYargs: CommonYargsArgv) => {
},
async (args) => {
const config = readConfig(args.config, args);
if (config.pages_build_output_dir) {
throw new UserError(
"It looks like you've run a Workers-specific command in a Pages project.\n" +
"For Pages, please run `wrangler pages secret delete` instead."
);
}

const scriptName = getLegacyScriptName(args, config);
if (!scriptName) {
Expand Down Expand Up @@ -299,6 +311,12 @@ export const secret = (secretYargs: CommonYargsArgv) => {
},
async (args) => {
const config = readConfig(args.config, args);
if (config.pages_build_output_dir) {
throw new UserError(
"It looks like you've run a Workers-specific command in a Pages project.\n" +
"For Pages, please run `wrangler pages secret list` instead."
);
}

const scriptName = getLegacyScriptName(args, config);
if (!scriptName) {
Expand Down Expand Up @@ -360,6 +378,12 @@ type SecretBulkArgs = StrictYargsOptionsToInterface<typeof secretBulkOptions>;
export const secretBulkHandler = async (secretBulkArgs: SecretBulkArgs) => {
await printWranglerBanner();
const config = readConfig(secretBulkArgs.config, secretBulkArgs);
if (config.pages_build_output_dir) {
throw new UserError(
"It looks like you've run a Workers-specific command in a Pages project.\n" +
"For Pages, please run `wrangler pages secret bulk` instead."
);
}

if (secretBulkArgs._.includes("secret:bulk")) {
logger.warn(
Expand Down
6 changes: 6 additions & 0 deletions packages/wrangler/src/tail/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,12 @@ export async function tailHandler(args: TailArgs) {
await printWranglerBanner();
}
const config = readConfig(args.config, args);
if (config.pages_build_output_dir) {
throw new UserError(
"It looks like you've run a Workers-specific command in a Pages project.\n" +
"For Pages, please run `wrangler pages deployment tail` instead."
);
}
await metrics.sendMetricsEvent("begin log stream", {
sendMetrics: config.send_metrics,
});
Expand Down
Loading