diff --git a/.changeset/forty-poets-invite.md b/.changeset/forty-poets-invite.md new file mode 100644 index 000000000000..96eb399210bb --- /dev/null +++ b/.changeset/forty-poets-invite.md @@ -0,0 +1,9 @@ +--- +"wrangler": patch +--- + +fix: only send durable object migrations when required + +We had a bug where even if you'd published a script with migrations, we would still send a blank set of migrations on the next round. The api doesn't accept this, so the fix is to not do so. I also expanded test coverage for migrations. + +Fixes https://github.com/cloudflare/wrangler2/issues/705 diff --git a/packages/wrangler/src/__tests__/publish.test.ts b/packages/wrangler/src/__tests__/publish.test.ts index 56447530737c..3eae211a8b7b 100644 --- a/packages/wrangler/src/__tests__/publish.test.ts +++ b/packages/wrangler/src/__tests__/publish.test.ts @@ -18,6 +18,7 @@ import { runWrangler } from "./helpers/run-wrangler"; import writeWranglerToml from "./helpers/write-wrangler-toml"; import type { WorkerMetadata } from "../create-worker-upload-form"; import type { KVNamespaceInfo } from "../kv"; +import type { CfWorkerInit } from "../worker"; import type { FormData, File } from "undici"; describe("publish", () => { @@ -2063,9 +2064,222 @@ export default{ }); }); + describe("durable object migrations", () => { + it("should warn when you try to publish durable objects without migrations", async () => { + writeWranglerToml({ + durable_objects: { + bindings: [{ name: "SOMENAME", class_name: "SomeClass" }], + }, + }); + fs.writeFileSync( + "index.js", + `export class SomeClass{}; export default {};` + ); + mockSubDomainRequest(); + mockUploadWorkerRequest(); + await runWrangler("publish index.js"); + expect(std.out).toMatchInlineSnapshot(` + "Uploaded test-name (TIMINGS) + Published test-name (TIMINGS) + test-name.test-sub-domain.workers.dev" + `); + expect(std.err).toMatchInlineSnapshot(`""`); + expect(std.warn).toMatchInlineSnapshot( + `"In wrangler.toml, you have configured [durable_objects] exported by this Worker (SomeClass), but no [migrations] for them. This may not work as expected until you add a [migrations] section to your wrangler.toml. Refer to https://developers.cloudflare.com/workers/learning/using-durable-objects/#durable-object-migrations-in-wranglertoml for more details."` + ); + }); + + it("does not warn if all the durable object bindings are to external classes", async () => { + writeWranglerToml({ + durable_objects: { + bindings: [ + { + name: "SOMENAME", + class_name: "SomeClass", + script_name: "some-script", + }, + ], + }, + }); + fs.writeFileSync( + "index.js", + `export class SomeClass{}; export default {};` + ); + mockSubDomainRequest(); + mockUploadWorkerRequest(); + await runWrangler("publish index.js"); + expect(std.out).toMatchInlineSnapshot(` + "Uploaded test-name (TIMINGS) + Published test-name (TIMINGS) + test-name.test-sub-domain.workers.dev" + `); + expect(std.err).toMatchInlineSnapshot(`""`); + expect(std.warn).toMatchInlineSnapshot(`""`); + }); + + it("should publish all migrations on first publish", async () => { + writeWranglerToml({ + durable_objects: { + bindings: [ + { name: "SOMENAME", class_name: "SomeClass" }, + { name: "SOMEOTHERNAME", class_name: "SomeOtherClass" }, + ], + }, + migrations: [ + { tag: "v1", new_classes: ["SomeClass"] }, + { tag: "v2", new_classes: ["SomeOtherClass"] }, + ], + }); + fs.writeFileSync( + "index.js", + `export class SomeClass{}; export class SomeOtherClass{}; export default {};` + ); + mockSubDomainRequest(); + mockScriptData({ scripts: [] }); // no previously uploaded scripts at all + mockUploadWorkerRequest({ + expectedMigrations: { + new_tag: "v2", + steps: [ + { new_classes: ["SomeClass"] }, + { new_classes: ["SomeOtherClass"] }, + ], + }, + }); + + await runWrangler("publish index.js"); + expect(std.out).toMatchInlineSnapshot(` + "Uploaded test-name (TIMINGS) + Published test-name (TIMINGS) + test-name.test-sub-domain.workers.dev" + `); + expect(std.err).toMatchInlineSnapshot(`""`); + expect(std.warn).toMatchInlineSnapshot(`""`); + }); + + it("should upload migrations past a previously uploaded tag", async () => { + writeWranglerToml({ + durable_objects: { + bindings: [ + { name: "SOMENAME", class_name: "SomeClass" }, + { name: "SOMEOTHERNAME", class_name: "SomeOtherClass" }, + ], + }, + migrations: [ + { tag: "v1", new_classes: ["SomeClass"] }, + { tag: "v2", new_classes: ["SomeOtherClass"] }, + ], + }); + fs.writeFileSync( + "index.js", + `export class SomeClass{}; export class SomeOtherClass{}; export default {};` + ); + mockSubDomainRequest(); + mockScriptData({ scripts: [{ id: "test-name", migration_tag: "v1" }] }); + mockUploadWorkerRequest({ + expectedMigrations: { + old_tag: "v1", + new_tag: "v2", + steps: [ + { + new_classes: ["SomeOtherClass"], + }, + ], + }, + }); + + await runWrangler("publish index.js"); + expect(std).toMatchInlineSnapshot(` + Object { + "err": "", + "out": "Uploaded test-name (TIMINGS) + Published test-name (TIMINGS) + test-name.test-sub-domain.workers.dev", + "warn": "", + } + `); + }); + + it("should not send migrations if they've all already been sent", async () => { + writeWranglerToml({ + durable_objects: { + bindings: [ + { name: "SOMENAME", class_name: "SomeClass" }, + { name: "SOMEOTHERNAME", class_name: "SomeOtherClass" }, + ], + }, + migrations: [ + { tag: "v1", new_classes: ["SomeClass"] }, + { tag: "v2", new_classes: ["SomeOtherClass"] }, + { tag: "v3", new_classes: ["YetAnotherClass"] }, + ], + }); + fs.writeFileSync( + "index.js", + `export class SomeClass{}; export class SomeOtherClass{}; export class YetAnotherClass{}; export default {};` + ); + mockSubDomainRequest(); + mockScriptData({ scripts: [{ id: "test-name", migration_tag: "v3" }] }); + mockUploadWorkerRequest({ + expectedMigrations: undefined, + }); + + await runWrangler("publish index.js"); + expect(std).toMatchInlineSnapshot(` + Object { + "err": "", + "out": "Uploaded test-name (TIMINGS) + Published test-name (TIMINGS) + test-name.test-sub-domain.workers.dev", + "warn": "", + } + `); + }); + + describe("service environments", () => { + it("should error when using service environments + durable objects", async () => { + writeWranglerToml({ + durable_objects: { + bindings: [ + { name: "SOMENAME", class_name: "SomeClass" }, + { name: "SOMEOTHERNAME", class_name: "SomeOtherClass" }, + ], + }, + migrations: [{ tag: "v1", new_classes: ["SomeClass"] }], + }); + fs.writeFileSync( + "index.js", + `export class SomeClass{}; export class SomeOtherClass{}; export default {};` + ); + + mockSubDomainRequest(); + + await expect( + runWrangler("publish index.js --legacy-env false") + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"Publishing Durable Objects to a service environment is not currently supported. This is being tracked at https://github.com/cloudflare/wrangler2/issues/739"` + ); + expect(std).toMatchInlineSnapshot(` + Object { + "err": "Publishing Durable Objects to a service environment is not currently supported. This is being tracked at https://github.com/cloudflare/wrangler2/issues/739 + + %s If you think this is a bug then please create an issue at https://github.com/cloudflare/wrangler2/issues/new.", + "out": "", + "warn": "", + } + `); + }); + }); + }); + describe("bindings", () => { it("should allow bindings with different names", async () => { writeWranglerToml({ + migrations: [ + { + tag: "v1", + new_classes: ["SomeDurableObject", "AnotherDurableObject"], + }, + ], durable_objects: { bindings: [ { @@ -2191,6 +2405,7 @@ export default{ ], }); mockSubDomainRequest(); + mockScriptData({ scripts: [] }); await expect(runWrangler("publish index.js")).resolves.toBeUndefined(); expect(std.out).toMatchInlineSnapshot(` @@ -2809,9 +3024,14 @@ export default{ }, ], }, + migrations: [{ tag: "v1", new_classes: ["ExampleDurableObject"] }], }); - writeWorkerSource({ type: "esm" }); + fs.writeFileSync( + "index.js", + `export class ExampleDurableObject {}; export default{};` + ); mockSubDomainRequest(); + mockScriptData({ scripts: [{ id: "test-name", migration_tag: "v1" }] }); mockUploadWorkerRequest({ expectedBindings: [ { @@ -2878,9 +3098,14 @@ export default{ }, ], }, + migrations: [{ tag: "v1", new_classes: ["ExampleDurableObject"] }], }); - writeWorkerSource({ type: "esm" }); + fs.writeFileSync( + "index.js", + `export class ExampleDurableObject {}; export default{};` + ); mockSubDomainRequest(); + mockScriptData({ scripts: [{ id: "test-name", migration_tag: "v1" }] }); mockUploadWorkerRequest({ expectedType: "esm", expectedBindings: [ @@ -3356,27 +3581,32 @@ function writeAssets(assets: { filePath: string; content: string }[]) { } /** Create a mock handler for the request to upload a worker script. */ -function mockUploadWorkerRequest({ - available_on_subdomain = true, - expectedEntry, - expectedType = "esm", - expectedBindings, - expectedModules = {}, - expectedCompatibilityDate, - expectedCompatibilityFlags, - env = undefined, - legacyEnv = false, -}: { - available_on_subdomain?: boolean; - expectedEntry?: string; - expectedType?: "esm" | "sw"; - expectedBindings?: unknown; - expectedModules?: Record; - expectedCompatibilityDate?: string; - expectedCompatibilityFlags?: string[]; - env?: string; - legacyEnv?: boolean; -} = {}) { +function mockUploadWorkerRequest( + options: { + available_on_subdomain?: boolean; + expectedEntry?: string; + expectedType?: "esm" | "sw"; + expectedBindings?: unknown; + expectedModules?: Record; + expectedCompatibilityDate?: string; + expectedCompatibilityFlags?: string[]; + expectedMigrations?: CfWorkerInit["migrations"]; + env?: string; + legacyEnv?: boolean; + } = {} +) { + const { + available_on_subdomain = true, + expectedEntry, + expectedType = "esm", + expectedBindings, + expectedModules = {}, + expectedCompatibilityDate, + expectedCompatibilityFlags, + env = undefined, + legacyEnv = false, + expectedMigrations, + } = options; setMockResponse( env && !legacyEnv ? "/accounts/:accountId/workers/services/:scriptName/environments/:envName" @@ -3405,17 +3635,20 @@ function mockUploadWorkerRequest({ } else { expect(metadata.body_part).toEqual("index.js"); } - if (expectedBindings !== undefined) { + if ("expectedBindings" in options) { expect(metadata.bindings).toEqual(expectedBindings); } - if (expectedCompatibilityDate !== undefined) { + if ("expectedCompatibilityDate" in options) { expect(metadata.compatibility_date).toEqual(expectedCompatibilityDate); } - if (expectedCompatibilityFlags !== undefined) { + if ("expectedCompatibilityFlags" in options) { expect(metadata.compatibility_flags).toEqual( expectedCompatibilityFlags ); } + if ("expectedMigrations" in options) { + expect(metadata.migrations).toEqual(expectedMigrations); + } for (const [name, content] of Object.entries(expectedModules)) { expect(await (formBody.get(name) as File).text()).toEqual(content); } @@ -3580,3 +3813,17 @@ function mockDeleteUnusedAssetsRequest( } ); } + +type LegacyScriptInfo = { id: string; migration_tag?: string }; + +function mockScriptData(options: { scripts: LegacyScriptInfo[] }) { + const { scripts } = options; + setMockResponse( + "/accounts/:accountId/workers/scripts", + "GET", + ([_url, accountId]) => { + expect(accountId).toEqual("some-account-id"); + return scripts; + } + ); +} diff --git a/packages/wrangler/src/publish.ts b/packages/wrangler/src/publish.ts index ebb6c0cc3e27..7ac4f2be6c07 100644 --- a/packages/wrangler/src/publish.ts +++ b/packages/wrangler/src/publish.ts @@ -102,14 +102,36 @@ export default async function publish(props: Props): Promise { } ); + // Some validation of durable objects + migrations + if (config.durable_objects.bindings.length > 0) { + // TODO: implement durable objects for service environments + if (!props.legacyEnv) { + throw new Error( + "Publishing Durable Objects to a service environment is not currently supported. This is being tracked at https://github.com/cloudflare/wrangler2/issues/739" + ); + } + + // intrinsic [durable_objects] implies [migrations] + const exportedDurableObjects = config.durable_objects.bindings.filter( + (binding) => !binding.script_name + ); + if (exportedDurableObjects.length > 0 && config.migrations.length === 0) { + console.warn( + `In wrangler.toml, you have configured [durable_objects] exported by this Worker (${exportedDurableObjects.map( + (durable) => durable.class_name + )}), but no [migrations] for them. This may not work as expected until you add a [migrations] section to your wrangler.toml. Refer to https://developers.cloudflare.com/workers/learning/using-durable-objects/#durable-object-migrations-in-wranglertoml for more details.` + ); + } + } + const content = readFileSync(resolvedEntryPointPath, { encoding: "utf-8", }); // if config.migrations - // get current migration tag let migrations; if (config.migrations.length > 0) { + // get current migration tag const scripts = await fetchResult< { id: string; migration_tag: string }[] >(`/accounts/${accountId}/workers/scripts`); @@ -129,15 +151,21 @@ export default async function publish(props: Props): Promise { steps: config.migrations.map(({ tag: _tag, ...rest }) => rest), }; } else { - migrations = { - old_tag: script.migration_tag, - new_tag: config.migrations[config.migrations.length - 1].tag, - steps: config.migrations - .slice(foundIndex + 1) - .map(({ tag: _tag, ...rest }) => rest), - }; + if (foundIndex !== config.migrations.length - 1) { + // there are new migrations to send up + migrations = { + old_tag: script.migration_tag, + new_tag: config.migrations[config.migrations.length - 1].tag, + steps: config.migrations + .slice(foundIndex + 1) + .map(({ tag: _tag, ...rest }) => rest), + }; + } + // else, we're up to date, no migrations to send } } else { + // first time publishing durable objects to this script, + // so we send all the migrations migrations = { new_tag: config.migrations[config.migrations.length - 1].tag, steps: config.migrations.map(({ tag: _tag, ...rest }) => rest),