Skip to content

Commit ec1f813

Browse files
stop getPlatformProxy crashing with internal DOs (#8697)
* pass name to getplatformproxy * fixup * remove out of date warning * stop gpp crashing with internal DOs * changeset * fix e2e * pr feedback * elaborate on error message * move to a docs link * fix failing test * copy improvement * update docs link * update link --------- Co-authored-by: Carmen Popoviciu <[email protected]>
1 parent 6e8e747 commit ec1f813

File tree

8 files changed

+134
-30
lines changed

8 files changed

+134
-30
lines changed

.changeset/shiny-ghosts-flash.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
"wrangler": patch
3+
---
4+
5+
fix: stop getPlatformProxy crashing when internal DOs are present
6+
7+
Internal DOs still do not work with getPlatformProxy, but warn instead of crashing.

fixtures/get-platform-proxy/tests/get-platform-proxy.env.test.ts

Lines changed: 55 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,14 @@
11
import path from "path";
22
import { D1Database, R2Bucket } from "@cloudflare/workers-types";
3-
import { beforeEach, describe, expect, it, vi } from "vitest";
3+
import {
4+
afterEach,
5+
beforeEach,
6+
describe,
7+
expect,
8+
it,
9+
MockInstance,
10+
vi,
11+
} from "vitest";
412
import { getPlatformProxy } from "./shared";
513
import type { Hyperdrive, KVNamespace } from "@cloudflare/workers-types";
614
import type { Unstable_DevWorker } from "wrangler";
@@ -197,6 +205,52 @@ describe("getPlatformProxy - env", () => {
197205
}
198206
});
199207

