Skip to content

Commit

Permalink
fix(wrangler): fix build-env to exit with code rather than throw fa…
Browse files Browse the repository at this point in the history
…tal error

Currently `pages functions build-env` throws a fatal
error if a config file does not exit, or if it is
invalid. This causes issues for the CI system. We
should instead exit with a specific code, if any of
those situations arises.
  • Loading branch information
CarmenPopoviciu committed Apr 9, 2024
1 parent 4f47f74 commit a3371e2
Show file tree
Hide file tree
Showing 3 changed files with 83 additions and 29 deletions.
7 changes: 7 additions & 0 deletions .changeset/green-rats-serve.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"wrangler": minor
---

fix: fix `pages function build-env` to exit with code rather than throw fatal error

Currently pages functions build-env throws a fatal error if a config file does not exit, or if it is invalid. This causes issues for the CI system. We should instead exit with a specific code, if any of those situations arises.
65 changes: 51 additions & 14 deletions packages/wrangler/src/__tests__/pages/pages-build-env.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
/* eslint-disable turbo/no-undeclared-env-vars */
import { readFileSync, writeFileSync } from "node:fs";
import {
EXIT_CODE_INVALID_PAGES_CONFIG,
EXIT_CODE_NO_CONFIG_FOUND,
} from "../../pages/errors";
import { mockConsoleMethods } from "../helpers/mock-console";
import { runInTempDir } from "../helpers/run-in-tmp";
import { runWrangler } from "../helpers/run-wrangler";
Expand Down Expand Up @@ -33,26 +37,39 @@ describe("pages build env", () => {
);
});

