Skip to content

Revert "[wrangler] fix: prevent remote binding sessions expiring during long dev sessions" #13464

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.

Sign up for GitHub

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
jamesopstad merged 1 commit into from
Apr 14, 2026
Merged

Revert "[wrangler] fix: prevent remote binding sessions expiring during long dev sessions" #13464

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 0 additions & 12 deletions .changeset/fix-remote-bindings-timeout.md
View file
Open in desktop

This file was deleted.

109 changes: 13 additions & 96 deletions packages/wrangler/src/__tests__/api/startDevWorker/RemoteRuntimeController.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { afterEach, beforeEach, describe, it, vi } from "vitest";
import { beforeEach, describe, it, vi } from "vitest";
import { RemoteRuntimeController } from "../../../api/startDevWorker/RemoteRuntimeController";
import { unwrapHook } from "../../../api/startDevWorker/utils";
// Import the mocked functions so we can set their behavior
import {
createPreviewSession,
Expand Down Expand Up @@ -37,6 +38,10 @@ vi.mock("../../../user/access", () => ({
domainUsesAccess: vi.fn(),
}));

vi.mock("../../../api/startDevWorker/utils", () => ({
unwrapHook: vi.fn(),
}));

function makeConfig(
overrides: Partial<StartDevWorkerOptions> = {}
): StartDevWorkerOptions {
Expand Down Expand Up @@ -98,6 +103,12 @@ describe("RemoteRuntimeController", () => {
}

beforeEach(() => {
// Setup mock implementations
vi.mocked(unwrapHook).mockResolvedValue({
accountId: "test-account-id",
apiToken: { apiToken: "test-token" },
});

vi.mocked(getWorkerAccountAndContext).mockResolvedValue({
workerAccount: {
accountId: "test-account-id",
Expand Down Expand Up @@ -148,106 +159,12 @@ describe("RemoteRuntimeController", () => {
vi.mocked(createWorkerPreview).mockResolvedValue({
value: "test-preview-token",
host: "test.workers.dev",
// No tailUrl — avoids real WebSocket connections in unit tests
tailUrl: "wss://test.workers.dev/tail",
});

vi.mocked(getAccessHeaders).mockResolvedValue({});
});

describe("proactive token refresh", () => {
afterEach(() => vi.useRealTimers());

it("should proactively refresh the token before expiry", async ({
expect,
}) => {
vi.useFakeTimers();

const { controller, bus } = setup();
const config = makeConfig();
const bundle = makeBundle();

controller.onBundleStart({ type: "bundleStart", config });
controller.onBundleComplete({ type: "bundleComplete", config, bundle });
await bus.waitFor("reloadComplete");

vi.mocked(createWorkerPreview).mockClear();
vi.mocked(createRemoteWorkerInit).mockClear();
vi.mocked(createWorkerPreview).mockResolvedValue({
value: "proactively-refreshed-token",
host: "test.workers.dev",
});

// Register the waiter before advancing so it's in place when the
// event fires. Use a timeout larger than the advance window so the
// waiter's own faked setTimeout doesn't race the refresh timer.
const reloadPromise = bus.waitFor(
"reloadComplete",
undefined,
60 * 60 * 1000
);
await vi.advanceTimersByTimeAsync(50 * 60 * 1000 + 1);
const reloadEvent = await reloadPromise;

expect(createWorkerPreview).toHaveBeenCalledTimes(1);
expect(reloadEvent).toMatchObject({
type: "reloadComplete",
proxyData: {
headers: {
"cf-workers-preview-token": "proactively-refreshed-token",
},
},
});
});

it("should cancel the proactive refresh timer on bundle start", async ({
expect,
}) => {
vi.useFakeTimers();

const { controller, bus } = setup();
const config = makeConfig();
const bundle = makeBundle();

controller.onBundleStart({ type: "bundleStart", config });
controller.onBundleComplete({ type: "bundleComplete", config, bundle });
await bus.waitFor("reloadComplete");

vi.mocked(createWorkerPreview).mockClear();

// A new bundleStart cancels the old timer before it fires
controller.onBundleStart({ type: "bundleStart", config });
controller.onBundleComplete({ type: "bundleComplete", config, bundle });
await bus.waitFor("reloadComplete");

vi.mocked(createWorkerPreview).mockClear();

// Advance to just before T2 would fire — no proactive refresh should occur
await vi.advanceTimersByTimeAsync(50 * 60 * 1000 - 1);
expect(createWorkerPreview).not.toHaveBeenCalled();
});

it("should cancel the proactive refresh timer on teardown", async ({
expect,
}) => {
vi.useFakeTimers();

const { controller, bus } = setup();
const config = makeConfig();
const bundle = makeBundle();

controller.onBundleStart({ type: "bundleStart", config });
controller.onBundleComplete({ type: "bundleComplete", config, bundle });
await bus.waitFor("reloadComplete");

vi.mocked(createWorkerPreview).mockClear();
await controller.teardown();

// Advance past where the timer would have fired
await vi.advanceTimersByTimeAsync(50 * 60 * 1000 + 1);
expect(createWorkerPreview).not.toHaveBeenCalled();
});
});

describe("preview token refresh", () => {
it("should handle missing state gracefully", async ({ expect }) => {
const { controller } = setup();
Expand Down
34 changes: 14 additions & 20 deletions packages/wrangler/src/__tests__/dev/remote-bindings.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
/* eslint-disable @typescript-eslint/consistent-type-imports */
import assert from "node:assert";
import { seed } from "@cloudflare/workers-utils/test-helpers";
import { fetch } from "undici";
/* eslint-disable no-restricted-imports */
Expand All @@ -14,7 +13,6 @@ import {
} from "vitest";
/* eslint-enable no-restricted-imports */
import { Binding, StartRemoteProxySessionOptions } from "../../api";
import { unwrapHook } from "../../api/startDevWorker/utils";
import { mockAccountId, mockApiToken } from "../helpers/mock-account-id";
import { mockConsoleMethods } from "../helpers/mock-console";
import {
Expand Down Expand Up @@ -716,18 +714,16 @@ describe("dev with remote bindings", { sequential: true, retry: 2 }, () => {
await vi.waitFor(() => expect(std.out).toMatch(/Ready/), {
timeout: 5_000,
});
expect(sessionOptions).toBeDefined();
assert(sessionOptions);
const { auth, ...rest1 } = sessionOptions;
expect(rest1).toEqual({
expect(sessionOptions).toEqual({
auth: {
accountId: "some-account-id",
apiToken: {
apiToken: "some-api-token",
},
},
complianceRegion: undefined,
workerName: "worker",
});
assert(auth);
expect(await unwrapHook(auth, { account_id: undefined })).toEqual({
accountId: "some-account-id",
apiToken: { apiToken: "some-api-token" },
});
await stopWrangler();
await wranglerStopped;
});
Expand Down Expand Up @@ -760,18 +756,16 @@ describe("dev with remote bindings", { sequential: true, retry: 2 }, () => {
timeout: 5_000,
});

expect(sessionOptions).toBeDefined();
assert(sessionOptions);
const { auth: auth2, ...rest2 } = sessionOptions;
expect(rest2).toEqual({
expect(sessionOptions).toEqual({
auth: {
accountId: "mock-account-id",
apiToken: {
apiToken: "some-api-token",
},
},
complianceRegion: undefined,
workerName: "worker",
});
assert(auth2);
expect(await unwrapHook(auth2, { account_id: undefined })).toEqual({
accountId: "mock-account-id",
apiToken: { apiToken: "some-api-token" },
});

await stopWrangler();

Expand Down
8 changes: 4 additions & 4 deletions packages/wrangler/src/api/remoteBindings/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,9 +82,9 @@ export async function maybeStartOrUpdateRemoteProxySession(
preExistingRemoteProxySessionData?: {
session: RemoteProxySession;
remoteBindings: Record<string, Binding>;
auth?: AsyncHook<CfAccount> | undefined;
auth?: CfAccount | undefined;
} | null,
auth?: AsyncHook<CfAccount> | undefined
auth?: CfAccount | undefined
): Promise<{
session: RemoteProxySession;
remoteBindings: Record<string, Binding>;
Expand Down Expand Up @@ -188,9 +188,9 @@ export async function maybeStartOrUpdateRemoteProxySession(
* @returns the auth hook to pass to the startRemoteProxy session function if any
*/
function getAuthHook(
auth: AsyncHook<CfAccount> | undefined,
auth: CfAccount | undefined,
config: Pick<Config, "account_id"> | undefined
): AsyncHook<CfAccount> | undefined {
): AsyncHook<CfAccount, [Pick<Config, "account_id">]> | undefined {
if (auth) {
return auth;
}
Expand Down
13 changes: 8 additions & 5 deletions packages/wrangler/src/api/startDevWorker/LocalRuntimeController.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import * as MF from "../../dev/miniflare";
import { logger } from "../../logger";
import { RuntimeController } from "./BaseController";
import { castErrorCause } from "./events";
import { getBinaryFileContents } from "./utils";
import { getBinaryFileContents, unwrapHook } from "./utils";
import type { RemoteProxySession } from "../remoteBindings";
import type {
BundleCompleteEvent,
Expand Down Expand Up @@ -209,6 +209,12 @@ export class LocalRuntimeController extends RuntimeController {

const remoteBindings = pickRemoteBindings(configBundle.bindings ?? {});

const auth =
Object.keys(remoteBindings).length === 0
? // If there are no remote bindings (this is a local only session) there's no need to get auth data
undefined
: await unwrapHook(data.config.dev.auth);

this.#remoteProxySessionData =
await maybeStartOrUpdateRemoteProxySession(
{
Expand All @@ -217,10 +223,7 @@ export class LocalRuntimeController extends RuntimeController {
bindings: remoteBindings,
},
this.#remoteProxySessionData ?? null,
Object.keys(remoteBindings).length === 0
? // If there are no remote bindings (this is a local only session) there's no need to get auth data
undefined
: data.config.dev.auth
auth
);
}

Expand Down
Loading
Loading