208+
describe("DO warnings", () => {
209+
let warn = {} as MockInstance<typeof console.warn>;
210+
beforeEach(() => {
211+
warn = vi.spyOn(console, "warn").mockImplementation(() => {});
212+
});
213+
afterEach(() => {
214+
warn.mockRestore();
215+
});
216+
217+
it("warns about internal DOs and doesn't crash", async () => {
218+
await getPlatformProxy<Env>({
219+
configPath: path.join(__dirname, "..", "wrangler_internal_do.jsonc"),
220+
});
221+
expect(warn).toMatchInlineSnapshot(`
222+
[MockFunction warn] {
223+
"calls": [
224+
[
225+
"▲ [WARNING]  You have defined bindings to the following internal Durable Objects:
226+
227+
- {"class_name":"MyDurableObject","name":"MY_DURABLE_OBJECT"}
228+
These will not work in local development, but they should work in production.
229+
230+
If you want to develop these locally, you can define your DO in a separate Worker, with a separate configuration file.
231+
For detailed instructions, refer to the Durable Objects section here: https://developers.cloudflare.com/workers/wrangler/api#supported-bindings
232+
233+
",
234+
],
235+
],
236+
"results": [
237+
{
238+
"type": "return",
239+
"value": undefined,
240+
},
241+
],
242+
}
243+
`);
244+
});
245+
246+
it("doesn't warn about external DOs and doesn't crash", async () => {
247+
await getPlatformProxy<Env>({
248+
configPath: path.join(__dirname, "..", "wrangler_external_do.jsonc"),
249+
});
250+
expect(warn).not.toHaveBeenCalled();
251+
});
252+
});
253+
200254
describe("with a target environment", () => {
201255
it("should provide bindings targeting a specified environment and also inherit top-level ones", async () => {
202256
const { env, dispose } = await getPlatformProxy<Env>({
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
{
2+
"name": "external-do",
3+
"main": "src/index.ts",
4+
// external durable object = binding has script_name. This indicates that
5+
// the DO is exported in a separate Worker called `do-worker`. For the
6+
// purposes of testing, we don't need to set that up because
7+
// getPlatformProxy would not be involved in running that Worker.
8+
9+
"durable_objects": {
10+
"bindings": [
11+
{
12+
"class_name": "MyDurableObject",
13+
"name": "MY_DURABLE_OBJECT",
14+
"script_name": "do-worker",
15+
},
16+
],
17+
},
18+
"migrations": [
19+
{
20+
"new_sqlite_classes": ["MyDurableObject"],
21+
"tag": "v1",
22+
},
23+
],
24+
}
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
{
2+
"name": "internal-do",
3+
"main": "src/index.ts",
4+
// Internal durable object = the binding does not specify a script name.
5+
// This implies the DO is exported alongside this worker in `index.ts`,
6+
// which it isn't actually. However we don't care about this here because
7+
// getPlatformProxy will discard all user code anyway. We are simply making
8+
// sure the warning shows up.
9+
"durable_objects": {
10+
"bindings": [
11+
{
12+
"class_name": "MyDurableObject",
13+
"name": "MY_DURABLE_OBJECT",
14+
},
15+
],
16+
},
17+
"migrations": [
18+
{
19+
"new_sqlite_classes": ["MyDurableObject"],
20+
"tag": "v1",
21+
},
22+
],
23+
}

packages/wrangler/e2e/__snapshots__/pages-dev.test.ts.snap

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,6 @@
22

33
exports[`Pages 'wrangler pages dev' > should merge (with override) \`wrangler.toml\` configuration with configuration provided via the command line, with command line args taking precedence 1`] = `
44
"✨ Compiled Worker successfully
5-
▲ [WARNING] WARNING: You have Durable Object bindings that are not defined locally in the worker being developed.
6-
Be aware that changes to the data stored in these Durable Objects will be permanent and affect the live instances.
7-
Remote Durable Objects that are affected:
8-
- {"name":"DO_BINDING_1_TOML","class_name":"DO_1_TOML","script_name":"DO_SCRIPT_1_TOML"}
9-
- {"name":"DO_BINDING_2_TOML","class_name":"DO_2_TOML","script_name":"DO_SCRIPT_2_TOML"}
105
Your Worker and resources are simulated locally via Miniflare. For more information, see: https://developers.cloudflare.com/workers/testing/local-development.
116
Your worker has access to the following bindings:
127
- Durable Objects:

packages/wrangler/src/__tests__/dev.test.ts

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1288,15 +1288,6 @@ describe.sequential("wrangler dev", () => {
12881288
https://developers.cloudflare.com/durable-objects/reference/durable-objects-migrations/ for more
12891289
details.
12901290
1291-
1292-
▲ [WARNING] WARNING: You have Durable Object bindings that are not defined locally in the worker being developed.
1293-
1294-
Be aware that changes to the data stored in these Durable Objects will be permanent and affect the
1295-
live instances.
1296-
Remote Durable Objects that are affected:
1297-
- {\\"name\\":\\"NAME_2\\",\\"class_name\\":\\"CLASS_2\\",\\"script_name\\":\\"SCRIPT_A\\"}
1298-
- {\\"name\\":\\"NAME_4\\",\\"class_name\\":\\"CLASS_4\\",\\"script_name\\":\\"SCRIPT_B\\"}
1299-
13001291
"
13011292
`);
13021293
});

packages/wrangler/src/api/integrations/platform/index.ts

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { kCurrentWorker, Miniflare } from "miniflare";
22
import { getAssetsOptions } from "../../../assets";
33
import { readConfig } from "../../../config";
4+
import { partitionDurableObjectBindings } from "../../../deployment-bundle/entry";
45
import { DEFAULT_MODULE_RULES } from "../../../deployment-bundle/rules";
56
import { getBindings } from "../../../dev";
67
import { getBoundRegisteredWorkers } from "../../../dev-registry";
@@ -11,7 +12,9 @@ import {
1112
buildSitesOptions,
1213
} from "../../../dev/miniflare";
1314
import { run } from "../../../experimental-flags";
15+
import { logger } from "../../../logger";
1416
import { getSiteAssetPaths } from "../../../sites";
17+
import { dedent } from "../../../utils/dedent";
1518
import { CacheStorage } from "./caches";
1619
import { ExecutionContext } from "./executionContext";
1720
import { getServiceBindings } from "./services";
@@ -129,21 +132,35 @@ export async function getPlatformProxy<
129132
};
130133
}
131134

135+
// this is only used by getPlatformProxy
132136
async function getMiniflareOptionsFromConfig(
133137
rawConfig: Config,
134138
env: string | undefined,
135139
options: GetPlatformProxyOptions
136140
): Promise<Partial<MiniflareOptions>> {
137141
const bindings = getBindings(rawConfig, env, true, {});
138142

143+
if (rawConfig["durable_objects"]) {
144+
const { localBindings } = partitionDurableObjectBindings(rawConfig);
145+
if (localBindings.length > 0) {
146+
logger.warn(dedent`
147+
You have defined bindings to the following internal Durable Objects:
148+
${localBindings.map((b) => `- ${JSON.stringify(b)}`).join("\n")}
149+
These will not work in local development, but they should work in production.
150+
151+
If you want to develop these locally, you can define your DO in a separate Worker, with a separate configuration file.
152+
For detailed instructions, refer to the Durable Objects section here: https://developers.cloudflare.com/workers/wrangler/api#supported-bindings
153+
`);
154+
}
155+
}
139156
const workerDefinitions = await getBoundRegisteredWorkers({
140157
name: rawConfig.name,
141158
services: bindings.services,
142159
durableObjects: rawConfig["durable_objects"],
143160
});
144161

145162
const { bindingOptions, externalWorkers } = buildMiniflareBindingOptions({
146-
name: undefined,
163+
name: rawConfig.name,
147164
bindings,
148165
workerDefinitions,
149166
queueConsumers: undefined,
@@ -162,6 +179,7 @@ async function getMiniflareOptionsFromConfig(
162179
{
163180
script: "",
164181
modules: true,
182+
name: rawConfig.name,
165183
...bindingOptions,
166184
serviceBindings: {
167185
...serviceBindings,

packages/wrangler/src/deployment-bundle/entry.ts

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import path from "node:path";
22
import dedent from "ts-dedent";
33
import { configFileName, formatConfigSnippet } from "../config";
44
import { UserError } from "../errors";
5-
import { logger } from "../logger";
65
import { sniffUserAgent } from "../package-manager";
76
import guessWorkerFormat from "./guess-worker-format";
87
import {
@@ -130,17 +129,7 @@ export async function getEntry(
130129
config.tsconfig
131130
);
132131

133-
const { localBindings, remoteBindings } =
134-
partitionDurableObjectBindings(config);
135-
136-
if (command === "dev" && remoteBindings.length > 0) {
137-
logger.warn(
138-
"WARNING: You have Durable Object bindings that are not defined locally in the worker being developed.\n" +
139-
"Be aware that changes to the data stored in these Durable Objects will be permanent and affect the live instances.\n" +
140-
"Remote Durable Objects that are affected:\n" +
141-
remoteBindings.map((b) => `- ${JSON.stringify(b)}`).join("\n")
142-
);
143-
}
132+
const { localBindings } = partitionDurableObjectBindings(config);
144133

145134
if (format === "service-worker" && localBindings.length > 0) {
146135
const errorMessage =
@@ -171,14 +160,17 @@ export async function getEntry(
171160
* Groups the durable object bindings into two lists:
172161
* those that are defined locally and those that refer to a durable object defined in another script.
173162
*/
174-
function partitionDurableObjectBindings(config: Config): {
163+
export function partitionDurableObjectBindings(config: Config): {
175164
localBindings: DurableObjectBindings;
176165
remoteBindings: DurableObjectBindings;
177166
} {
178167
const localBindings: DurableObjectBindings = [];
179168
const remoteBindings: DurableObjectBindings = [];
180169
for (const binding of config.durable_objects.bindings) {
181-
if (binding.script_name === undefined) {
170+
if (
171+
binding.script_name === undefined ||
172+
binding.script_name === config.name
173+
) {
182174
localBindings.push(binding);
183175
} else {
184176
remoteBindings.push(binding);

0 commit comments

Comments
 (0)