Skip to content

[wrangler] make sure that the ready-on message is printed after the local runtime is ready #9910

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

Merged
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/fluffy-seals-arrive.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"wrangler": patch
---

make sure that the ready-on message is printed after the appropriate runtime controller is ready

fix the fact that when starting a local (or remote) dev session the log saying `Ready on http://localhost:xxxx` could be displayed before the runtime is actually ready to handle requests (this is quite noticeable when locally running dev sessions with containers, where the ready message currently gets displayed before the container images building/pulling process)
18 changes: 8 additions & 10 deletions fixtures/interactive-dev-tests/tests/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -500,11 +500,10 @@ baseDescribe.skipIf(process.platform !== "linux" && process.env.CI === "true")(
});

it("should allow quitting while the image is building", async () => {
const wrangler = await startWranglerDev([
"dev",
"-c",
path.join(tmpDir, "wrangler.jsonc"),
]);
const wrangler = await startWranglerDev(
["dev", "-c", path.join(tmpDir, "wrangler.jsonc")],
true
);

const waitForOptions = {
timeout: 10_000,
Expand All @@ -524,11 +523,10 @@ baseDescribe.skipIf(process.platform !== "linux" && process.env.CI === "true")(
});

it("should rebuilding while the image is building", async () => {
const wrangler = await startWranglerDev([
"dev",
"-c",
path.join(tmpDir, "wrangler.jsonc"),
]);
const wrangler = await startWranglerDev(
["dev", "-c", path.join(tmpDir, "wrangler.jsonc")],
true
);

const waitForOptions = {
timeout: 15_000,
Expand Down
20 changes: 19 additions & 1 deletion fixtures/pages-workerjs-app/tests/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { rename } from "node:fs/promises";
import path, { resolve } from "node:path";
import { setTimeout } from "node:timers/promises";
import { fetch } from "undici";
import { describe, it } from "vitest";
import { describe, it, vi } from "vitest";
import { runWranglerPagesDev } from "../../shared/src/run-wrangler-long-lived";

describe("Pages _worker.js", () => {
Expand Down Expand Up @@ -72,6 +72,15 @@ describe("Pages _worker.js", () => {
"--port=0",
"--inspector-port=0",
]);
vi.waitFor(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why are there changes to a pages fixture?

Copy link
Member Author

@dario-piotrowicz dario-piotrowicz Jul 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test below does the following:

  • clears the output (clearOutput())
  • renames a file
  • makes sure that the output is (still) empty (expect(getOutput()).toBe(""))

Before ready on was being printed basically at the earlier time (as soon as the proxy controller is initialized), before the output gets cleared, but now the log is being printed asynchronously when all runtimes are ready, which actually happens after the output gets cleared (making the toBe("") assertion fail).
That's why now we need to wait for the ready on message to be there first (so that we can be sure that that won't appear in the output)

If you'd prefer a different change here please let me know 🙂

() => {
expect(getOutput()).toContain("Ready on");
},
{
timeout: 5_000,
}
);
await setTimeout(200);
try {
clearOutput();
await tryRename(
Expand Down Expand Up @@ -109,6 +118,15 @@ describe("Pages _worker.js", () => {
"--port=0",
"--inspector-port=0",
]);
vi.waitFor(
() => {
expect(getOutput()).toContain("Ready on");
},
{
timeout: 5_000,
}
);
await setTimeout(200);
try {
clearOutput();
await tryRename(
Expand Down
33 changes: 26 additions & 7 deletions packages/wrangler/e2e/containers.dev.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -196,14 +196,15 @@ for (const source of imageSource) {
it(`will be able to interact with the container`, async () => {
const worker = helper.runLongLived("wrangler dev");
const ready = await worker.waitForReady();
await worker.readUntil(/Container image\(s\) ready/);

let response = await fetch(`${ready.url}/status`);
expect(response.status).toBe(200);
let status = await response.json();
expect(status).toBe(false);
await vi.waitFor(async () => {
const response = await fetch(`${ready.url}/status`);
expect(response.status).toBe(200);
const status = await response.json();
expect(status).toBe(false);
});

response = await fetch(`${ready.url}/start`);
let response = await fetch(`${ready.url}/start`);
let text = await response.text();
expect(response.status).toBe(200);
expect(text).toBe("Container create request sent...");
Expand All @@ -212,7 +213,7 @@ for (const source of imageSource) {
await new Promise((resolve) => setTimeout(resolve, 2_000));

response = await fetch(`${ready.url}/status`);
status = await response.json();
const status = await response.json();
expect(response.status).toBe(200);
expect(status).toBe(true);

Expand Down Expand Up @@ -254,6 +255,24 @@ for (const source of imageSource) {
);
});

it("will display the ready-on message after the container(s) have been built/pulled", async () => {
const worker = helper.runLongLived("wrangler dev");
const readyRegexp = /Ready on (http:\/\/[a-z0-9.]+:[0-9]+)/;
await worker.readUntil(readyRegexp);

await worker.stop();

const fullOutput = await worker.output;
const indexOfContainersReadyMessage = fullOutput.indexOf(
"Container image(s) ready"
);

const indexOfReadyOnMessage = fullOutput.indexOf("Ready on");
expect(indexOfReadyOnMessage).toBeGreaterThan(
indexOfContainersReadyMessage
);
});

it("won't start the container service if --enable-containers is set to false via CLI", async () => {
const worker = helper.runLongLived(
"wrangler dev --enable-containers=false"
Expand Down
2 changes: 1 addition & 1 deletion packages/wrangler/e2e/helpers/wrangler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export class WranglerLongLivedCommand extends LongLivedCommand {
super(getWranglerCommand(wranglerCommand), getOptions(options));
}

async waitForReady(readTimeout = 5_000): Promise<{ url: string }> {
async waitForReady(readTimeout = 15_000): Promise<{ url: string }> {
const match = await this.readUntil(
/Ready on (?<url>https?:\/\/.*)/,
readTimeout
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,18 +177,18 @@ describe.skipIf(!CLOUDFLARE_ACCOUNT_ID)(
Your Worker has access to the following bindings:
Binding Resource Mode
env.AI AI remote
[wrangler:info] Ready on http://<HOST>:<PORT>
▲ [WARNING] AI bindings always access remote resources, and so may incur usage charges even in local dev. To suppress this warning, set \`experimental_remote: true\` for the binding definition in your configuration file.
⎔ Starting local server...
[wrangler:info] Ready on http://<HOST>:<PORT>
[wrangler:info] GET / 200 OK (TIMINGS)`;

const output2 = dedent`
Your Worker has access to the following bindings:
Binding Resource Mode
env.AI AI remote
▲ [WARNING] AI bindings always access remote resources, and so may incur usage charges even in local dev. To suppress this warning, set \`experimental_remote: true\` for the binding definition in your configuration file.
[wrangler:info] Ready on http://<HOST>:<PORT>
⎔ Starting local server...
[wrangler:info] Ready on http://<HOST>:<PORT>
[wrangler:info] GET / 200 OK (TIMINGS)`;

const normalizedOutput = normalizeOutput(worker.currentOutput);
Expand Down
34 changes: 28 additions & 6 deletions packages/wrangler/src/api/startDevWorker/ProxyController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ import type {
} from "./events";
import type { StartDevWorkerOptions } from "./types";
import type { DeferredPromise } from "./utils";
import type { MiniflareOptions } from "miniflare";
import type { LogOptions, MiniflareOptions } from "miniflare";

type ProxyControllerEventMap = ControllerEventMap & {
ready: [ReadyEvent];
Expand All @@ -50,6 +50,8 @@ type ProxyControllerEventMap = ControllerEventMap & {
export class ProxyController extends Controller<ProxyControllerEventMap> {
public ready = createDeferred<ReadyEvent>();

public localServerReady = createDeferred<void>();

public proxyWorker?: Miniflare;
proxyWorkerOptions?: MiniflareOptions;
private inspectorProxyWorkerWebSocket?: DeferredPromise<WebSocket>;
Expand Down Expand Up @@ -126,11 +128,17 @@ export class ProxyController extends Controller<ProxyControllerEventMap> {
verbose: logger.loggerLevel === "debug",

// log requests into the ProxyWorker (for local + remote mode)
log: new ProxyControllerLogger(castLogLevel(logger.loggerLevel), {
prefix:
// if debugging, log requests with specic ProxyWorker prefix
logger.loggerLevel === "debug" ? "wrangler-ProxyWorker" : "wrangler",
}),
log: new ProxyControllerLogger(
castLogLevel(logger.loggerLevel),
{
prefix:
// if debugging, log requests with specic ProxyWorker prefix
logger.loggerLevel === "debug"
? "wrangler-ProxyWorker"
: "wrangler",
},
this.localServerReady.promise
),
handleRuntimeStdio,
liveReload: false,
};
Expand Down Expand Up @@ -419,6 +427,8 @@ export class ProxyController extends Controller<ProxyControllerEventMap> {
}
}
onReloadComplete(data: ReloadCompleteEvent) {
this.localServerReady.resolve();

this.latestConfig = data.config;
this.latestBundle = data.bundle;

Expand Down Expand Up @@ -606,6 +616,18 @@ export class ProxyController extends Controller<ProxyControllerEventMap> {
}

class ProxyControllerLogger extends WranglerLog {
constructor(
level: LogLevel,
opts: LogOptions,
private localServerReady: Promise<void>
) {
super(level, opts);
}

logReady(message: string): void {
this.localServerReady.then(() => super.logReady(message)).catch(() => {});
}

log(message: string) {
// filter out request logs being handled by the ProxyWorker
// the requests log remaining are handled by the UserWorker
Expand Down
Loading