Skip to content

Commit

Permalink
Better messaging in remote mode (#5107)
Browse files Browse the repository at this point in the history
* Partial refactor: remove zoneID from `getZoneIdHostAndRoutes()`

`zoneId` is only fetched in remote mode. Whether or not Wrangler is in remote mode can be decided further down the call stack than this function, and so when Wrangler is switched between local mode and remote mode the `zoneId` value returned from this function is stale. The `host` and `routes` returned from this function are always the same regardless of remote mode/local mode

* Remove `zone` and `zoneId` from props & typing

* Get zone in `remote.tsx` and only switch host when running a zone preview

* More detailed error messages + tests

* Fix snapshots

* changesets

* Remove .only

* fix build

* Make e2e validation stricter

* Fix tests & push inferred host down in call stack

* setTimeout -> retry

* strip ansi
  • Loading branch information
penalosa authored Feb 29, 2024
1 parent 2ed7f32 commit 65d0399
Show file tree
Hide file tree
Showing 17 changed files with 385 additions and 190 deletions.
5 changes: 5 additions & 0 deletions .changeset/tame-needles-give.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"wrangler": patch
---

fix: Ensures that switching to remote mode during a dev session (from local mode) will correctly use the right zone. Previously, zone detection happened before the dev session was mounted, and so dev sessions started with local mode would have no zone inferred, and would have failed to start, with an ugly error.
5 changes: 5 additions & 0 deletions .changeset/tasty-jokes-provide.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"wrangler": patch
---

fix: Ensure that preview sessions created without a zone don't switch the host on which to start the preview from the one returned by the API.
120 changes: 120 additions & 0 deletions packages/wrangler/e2e/dev.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -733,3 +733,123 @@ describe("writes debug logs to hidden file", () => {
});
});
});

describe("zone selection", () => {
let worker: DevWorker;

beforeEach(async () => {
worker = await makeWorker();
await worker.seed((workerName) => ({
"wrangler.toml": dedent`
name = "${workerName}"
main = "src/index.ts"
compatibility_date = "2023-01-01"
compatibility_flags = ["nodejs_compat"]
`,
"src/index.ts": dedent`
export default {
fetch(request) {
return new Response(request.url)
}
}`,
"package.json": dedent`
{
"name": "${workerName}",
"version": "0.0.0",
"private": true
}
`,
}));
});

it("defaults to a workers.dev preview", async () => {
await worker.runDevSession("--remote --ip 127.0.0.1", async (session) => {
const { text } = await retry(
(s) => s.status !== 200,
async () => {
const r = await fetch(`http://127.0.0.1:${session.port}`);
return { text: await r.text(), status: r.status };
}
);
expect(text).toContain(`devprod-testing7928.workers.dev`);
});
});

it("respects dev.host setting", async () => {
await worker.seed((workerName) => ({
"wrangler.toml": dedent`
name = "${workerName}"
main = "src/index.ts"
compatibility_date = "2023-01-01"
compatibility_flags = ["nodejs_compat"]
[dev]
host = "wrangler-testing.testing.devprod.cloudflare.dev"
`,
}));
await worker.runDevSession("--remote --ip 127.0.0.1", async (session) => {
const { text } = await retry(
(s) => s.status !== 200,
async () => {
const r = await fetch(`http://127.0.0.1:${session.port}`);
return { text: await r.text(), status: r.status };
}
);
expect(text).toMatchInlineSnapshot(
`"https://wrangler-testing.testing.devprod.cloudflare.dev/"`
);
});
});
it("infers host from first route", async () => {
await worker.seed((workerName) => ({
"wrangler.toml": dedent`
name = "${workerName}"
main = "src/index.ts"
compatibility_date = "2023-01-01"
compatibility_flags = ["nodejs_compat"]
[[routes]]
pattern = "wrangler-testing.testing.devprod.cloudflare.dev/*"
zone_name = "testing.devprod.cloudflare.dev"
`,
}));
await worker.runDevSession("--remote --ip 127.0.0.1", async (session) => {
const { text } = await retry(
(s) => s.status !== 200,
async () => {
const r = await fetch(`http://127.0.0.1:${session.port}`);
return { text: await r.text(), status: r.status };
}
);
expect(text).toMatchInlineSnapshot(
`"https://wrangler-testing.testing.devprod.cloudflare.dev/"`
);
});
});
it("fails with useful error message if host is not routable", async () => {
await worker.seed((workerName) => ({
"wrangler.toml": dedent`
name = "${workerName}"
main = "src/index.ts"
compatibility_date = "2023-01-01"
compatibility_flags = ["nodejs_compat"]
[[routes]]
pattern = "not-a-domain.testing.devprod.cloudflare.dev/*"
zone_name = "testing.devprod.cloudflare.dev"
`,
}));
await worker.runDevSession("--remote --ip 127.0.0.1", async (session) => {
const { stderr } = await retry(
(s) => !s.stderr.includes("ERROR"),
() => {
return { stderr: session.stderr };
}
);
expect(normalizeOutput(stderr)).toMatchInlineSnapshot(`
"X [ERROR] Could not access \`not-a-domain.testing.devprod.cloudflare.dev\`. Make sure the domain is set up to be proxied by Cloudflare.
For more details, refer to https://developers.cloudflare.com/workers/configuration/routing/routes/#set-up-a-route"
`);
});
});
});
10 changes: 2 additions & 8 deletions packages/wrangler/e2e/helpers/account-id.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,2 @@
import assert from "node:assert";

