diff --git a/.changeset/fix-remote-bindings-timeout.md b/.changeset/fix-remote-bindings-timeout.md new file mode 100644 index 0000000000..ae83a4a1ac --- /dev/null +++ b/.changeset/fix-remote-bindings-timeout.md @@ -0,0 +1,12 @@ +--- +"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 60f807ca6f..3167ee2dec 100644 --- a/packages/wrangler/src/__tests__/api/startDevWorker/RemoteRuntimeController.test.ts +++ b/packages/wrangler/src/__tests__/api/startDevWorker/RemoteRuntimeController.test.ts @@ -1,6 +1,5 @@ -import { beforeEach, describe, it, vi } from "vitest"; +import { afterEach, 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, @@ -38,10 +37,6 @@ vi.mock("../../../user/access", () => ({ domainUsesAccess: vi.fn(), })); -vi.mock("../../../api/startDevWorker/utils", () => ({ - unwrapHook: vi.fn(), -})); - function makeConfig( overrides: Partial = {} ): StartDevWorkerOptions { @@ -103,12 +98,6 @@ 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", @@ -159,12 +148,106 @@ describe("RemoteRuntimeController", () => { vi.mocked(createWorkerPreview).mockResolvedValue({ value: "test-preview-token", host: "test.workers.dev", - tailUrl: "wss://test.workers.dev/tail", + // No tailUrl — avoids real WebSocket connections in unit tests }); 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 d324ba600d..77e9e7b4cb 100644 --- a/packages/wrangler/src/__tests__/dev/remote-bindings.test.ts +++ b/packages/wrangler/src/__tests__/dev/remote-bindings.test.ts @@ -1,4 +1,5 @@ /* 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 */ @@ -13,6 +14,7 @@ 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 { @@ -714,16 +716,18 @@ describe("dev with remote bindings", { sequential: true, retry: 2 }, () => { await vi.waitFor(() => expect(std.out).toMatch(/Ready/), { timeout: 5_000, }); - expect(sessionOptions).toEqual({ - auth: { - accountId: "some-account-id", - apiToken: { - apiToken: "some-api-token", - }, - }, + expect(sessionOptions).toBeDefined(); + assert(sessionOptions); + const { auth, ...rest1 } = sessionOptions; + expect(rest1).toEqual({ 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; }); @@ -756,16 +760,18 @@ describe("dev with remote bindings", { sequential: true, retry: 2 }, () => { timeout: 5_000, }); - expect(sessionOptions).toEqual({ - auth: { - accountId: "mock-account-id", - apiToken: { - apiToken: "some-api-token", - }, - }, + expect(sessionOptions).toBeDefined(); + assert(sessionOptions); + const { auth: auth2, ...rest2 } = sessionOptions; + expect(rest2).toEqual({ 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 da0bee651a..82b7925226 100644 --- a/packages/wrangler/src/api/remoteBindings/index.ts +++ b/packages/wrangler/src/api/remoteBindings/index.ts @@ -74,9 +74,9 @@ export async function maybeStartOrUpdateRemoteProxySession( preExistingRemoteProxySessionData?: { session: RemoteProxySession; remoteBindings: Record; - auth?: CfAccount | undefined; + auth?: AsyncHook | undefined; } | null, - auth?: CfAccount | undefined + auth?: AsyncHook | undefined ): Promise<{ session: RemoteProxySession; remoteBindings: Record; @@ -180,9 +180,9 @@ export async function maybeStartOrUpdateRemoteProxySession( * @returns the auth hook to pass to the startRemoteProxy session function if any */ function getAuthHook( - auth: CfAccount | undefined, + auth: AsyncHook | 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 e56d0f44cc..2e5be48779 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, unwrapHook } from "./utils"; +import { getBinaryFileContents } from "./utils"; import type { RemoteProxySession } from "../remoteBindings"; import type { BundleCompleteEvent, @@ -208,12 +208,6 @@ 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( { @@ -222,7 +216,10 @@ export class LocalRuntimeController extends RuntimeController { bindings: remoteBindings, }, this.#remoteProxySessionData ?? null, - 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 + : data.config.dev.auth ); } diff --git a/packages/wrangler/src/api/startDevWorker/RemoteRuntimeController.ts b/packages/wrangler/src/api/startDevWorker/RemoteRuntimeController.ts index 039bc5bdca..32afcf750d 100644 --- a/packages/wrangler/src/api/startDevWorker/RemoteRuntimeController.ts +++ b/packages/wrangler/src/api/startDevWorker/RemoteRuntimeController.ts @@ -1,3 +1,4 @@ +import assert from "node:assert"; import { MissingConfigError } from "@cloudflare/workers-utils"; import chalk from "chalk"; import { Mutex } from "miniflare"; @@ -20,7 +21,7 @@ import { getAccessHeaders } from "../../user/access"; import { retryOnAPIFailure } from "../../utils/retry"; import { RuntimeController } from "./BaseController"; import { castErrorCause } from "./events"; -import { unwrapHook } from "./utils"; +import { PREVIEW_TOKEN_REFRESH_INTERVAL, unwrapHook } from "./utils"; import type { CfAccount, CfPreviewSession, @@ -30,6 +31,7 @@ import type { BundleCompleteEvent, BundleStartEvent, PreviewTokenExpiredEvent, + ProxyData, ReloadCompleteEvent, ReloadStartEvent, } from "./events"; @@ -51,6 +53,10 @@ 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] & { @@ -81,13 +87,7 @@ export class RemoteRuntimeController extends RuntimeController { handlePreviewSessionCreationError(err, props.accountId); - this.emitErrorEvent({ - type: "error", - reason: "Failed to create a preview token", - cause: castErrorCause(err), - source: "RemoteRuntimeController", - data: undefined, - }); + throw err; } } @@ -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; + return false; } const token = await this.#previewToken({ @@ -307,30 +307,49 @@ 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; + return false; } 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: { - 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, - }, + proxyData, }); + + 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) { @@ -343,9 +362,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; @@ -385,34 +404,27 @@ export class RemoteRuntimeController extends RuntimeController { return; } - 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 { + assert(this.#latestConfig.dev.auth); + const auth = await unwrapHook(this.#latestConfig.dev.auth); + this.#session = await this.#getPreviewSession( this.#latestConfig, auth, this.#latestRoutes ); - await this.#updatePreviewToken( + const refreshed = await this.#updatePreviewToken( this.#latestConfig, this.#latestBundle, auth, this.#latestRoutes, this.#currentBundleId ); - logger.log(chalk.green("✔ Preview token refreshed successfully")); + + if (refreshed) { + logger.log(chalk.green("✔ Preview token refreshed successfully")); + } } catch (error) { if (error instanceof Error && error.name == "AbortError") { return; @@ -436,6 +448,7 @@ 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; @@ -454,7 +467,7 @@ export class RemoteRuntimeController extends RuntimeController { void this.#mutex.runWith(() => this.#onBundleComplete(ev, id)); } onPreviewTokenExpired(_: PreviewTokenExpiredEvent): void { - logger.log(chalk.dim("⎔ Preview token expired, refreshing...")); + logger.log(chalk.dim("⎔ Refreshing preview token...")); void this.#mutex.runWith(() => this.#refreshPreviewToken()); } @@ -465,6 +478,7 @@ 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 bf195fceb4..46f133e2e7 100644 --- a/packages/wrangler/src/api/startDevWorker/utils.ts +++ b/packages/wrangler/src/api/startDevWorker/utils.ts @@ -16,6 +16,13 @@ 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 bb7a94ef5d..312fae8b48 100644 --- a/packages/wrangler/templates/startDevWorker/ProxyWorker.ts +++ b/packages/wrangler/templates/startDevWorker/ProxyWorker.ts @@ -155,8 +155,9 @@ 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); } @@ -256,8 +257,15 @@ 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 - if (text.includes("Invalid Workers Preview configuration")) { + // 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") + ) { void sendMessageToProxyController(env, { type: "previewTokenExpired", proxyData,