diff --git a/.changeset/fix-remote-bindings-timeout.md b/.changeset/fix-remote-bindings-timeout.md deleted file mode 100644 index ae83a4a1ac..0000000000 --- a/.changeset/fix-remote-bindings-timeout.md +++ /dev/null @@ -1,12 +0,0 @@ ---- -"wrangler": patch -"miniflare": patch ---- - -fix: prevent remote binding sessions from expiring during long-running dev sessions - -Preview tokens for remote bindings expire after one hour. Previously, the first request after expiry would fail before a refresh was triggered. This change proactively refreshes the token at 50 minutes so no request ever sees an expired session. - -The reactive recovery path is also improved: `error code: 1031` responses (returned by bindings such as Workers AI when their session times out) now correctly trigger a refresh, where previously only `Invalid Workers Preview configuration` HTML responses did. - -Auth credentials are now resolved lazily when a remote proxy session starts rather than at bundle-complete time. This means that if your OAuth access token has been refreshed since `wrangler dev` started, the new token is used rather than the one captured at startup. diff --git a/packages/wrangler/src/__tests__/api/startDevWorker/RemoteRuntimeController.test.ts b/packages/wrangler/src/__tests__/api/startDevWorker/RemoteRuntimeController.test.ts index 3167ee2dec..60f807ca6f 100644 --- a/packages/wrangler/src/__tests__/api/startDevWorker/RemoteRuntimeController.test.ts +++ b/packages/wrangler/src/__tests__/api/startDevWorker/RemoteRuntimeController.test.ts @@ -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, @@ -37,6 +38,10 @@ vi.mock("../../../user/access", () => ({ domainUsesAccess: vi.fn(), })); +vi.mock("../../../api/startDevWorker/utils", () => ({ + unwrapHook: vi.fn(), +})); + function makeConfig( overrides: Partial = {} ): StartDevWorkerOptions { @@ -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", @@ -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(); diff --git a/packages/wrangler/src/__tests__/dev/remote-bindings.test.ts b/packages/wrangler/src/__tests__/dev/remote-bindings.test.ts index 77e9e7b4cb..d324ba600d 100644 --- a/packages/wrangler/src/__tests__/dev/remote-bindings.test.ts +++ b/packages/wrangler/src/__tests__/dev/remote-bindings.test.ts @@ -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 */ @@ -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 { @@ -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; }); @@ -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(); diff --git a/packages/wrangler/src/api/remoteBindings/index.ts b/packages/wrangler/src/api/remoteBindings/index.ts index 318c52bb19..505b171daf 100644 --- a/packages/wrangler/src/api/remoteBindings/index.ts +++ b/packages/wrangler/src/api/remoteBindings/index.ts @@ -82,9 +82,9 @@ export async function maybeStartOrUpdateRemoteProxySession( preExistingRemoteProxySessionData?: { session: RemoteProxySession; remoteBindings: Record; - auth?: AsyncHook | undefined; + auth?: CfAccount | undefined; } | null, - auth?: AsyncHook | undefined + auth?: CfAccount | undefined ): Promise<{ session: RemoteProxySession; remoteBindings: Record; @@ -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 | undefined, + auth: CfAccount | undefined, config: Pick | undefined -): AsyncHook | undefined { +): AsyncHook]> | undefined { if (auth) { return auth; } diff --git a/packages/wrangler/src/api/startDevWorker/LocalRuntimeController.ts b/packages/wrangler/src/api/startDevWorker/LocalRuntimeController.ts index f0982b9f14..3d261527cd 100644 --- a/packages/wrangler/src/api/startDevWorker/LocalRuntimeController.ts +++ b/packages/wrangler/src/api/startDevWorker/LocalRuntimeController.ts @@ -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, @@ -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( { @@ -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 ); } diff --git a/packages/wrangler/src/api/startDevWorker/RemoteRuntimeController.ts b/packages/wrangler/src/api/startDevWorker/RemoteRuntimeController.ts index 32afcf750d..039bc5bdca 100644 --- a/packages/wrangler/src/api/startDevWorker/RemoteRuntimeController.ts +++ b/packages/wrangler/src/api/startDevWorker/RemoteRuntimeController.ts @@ -1,4 +1,3 @@ -import assert from "node:assert"; import { MissingConfigError } from "@cloudflare/workers-utils"; import chalk from "chalk"; import { Mutex } from "miniflare"; @@ -21,7 +20,7 @@ import { getAccessHeaders } from "../../user/access"; import { retryOnAPIFailure } from "../../utils/retry"; import { RuntimeController } from "./BaseController"; import { castErrorCause } from "./events"; -import { PREVIEW_TOKEN_REFRESH_INTERVAL, unwrapHook } from "./utils"; +import { unwrapHook } from "./utils"; import type { CfAccount, CfPreviewSession, @@ -31,7 +30,6 @@ import type { BundleCompleteEvent, BundleStartEvent, PreviewTokenExpiredEvent, - ProxyData, ReloadCompleteEvent, ReloadStartEvent, } from "./events"; @@ -53,10 +51,6 @@ export class RemoteRuntimeController extends RuntimeController { #latestConfig?: StartDevWorkerOptions; #latestBundle?: Bundle; #latestRoutes?: Route[]; - #latestProxyData?: ProxyData; - - // Timer for proactive token refresh before the 1-hour expiry - #refreshTimer?: ReturnType; async #previewSession( props: Parameters[0] & { @@ -87,7 +81,13 @@ export class RemoteRuntimeController extends RuntimeController { handlePreviewSessionCreationError(err, props.accountId); - throw err; + this.emitErrorEvent({ + type: "error", + reason: "Failed to create a preview token", + cause: castErrorCause(err), + source: "RemoteRuntimeController", + data: undefined, + }); } } @@ -267,11 +267,11 @@ export class RemoteRuntimeController extends RuntimeController { auth: CfAccount, routes: Route[] | undefined, bundleId: number - ): Promise { + ) { // If we received a new `bundleComplete` event before we were able to // dispatch a `reloadComplete` for this bundle, ignore this bundle. if (bundleId !== this.#currentBundleId) { - return false; + return; } const token = await this.#previewToken({ @@ -307,49 +307,30 @@ export class RemoteRuntimeController extends RuntimeController { // dispatch a `reloadComplete` for this bundle, ignore this bundle. // If `token` is undefined, we've surfaced a relevant error to the user above, so ignore this bundle if (bundleId !== this.#currentBundleId || !token) { - return false; + return; } const accessHeaders = await getAccessHeaders(token.host); - const proxyData: ProxyData = { - userWorkerUrl: { - protocol: "https:", - hostname: token.host, - port: "443", - }, - headers: { - "cf-workers-preview-token": token.value, - ...accessHeaders, - "cf-connecting-ip": "", - }, - liveReload: config.dev.liveReload, - proxyLogsToController: true, - }; - - this.#latestProxyData = proxyData; - this.emitReloadCompleteEvent({ type: "reloadComplete", bundle, config, - proxyData, + proxyData: { + userWorkerUrl: { + protocol: "https:", + hostname: token.host, + port: "443", + }, + headers: { + "cf-workers-preview-token": token.value, + ...accessHeaders, + "cf-connecting-ip": "", + }, + liveReload: config.dev.liveReload, + proxyLogsToController: true, + }, }); - - this.#scheduleRefresh(PREVIEW_TOKEN_REFRESH_INTERVAL); - return true; - } - - #scheduleRefresh(interval: number) { - clearTimeout(this.#refreshTimer); - this.#refreshTimer = setTimeout(() => { - if (this.#latestProxyData) { - this.onPreviewTokenExpired({ - type: "previewTokenExpired", - proxyData: this.#latestProxyData, - }); - } - }, interval); } async #onBundleComplete({ config, bundle }: BundleCompleteEvent, id: number) { @@ -362,9 +343,9 @@ export class RemoteRuntimeController extends RuntimeController { throw new MissingConfigError("config.dev.auth"); } - assert(config.dev.auth); const auth = await unwrapHook(config.dev.auth); + // Store for token refresh this.#latestConfig = config; this.#latestBundle = bundle; this.#latestRoutes = routes; @@ -404,27 +385,34 @@ export class RemoteRuntimeController extends RuntimeController { return; } - try { - assert(this.#latestConfig.dev.auth); - const auth = await unwrapHook(this.#latestConfig.dev.auth); + this.emitReloadStartEvent({ + type: "reloadStart", + config: this.#latestConfig, + bundle: this.#latestBundle, + }); + + if (!this.#latestConfig.dev?.auth) { + // This shouldn't happen as it's checked earlier, but we guard against it anyway + throw new MissingConfigError("config.dev.auth"); + } + + const auth = await unwrapHook(this.#latestConfig.dev.auth); + try { this.#session = await this.#getPreviewSession( this.#latestConfig, auth, this.#latestRoutes ); - const refreshed = await this.#updatePreviewToken( + await this.#updatePreviewToken( this.#latestConfig, this.#latestBundle, auth, this.#latestRoutes, this.#currentBundleId ); - - if (refreshed) { - logger.log(chalk.green("✔ Preview token refreshed successfully")); - } + logger.log(chalk.green("✔ Preview token refreshed successfully")); } catch (error) { if (error instanceof Error && error.name == "AbortError") { return; @@ -448,7 +436,6 @@ export class RemoteRuntimeController extends RuntimeController { // Abort any previous operations when a new bundle is started this.#abortController.abort(); this.#abortController = new AbortController(); - clearTimeout(this.#refreshTimer); } onBundleComplete(ev: BundleCompleteEvent) { const id = ++this.#currentBundleId; @@ -467,7 +454,7 @@ export class RemoteRuntimeController extends RuntimeController { void this.#mutex.runWith(() => this.#onBundleComplete(ev, id)); } onPreviewTokenExpired(_: PreviewTokenExpiredEvent): void { - logger.log(chalk.dim("⎔ Refreshing preview token...")); + logger.log(chalk.dim("⎔ Preview token expired, refreshing...")); void this.#mutex.runWith(() => this.#refreshPreviewToken()); } @@ -478,7 +465,6 @@ export class RemoteRuntimeController extends RuntimeController { } logger.debug("RemoteRuntimeController teardown beginning..."); this.#session = undefined; - clearTimeout(this.#refreshTimer); this.#abortController.abort(); // Suppress errors from terminating a WebSocket that hasn't connected yet this.#activeTail?.removeAllListeners("error"); diff --git a/packages/wrangler/src/api/startDevWorker/utils.ts b/packages/wrangler/src/api/startDevWorker/utils.ts index 1db263b8c5..66e8385aa0 100644 --- a/packages/wrangler/src/api/startDevWorker/utils.ts +++ b/packages/wrangler/src/api/startDevWorker/utils.ts @@ -16,13 +16,6 @@ import type { export function assertNever(_value: never) {} -/** - * When to proactively refresh the preview token. - * - * Preview tokens expire after 1 hour (hardcoded in the Workers control plane), so we retry after 50 mins. - */ -export const PREVIEW_TOKEN_REFRESH_INTERVAL = 50 * 60 * 1000; - export type MaybePromise = T | Promise; export type DeferredPromise = { promise: Promise; diff --git a/packages/wrangler/templates/startDevWorker/ProxyWorker.ts b/packages/wrangler/templates/startDevWorker/ProxyWorker.ts index 312fae8b48..bb7a94ef5d 100644 --- a/packages/wrangler/templates/startDevWorker/ProxyWorker.ts +++ b/packages/wrangler/templates/startDevWorker/ProxyWorker.ts @@ -155,9 +155,8 @@ export class ProxyWorker implements DurableObject { res = new Response(res.body, res); rewriteUrlRelatedHeaders(res.headers, innerUrl, outerUrl); - await checkForPreviewTokenError(res, this.env, proxyData); - if (isHtmlResponse(res)) { + await checkForPreviewTokenError(res, this.env, proxyData); res = insertLiveReloadScript(request, res, this.env, proxyData); } @@ -257,15 +256,8 @@ async function checkForPreviewTokenError( // so we clone and read the text instead. const clone = response.clone(); const text = await clone.text(); - // Naive string match should be good enough when combined with status code check. - // "Invalid Workers Preview configuration" is the HTML error returned when the - // preview token has expired. "error code: 1031" is a text/plain error returned - // by remote bindings (e.g. Workers AI) when their underlying session has timed out. - // Both indicate the preview session needs to be refreshed. - if ( - text.includes("Invalid Workers Preview configuration") || - text.includes("error code: 1031") - ) { + // Naive string match should be good enough when combined with status code check + if (text.includes("Invalid Workers Preview configuration")) { void sendMessageToProxyController(env, { type: "previewTokenExpired", proxyData,