[wrangler] test: fix E2E tests (#4907)

* ci: ensure E2E tests failures not reported as successes

#4458 accidentally left `continue-on-error: true` in the workflow.
This meant that E2E test failures weren't reported as check failures.

* test: remove redundant `deploy.test.ts` E2E test

This test was a strict subset of `deployments.test.ts`, and wasn't
providing any additional value.

* test: remove standard pricing warning from E2E test output

This warning may not be shown if an account has opted-in to standard
pricing. This change normalises output to remove it, meaning tests
can be run on any account, regardless of opt-in state.

* test: update format of writing logs message in E2E tests

* test: use `fallback` instead of `default` in E2E tests

#4571 updated the message logged in non-interactive contexts for
default values to use the word `fallback` instead of `default`.

* fix: ensure `--log-level` argument applied immediately

Some messages were being logged before the `--log-level` argument was
applied. In particular, this meant the "writing logs to" message at
`debug` level, was not output when using `--log-level=debug`.

* test: improve reliability of `dev.test.ts` E2E test

This change makes a few improvements to `dev.test.ts`:

- Shorter timeouts for `fetch()` retries
- Replaces `get-port` with native port `0` implementation
- Waits for stdout handlers to be registered before starting
  `wrangler dev`
- Ignores errors if we failed to find a proccess to kill
  (assumes the process was already killed)

* fix: throw `UserError`s for R2 object/bucket not-found errors

These errors should not be reported to Sentry.

* global install

* Ensure internal workers listen on ipv4 only

* shorter timeouts


---------

Co-authored-by: Samuel Macleod <smacleod@cloudflare.com>

.changeset/mighty-coins-cover.md

@@ -0,0 +1,7 @@
+---
+"wrangler": patch
+---
+
+fix: mark R2 object and bucket not found errors as unreportable
+
+Previously, running `wrangler r2 objects {get,put}` with an object or bucket that didn't exist would ask if you wanted to report that error to Cloudflare. There's nothing we can do to fix this, so this change prevents the prompt in this case.

.changeset/spotty-papayas-sneeze.md

@@ -0,0 +1,7 @@
+---
+"wrangler": patch
+---
+
+fix: ensure `wrangler dev --log-level` flag applied to all logs
+
+Previously, `wrangler dev` may have ignored the `--log-level` flag for some startup logs. This change ensures the `--log-level` flag is applied immediately.

.github/workflows/e2e.yml

@@ -13,7 +13,7 @@ jobs:
     concurrency:
       group: ${{ github.workflow }}-${{ github.ref }}-${{ matrix.os }}-${{ matrix.node }}
       cancel-in-progress: true
-    timeout-minutes: 30
+    timeout-minutes: 15
     if: github.repository_owner == 'cloudflare' && (github.event_name != 'pull_request' || (github.event_name == 'pull_request' && contains(github.event.*.labels.*.name, 'e2e' )))
     name: "E2E Test"
     strategy:
@@ -73,9 +73,20 @@ jobs:
         id: "find-wrangler"
         run: echo "dir=$(ls $HOME/wrangler-*.tgz)" >> $GITHUB_OUTPUT;
 
-      - name: Run tests
-        id: e2e-1
-        continue-on-error: true
+      - name: Run tests (unix)
+        if: matrix.os == 'macos-13' || matrix.os == 'ubuntu-22.04'
+        run: |
+          pnpm add ${{ steps.find-wrangler.outputs.dir}} --global
+          pnpm run --filter wrangler test:e2e
+        env:
+          CLOUDFLARE_API_TOKEN: ${{ secrets.TEST_CLOUDFLARE_API_TOKEN }}
+          CLOUDFLARE_ACCOUNT_ID: ${{ secrets.TEST_CLOUDFLARE_ACCOUNT_ID }}
+          WRANGLER: wrangler
+          NODE_OPTIONS: "--max_old_space_size=8192"
+          WRANGLER_LOG_PATH: ${{ runner.temp }}/wrangler-debug-logs/
+
+      - name: Run tests (windows)
+        if: matrix.os == 'windows-2022'
         run: pnpm run --filter wrangler test:e2e
         env:
           CLOUDFLARE_API_TOKEN: ${{ secrets.TEST_CLOUDFLARE_API_TOKEN }}

packages/miniflare/src/index.ts

@@ -1541,6 +1541,7 @@ export class Miniflare {
 		// Get a `Fetcher` to that worker (NOTE: the `ProxyServer` Durable Object
 		// shares its `env` with Miniflare's entry worker, so has access to routes)
 		const bindingName = CoreBindings.SERVICE_USER_ROUTE_PREFIX + workerName;
+
 		const fetcher = proxyClient.env[bindingName];
 		if (fetcher === undefined) {
 			// `#findAndAssertWorkerIndex()` will throw if a "worker" doesn't exist

packages/miniflare/src/plugins/core/proxy/client.ts

@@ -332,6 +332,7 @@ class ProxyStubHandler<T extends object> implements ProxyHandler<T> {
 			{ value: stringifiedResult, unbufferedStream },
 			this.revivers
 		);
+
 		// We get an empty stack trace if we thread the caller through here,
 		// specifying `this.#parseAsyncResponse` is good enough though, we just
 		// get an extra `processTicksAndRejections` entry

packages/miniflare/src/workers/core/devalue.ts

@@ -309,5 +309,6 @@ export function parseWithReadableStreams<RS>(
 		},
 		...revivers,
 	};
+
 	return parse(stringified.value, streamRevivers);
 }

packages/wrangler/e2e/c3-integration.test.ts

@@ -63,8 +63,7 @@ describe("c3 integration", () => {
 	it("deploy the worker", async () => {
 		const { stdout, stderr } = await runInWorker`$ ${WRANGLER} deploy`;
 		expect(normalize(stdout)).toMatchInlineSnapshot(`
-			"🚧 New Workers Standard pricing is now available. Please visit the dashboard to view details and opt-in to new pricing: https://dash.cloudflare.com/CLOUDFLARE_ACCOUNT_ID/workers/standard/opt-in.
-			Total Upload: xx KiB / gzip: xx KiB
+			"Total Upload: xx KiB / gzip: xx KiB
 			Uploaded smoke-test-worker (TIMINGS)
 			Published smoke-test-worker (TIMINGS)
 			  https://smoke-test-worker.SUBDOMAIN.workers.dev
@@ -86,7 +85,7 @@ describe("c3 integration", () => {
 		const { stdout, stderr } = await runInWorker`$$ ${WRANGLER} delete`;
 		expect(normalize(stdout)).toMatchInlineSnapshot(`
 			"? Are you sure you want to delete smoke-test-worker? This action cannot be undone.
-			🤖 Using default value in non-interactive context: yes
+			🤖 Using fallback value in non-interactive context: yes
 			Successfully deleted smoke-test-worker"
 		`);
 		expect(stderr).toMatchInlineSnapshot('""');

packages/wrangler/e2e/deployments.test.ts

@@ -56,8 +56,7 @@ describe("deployments", () => {
 	it("deploy worker", async () => {
 		const { stdout } = await runInWorker`$ ${WRANGLER} deploy`;
 		expect(normalize(stdout)).toMatchInlineSnapshot(`
-			"🚧 New Workers Standard pricing is now available. Please visit the dashboard to view details and opt-in to new pricing: https://dash.cloudflare.com/CLOUDFLARE_ACCOUNT_ID/workers/standard/opt-in.
-			Total Upload: xx KiB / gzip: xx KiB
+			"Total Upload: xx KiB / gzip: xx KiB
 			Uploaded smoke-test-worker (TIMINGS)
 			Published smoke-test-worker (TIMINGS)
 			  https://smoke-test-worker.SUBDOMAIN.workers.dev
@@ -101,8 +100,7 @@ describe("deployments", () => {
 		});
 		const { stdout, stderr } = await runInWorker`$ ${WRANGLER} deploy`;
 		expect(normalize(stdout)).toMatchInlineSnapshot(`
-			"🚧 New Workers Standard pricing is now available. Please visit the dashboard to view details and opt-in to new pricing: https://dash.cloudflare.com/CLOUDFLARE_ACCOUNT_ID/workers/standard/opt-in.
-			Total Upload: xx KiB / gzip: xx KiB
+			"Total Upload: xx KiB / gzip: xx KiB
 			Uploaded smoke-test-worker (TIMINGS)
 			Published smoke-test-worker (TIMINGS)
 			  https://smoke-test-worker.SUBDOMAIN.workers.dev
@@ -178,7 +176,7 @@ describe("deployments", () => {
 		const { stdout, stderr } = await runInWorker`$ ${WRANGLER} delete`;
 		expect(normalize(stdout)).toMatchInlineSnapshot(`
 			"? Are you sure you want to delete smoke-test-worker? This action cannot be undone.
-			🤖 Using default value in non-interactive context: yes
+			🤖 Using fallback value in non-interactive context: yes
 			Successfully deleted smoke-test-worker"
 		`);
 		expect(stderr).toMatchInlineSnapshot('""');

packages/wrangler/e2e/dev.test.ts

@@ -4,15 +4,23 @@ import { existsSync } from "node:fs";
 import * as nodeNet from "node:net";
 import path from "node:path";
 import { setTimeout } from "node:timers/promises";
-import getPort from "get-port";
 import shellac from "shellac";
-import { fetch } from "undici";
+import { Agent, fetch, setGlobalDispatcher } from "undici";
 import { afterEach, beforeEach, describe, expect, it } from "vitest";
 import { normalizeOutput } from "./helpers/normalize";
 import { retry } from "./helpers/retry";
 import { dedent, makeRoot, seed } from "./helpers/setup";
 import { WRANGLER } from "./helpers/wrangler-command";
 
+// Use `Agent` with lower timeouts so `fetch()`s inside `retry()`s don't block for a long time
+setGlobalDispatcher(
+	new Agent({
+		connectTimeout: 10_000,
+		headersTimeout: 10_000,
+		bodyTimeout: 10_000,
+	})
+);
+
 type MaybePromise<T = void> = T | Promise<T>;
 
 const waitForPortToBeBound = async (port: number) => {
@@ -31,11 +39,7 @@ const waitUntilOutputContains = async (
 		(stdout) => !stdout.includes(substring),
 		async () => {
 			await setTimeout(intervalMs);
-			return (
-				normalizeOutput(session.stdout) +
-				"\n\n\n" +
-				normalizeOutput(session.stderr)
-			);
+			return session.stdout + "\n\n\n" + session.stderr;
 		}
 	);
 };
@@ -46,6 +50,20 @@ interface SessionData {
 	stderr: string;
 }
 
+function getPort() {
+	return new Promise<number>((resolve, reject) => {
+		const server = nodeNet.createServer((socket) => socket.destroy());
+		server.listen(0, () => {
+			const address = server.address();
+			assert(typeof address === "object" && address !== null);
+			server.close((err) => {
+				if (err) reject(err);
+				else resolve(address.port);
+			});
+		});
+	});
+}
+
 async function runDevSession(
 	workerPath: string,
 	flags: string,
@@ -65,7 +83,11 @@ async function runDevSession(
 
 		// Must use the `in` statement in the shellac script rather than `.in()` modifier on the `shellac` object
 		// otherwise the working directory does not get picked up.
+		let promiseResolve: (() => void) | undefined;
+		const promise = new Promise<void>((resolve) => (promiseResolve = resolve));
 		const bg = await shellac.env(process.env).bg`
+		await ${() => promise}
+
 		in ${workerPath} {
 			exits {
         $ ${WRANGLER} dev ${flags}
@@ -82,12 +104,18 @@ async function runDevSession(
 		};
 		bg.process.stdout.on("data", (chunk) => (sessionData.stdout += chunk));
 		bg.process.stderr.on("data", (chunk) => (sessionData.stderr += chunk));
+		// Only start `wrangler dev` once we've registered output listeners so we don't miss messages
+		promiseResolve?.();
 
 		await session(sessionData);
 
 		return bg.promise;
 	} finally {
-		if (pid) process.kill(pid);
+		try {
+			if (pid) process.kill(pid);
+		} catch {
+			// Ignore errors if we failed to kill the process (i.e. ESRCH if it's already terminated)
+		}
 	}
 }
 
@@ -489,14 +517,14 @@ describe("writes debug logs to hidden file", () => {
 			async (session) => {
 				await waitForPortToBeBound(session.port);
 
-				await waitUntilOutputContains(session, "🐛 Writing debug logs to");
+				await waitUntilOutputContains(session, "Writing logs to");
 
 				await setTimeout(1000); // wait a bit to ensure the file is written to disk
 			}
 		);
 
 		const filepath = finalA.stdout.match(
-			/🐛 Writing debug logs to "(.+\.log)"/
+			/🪵 {2}Writing logs to "(.+\.log)"/
 		)?.[1];
 		assert(filepath);
 
@@ -511,13 +539,13 @@ describe("writes debug logs to hidden file", () => {
 		});
 
 		const filepath = finalA.stdout.match(
-			/🐛 Writing debug logs to "(.+\.log)"/
+			/🪵 {2}Writing logs to "(.+\.log)"/
 		)?.[1];
 
 		expect(filepath).toBeUndefined();
 	});
 
-	it("rewrites address-in-use error logs", async () => {
+	it.skip("rewrites address-in-use error logs", async () => {
 		// 1. start worker A on a (any) port
 		await a.runDevSession("", async (sessionA) => {
 			const normalize = (text: string) =>