it("should fail with no config file", async () => {
it("should exit with specific exit code if no config file is found", async () => {
const processExitMock = jest
.spyOn(process, "exit")
.mockImplementation(() => {
throw new Error("process exit mock");
});
await expect(
runWrangler("pages functions build-env . --outfile data.json")
).rejects.toThrowErrorMatchingInlineSnapshot(
`"No Pages config file found"`
);
runWrangler("pages functions build-env . --outfile out.json")
).rejects.toThrowErrorMatchingInlineSnapshot(`"process exit mock"`);
expect(processExitMock).toHaveBeenCalledWith(EXIT_CODE_NO_CONFIG_FOUND);
processExitMock.mockRestore();
});

it("should fail with no project dir", async () => {
await expect(
runWrangler("pages functions build-env")
).rejects.toThrowErrorMatchingInlineSnapshot(
`"No Pages project location specified"`
);
});

it("should fail with no outfile", async () => {
await expect(
runWrangler("pages functions build-env .")
).rejects.toThrowErrorMatchingInlineSnapshot(`"No outfile specified"`);
});
it("should fail correctly with a non-pages config file", async () => {

it("should exit with specific code if a non-pages config file is found", async () => {
const processExitMock = jest
.spyOn(process, "exit")
.mockImplementation(() => {
throw new Error("process exit mock");
});
writeWranglerToml({
vars: {
VAR1: "VALUE1",
Expand Down Expand Up @@ -81,21 +98,38 @@ describe("pages build env", () => {
// This error is specifically handled by the caller of build-env
await expect(
runWrangler("pages functions build-env . --outfile data.json")
).rejects.toThrowErrorMatchingInlineSnapshot(
`"Your wrangler.toml is not a valid Pages config file"`
).rejects.toThrowErrorMatchingInlineSnapshot(`"process exit mock"`);
expect(processExitMock).toHaveBeenCalledWith(
EXIT_CODE_INVALID_PAGES_CONFIG
);
processExitMock.mockRestore();
});
it("should fail correctly with an unparseable config file", async () => {

it("should exit correctly with an unparseable config file", async () => {
const processExitMock = jest
.spyOn(process, "exit")
.mockImplementation(() => {
throw new Error("process exit mock");
});
writeFileSync("./wrangler.toml", 'INVALID "FILE');
// This error is specifically handled by the caller of build-env
await expect(
runWrangler("pages functions build-env . --outfile data.json")
).rejects.toThrowErrorMatchingInlineSnapshot(
`"Your wrangler.toml is not a valid Pages config file"`
).rejects.toThrowErrorMatchingInlineSnapshot(`"process exit mock"`);
expect(processExitMock).toHaveBeenCalledWith(
EXIT_CODE_INVALID_PAGES_CONFIG
);
expect(std.err).toContain("ParseError");
processExitMock.mockRestore();
});
it("should fail correctly with a non-pages config file w/ invalid environment", async () => {

it("should exit correctly with a non-pages config file w/ invalid environment", async () => {
const processExitMock = jest
.spyOn(process, "exit")
.mockImplementation(() => {
throw new Error("process exit mock");
});

writeWranglerToml({
vars: {
VAR1: "VALUE1",
Expand Down Expand Up @@ -124,9 +158,11 @@ describe("pages build env", () => {
// This error is specifically handled by the caller of build-env
await expect(
runWrangler("pages functions build-env . --outfile data.json")
).rejects.toThrowErrorMatchingInlineSnapshot(
`"Your wrangler.toml is not a valid Pages config file"`
).rejects.toThrowErrorMatchingInlineSnapshot(`"process exit mock"`);
expect(processExitMock).toHaveBeenCalledWith(
EXIT_CODE_INVALID_PAGES_CONFIG
);
processExitMock.mockRestore();
});

it("should return top-level by default", async () => {
Expand Down Expand Up @@ -169,6 +205,7 @@ describe("pages build env", () => {
`"{\\"vars\\":{\\"VAR1\\":\\"VALUE1\\",\\"VAR2\\":\\"VALUE2\\"},\\"pages_build_output_dir\\":\\"dist\\"}"`
);
});

it("should return production", async () => {
process.env.PAGES_ENVIRONMENT = "production";
writeWranglerToml({
Expand Down
40 changes: 25 additions & 15 deletions packages/wrangler/src/pages/build-env.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@ import path from "node:path";
import { readConfig } from "../config";
import { FatalError } from "../errors";
import { logger } from "../logger";
import { EXIT_CODE_NO_CONFIG_FOUND } from "./errors";
import {
EXIT_CODE_INVALID_PAGES_CONFIG,
EXIT_CODE_NO_CONFIG_FOUND,
} from "./errors";
import type { Config } from "../config";
import type {
CommonYargsArgv,
StrictYargsOptionsToInterface,
Expand Down Expand Up @@ -34,24 +38,30 @@ export const Handler = async (args: PagesBuildEnvArgs) => {
}

const configPath = path.resolve(args.projectDir, "wrangler.toml");
// Fail early if the config file doesn't exist
if (!existsSync(configPath)) {
throw new FatalError(
"No Pages config file found",
EXIT_CODE_NO_CONFIG_FOUND
);
logger.log("No Pages config file found. Skipping...");
process.exit(EXIT_CODE_NO_CONFIG_FOUND);
}

logger.log("Reading build configuration from your wrangler.toml file...");

const config = readConfig(
configPath,
{
...args,
// eslint-disable-next-line turbo/no-undeclared-env-vars
env: process.env.PAGES_ENVIRONMENT,
},
true
);
let config: Omit<Config, "pages_build_output_dir"> & {
pages_build_output_dir: string;
};
try {
config = readConfig(
configPath,
{
...args,
// eslint-disable-next-line turbo/no-undeclared-env-vars
env: process.env.PAGES_ENVIRONMENT,
},
true
);
} catch (err) {
logger.log("Invalid Pages config file found. Skipping...");
process.exit(EXIT_CODE_INVALID_PAGES_CONFIG);
}

// Ensure JSON variables are not included
const textVars = Object.fromEntries(
Expand Down

0 comments on commit a3371e2

Please sign in to comment.