Skip to content

Commit

Permalink
refactor: remove process.exit() from the pages code
Browse files Browse the repository at this point in the history
This enables simpler testing, as we do not have to spawn new child processes
to avoid the `process.exit()` from killing the jest process.
  • Loading branch information
petebacondarwin committed Apr 1, 2022
1 parent ea8e701 commit 2b796ec
Show file tree
Hide file tree
Showing 5 changed files with 73 additions and 65 deletions.
8 changes: 8 additions & 0 deletions .changeset/polite-lemons-shop.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
"wrangler": patch
---

refactor: remove `process.exit()` from the pages code

This enables simpler testing, as we do not have to spawn new child processes
to avoid the `process.exit()` from killing the jest process.
8 changes: 7 additions & 1 deletion .vscode/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
"pgrep",
"PKCE",
"Positionals",
"scandir",
"selfsigned",
"textfile",
"tsbuildinfo",
Expand All @@ -28,7 +29,12 @@
"websockets",
"xxhash"
],
"cSpell.ignoreWords": ["TESTTEXTBLOBNAME", "TESTWASMNAME", "yxxx"],
"cSpell.ignoreWords": [
"TESTTEXTBLOBNAME",
"TESTWASMNAME",
"extensionless",
"yxxx"
],
"eslint.runtime": "node",
"files.trimTrailingWhitespace": true
}
75 changes: 27 additions & 48 deletions packages/wrangler/src/__tests__/pages.test.ts
Original file line number Diff line number Diff line change
@@ -1,25 +1,9 @@
import { execaSync } from "execa";
import { getPackageManager } from "../package-manager";
import { mockConsoleMethods } from "./helpers/mock-console";
import { runInTempDir } from "./helpers/run-in-tmp";
import { runWrangler } from "./helpers/run-wrangler";
import type { PackageManager } from "../package-manager";