assert(
process.env.CLOUDFLARE_ACCOUNT_ID,
"Please provide a CLOUDFLARE_ACCOUNT_ID as an environment variable"
);

export const CLOUDFLARE_ACCOUNT_ID = process.env.CLOUDFLARE_ACCOUNT_ID;
export const CLOUDFLARE_ACCOUNT_ID = process.env
.CLOUDFLARE_ACCOUNT_ID as string;
3 changes: 1 addition & 2 deletions packages/wrangler/e2e/helpers/wrangler-command.ts
Original file line number Diff line number Diff line change
@@ -1,2 +1 @@
export const WRANGLER =
process.env.WRANGLER ?? `pnpm --silent dlx wrangler@beta`;
export const WRANGLER = process.env.WRANGLER as string;
23 changes: 23 additions & 0 deletions packages/wrangler/e2e/validate-environment.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import assert from "node:assert";

assert(
process.env.WRANGLER,
'You must provide a way to run Wrangler (WRANGLER="pnpm --silent dlx wrangler@beta" will run the latest beta)'
);

assert(
process.env.CLOUDFLARE_ACCOUNT_ID,
"You must provide a CLOUDFLARE_ACCOUNT_ID as an environment variable"
);

assert(
process.env.CLOUDFLARE_API_TOKEN,
"You must provide a CLOUDFLARE_API_TOKEN as an environment variable"
);

assert(
process.env.CLOUDFLARE_ACCOUNT_ID === "8d783f274e1f82dc46744c297b015a2f",
"You must run Wrangler's e2e tests against DevProd Testing"
);

export const setup = () => {};
16 changes: 16 additions & 0 deletions packages/wrangler/e2e/vitest.config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import path from "path";
import { defineConfig } from "vitest/config";

export default defineConfig({
test: {
testTimeout: 240_000,
poolOptions: {
threads: {
singleThread: true,
},
},
retry: 2,
include: ["e2e/**/*.test.ts"],
globalSetup: path.resolve(__dirname, "./validate-environment.ts"),
},
});
2 changes: 1 addition & 1 deletion packages/wrangler/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@
"test": "pnpm run assert-git-version && jest",
"test:ci": "pnpm run test --coverage",
"test:debug": "pnpm run test --silent=false --verbose=true",
"test:e2e": "vitest --test-timeout 240000 --poolOptions.threads.singleThread --dir ./e2e --retry 2 run",
"test:e2e": "vitest -c ./e2e/vitest.config.ts",
"test:watch": "pnpm run test --testTimeout=50000 --watch",
"type:tests": "tsc -p ./src/__tests__/tsconfig.json && tsc -p ./e2e/tsconfig.json"
},
Expand Down
Loading

0 comments on commit 65d0399

Please sign in to comment.