describe("subcommand implicit help ran on imcomplete command execution", () => {
let mockPackageManager: PackageManager;
describe("subcommand implicit help ran on incomplete command execution", () => {
runInTempDir();

beforeEach(() => {
mockPackageManager = {
cwd: process.cwd(),
// @ts-expect-error we're making a fake package manager here
type: "mockpm",
addDevDeps: jest.fn(),
install: jest.fn(),
};
(getPackageManager as jest.Mock).mockResolvedValue(mockPackageManager);
});

const std = mockConsoleMethods();
function endEventLoop() {
return new Promise((resolve) => setImmediate(resolve));
Expand All @@ -46,37 +30,32 @@ describe("subcommand implicit help ran on imcomplete command execution", () => {
🚧 'wrangler pages <command>' is a beta command. Please report any issues to https://github.com/cloudflare/wrangler2/issues/new/choose"
`);
});
});

describe("beta message for subcommands", () => {
const isWindows = process.platform === "win32";
it("should display for pages:dev", async () => {
try {
execaSync("npx", ["wrangler", "pages", "dev"], {
shell: isWindows,
env: { BROWSER: "none", ...process.env },
}).stderr;
} catch (e: unknown) {
expect(e).toMatchInlineSnapshot(`
[Error: Command failed with exit code 1: npx wrangler pages dev
🚧 'wrangler pages <command>' is a beta command. Please report any issues to https://github.com/cloudflare/wrangler2/issues/new/choose
Must specify a directory of static assets to serve or a command to run.]
`);
}
});

it("should display for pages:functions", async () => {
try {
execaSync("npx", ["wrangler", "pages", "functions"], {
shell: isWindows,
env: { BROWSER: "none", ...process.env },
}).stderr;
} catch (e: unknown) {
expect(e).toMatchInlineSnapshot(`
[Error: Command failed with exit code 1: npx wrangler pages functions
🚧 'wrangler pages <command>' is a beta command. Please report any issues to https://github.com/cloudflare/wrangler2/issues/new/choose
Must specify a directory of static assets to serve or a command to run.]
`);
}
describe("beta message for subcommands", () => {
it("should display for pages:dev", async () => {
await expect(
runWrangler("pages dev")
).rejects.toThrowErrorMatchingInlineSnapshot(
`"Must specify a directory of static assets to serve or a command to run."`
);

expect(std.out).toMatchInlineSnapshot(
`"🚧 'wrangler pages <command>' is a beta command. Please report any issues to https://github.com/cloudflare/wrangler2/issues/new/choose"`
);
});

// Note that `wrangler pages functions` does nothing...

it("should display for pages:functions:build", async () => {
await expect(
runWrangler("pages functions build")
).rejects.toThrowErrorMatchingInlineSnapshot(
`"ENOENT: no such file or directory, scandir 'functions'"`
);

expect(std.out).toMatchInlineSnapshot(
`"🚧 'wrangler pages <command>' is a beta command. Please report any issues to https://github.com/cloudflare/wrangler2/issues/new/choose"`
);
});
});
});
6 changes: 4 additions & 2 deletions packages/wrangler/src/cli.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import process from "process";
import { hideBin } from "yargs/helpers";
import { PagesFatalError } from "./pages";
import { reportError, initReporting } from "./reporting";
import { main } from ".";

Expand All @@ -11,9 +12,10 @@ process.on("uncaughtExceptionMonitor", async (err, origin) => {
await reportError(err, origin);
});

main(hideBin(process.argv)).catch(() => {
main(hideBin(process.argv)).catch((e) => {
// The logging of any error that was thrown from `main()` is handled in the `yargs.fail()` handler.
// Here we just want to ensure that the process exits with a non-zero code.
// We don't want to do this inside the `main()` function, since that would kill the process when running our tests.
process.exit(1);
const exitCode = (e instanceof PagesFatalError && e.code) || 1;
process.exit(exitCode);
});
41 changes: 27 additions & 14 deletions packages/wrangler/src/pages.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,17 +25,26 @@ import type { BuilderCallback } from "yargs";
export const pagesBetaWarning =
"🚧 'wrangler pages <command>' is a beta command. Please report any issues to https://github.com/cloudflare/wrangler2/issues/new/choose";

const EXIT_CALLBACKS: (() => void)[] = [];
const EXIT = (message?: string, code?: number) => {
if (message) console.log(message);
if (code) process.exitCode = code;
EXIT_CALLBACKS.forEach((callback) => callback());
export class PagesFatalError extends Error {
constructor(message?: string, readonly code?: number) {
super(message);
}
}

const CLEANUP_CALLBACKS: (() => void)[] = [];
const CLEANUP = () => {
CLEANUP_CALLBACKS.forEach((callback) => callback());
RUNNING_BUILDERS.forEach((builder) => builder.stop?.());
process.exit(code);
};

process.on("SIGINT", () => EXIT());
process.on("SIGTERM", () => EXIT());
process.on("SIGINT", () => {
CLEANUP();
process.exit();
});
process.on("SIGTERM", () => {
CLEANUP();
process.exit();
});

function isWindows() {
return process.platform === "win32";
Expand Down Expand Up @@ -108,11 +117,13 @@ async function spawnProxyProcess({
port?: number;
command: (string | number)[];
}): Promise<void | number> {
if (command.length === 0)
return EXIT(
if (command.length === 0) {
CLEANUP();
throw new PagesFatalError(
"Must specify a directory of static assets to serve or a command to run.",
1
);
}

console.log(`Running ${command.join(" ")}...`);
const proxy = spawn(
Expand All @@ -126,7 +137,7 @@ async function spawnProxyProcess({
},
}
);
EXIT_CALLBACKS.push(() => {
CLEANUP_CALLBACKS.push(() => {
proxy.kill();
});

Expand Down Expand Up @@ -157,7 +168,8 @@ async function spawnProxyProcess({
.filter((port) => port !== undefined)[0];

if (port === undefined) {
return EXIT(
CLEANUP();
throw new PagesFatalError(
"Could not automatically determine proxy port. Please specify the proxy port with --proxy.",
1
);
Expand Down Expand Up @@ -970,13 +982,14 @@ export const pages: BuilderCallback<unknown, unknown> = (yargs) => {
});
}

EXIT_CALLBACKS.push(() => {
CLEANUP_CALLBACKS.push(() => {
server.close();
miniflare.dispose().catch((err) => miniflare.log.error(err));
});
} catch (e) {
miniflare.log.error(e as Error);
EXIT("Could not start Miniflare.", 1);
CLEANUP();
throw new PagesFatalError("Could not start Miniflare.", 1);
}
}
)
Expand Down

0 comments on commit 2b796ec

Please sign in to comment.