Skip to content

Commit 452ad0b

Browse files
dario-piotrowiczdario-piotrowicz
authored
Loosen validation around different configurations for Durable Object (#10437)
* Loosen validation around different configurations for Durable Object * improve solution so that the order of the workers doesn't matter * improve sorting by also checking if the scriptName matches the worker name * Update .changeset/loose-glasses-poke.md
1 parent 30716ed commit 452ad0b

File tree

3 files changed

+232
-53
lines changed

3 files changed

+232
-53
lines changed

‎.changeset/loose-glasses-poke.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
"miniflare": patch
3+
---
4+
5+
Loosen validation around different configurations for Durable Object
6+
7+
Allow durable objects to have `enableSql`, `unsafeUniqueKey` and `unsafePreventEviction` configurations set to `undefined` even if the same durable objects are defined with those configurations set to different values (this allows workers using external durable objects not to have to duplicate such configurations in their options)

‎packages/miniflare/src/index.ts‎

Lines changed: 104 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -331,63 +331,114 @@ function getDurableObjectClassNames(
331331
allWorkerOpts: PluginWorkerOptions[]
332332
): DurableObjectClassNames {
333333
const serviceClassNames: DurableObjectClassNames = new Map();
334-
for (const workerOpts of allWorkerOpts) {
335-
const workerServiceName = getUserServiceName(workerOpts.core.name);
336-
for (const designator of Object.values(
337-
workerOpts.do.durableObjects ?? {}
338-
)) {
339-
const {
340-
className,
341-
// Fallback to current worker service if name not defined
342-
serviceName = workerServiceName,
343-
enableSql,
344-
unsafeUniqueKey,
345-
unsafePreventEviction,
346-
container,
347-
} = normaliseDurableObject(designator);
348-
// Get or create `Map` mapping class name to optional unsafe unique key
349-
let classNames = serviceClassNames.get(serviceName);
350-
if (classNames === undefined) {
351-
classNames = new Map();
352-
serviceClassNames.set(serviceName, classNames);
353-
}
354-
if (classNames.has(className)) {
355-
// If we've already seen this class in this service, make sure the
356-
// unsafe unique keys and unsafe prevent eviction values match
357-
const existingInfo = classNames.get(className);
358-
if (existingInfo?.enableSql !== enableSql) {
359-
throw new MiniflareCoreError(
360-
"ERR_DIFFERENT_STORAGE_BACKEND",
361-
`Different storage backends defined for Durable Object "${className}" in "${serviceName}": ${JSON.stringify(
362-
enableSql
363-
)} and ${JSON.stringify(existingInfo?.enableSql)}`
364-
);
334+
335+
const allDurableObjects = allWorkerOpts
336+
.flatMap((workerOpts) => {
337+
const workerServiceName = getUserServiceName(workerOpts.core.name);
338+
339+
return Object.values(workerOpts.do.durableObjects ?? {}).map(
340+
(workerDODesignator) => {
341+
const doInfo = normaliseDurableObject(workerDODesignator);
342+
if (doInfo.serviceName === undefined) {
343+
// Fallback to current worker service if name not defined
344+
doInfo.serviceName = workerServiceName;
345+
}
346+
return {
347+
doInfo,
348+
workerRawName: workerOpts.core.name,
349+
};
365350
}
366-
if (existingInfo?.unsafeUniqueKey !== unsafeUniqueKey) {
367-
throw new MiniflareCoreError(
368-
"ERR_DIFFERENT_UNIQUE_KEYS",
369-
`Multiple unsafe unique keys defined for Durable Object "${className}" in "${serviceName}": ${JSON.stringify(
370-
unsafeUniqueKey
371-
)} and ${JSON.stringify(existingInfo?.unsafeUniqueKey)}`
372-
);
351+
);
352+
})
353+
// We sort the list of durable objects because we want the durable objects without a scriptName or a scriptName
354+
// that matches the raw worker's name (meaning that they are defined within their worker) to be processed first
355+
.sort(({ doInfo, workerRawName }) =>
356+
doInfo.scriptName === undefined || doInfo.scriptName === workerRawName
357+
? -1
358+
: 0
359+
)
360+
.map(({ doInfo }) => doInfo);
361+
362+
for (const doInfo of allDurableObjects) {
363+
const { className, serviceName, container, ...doConfigs } = doInfo;
364+
// We know that the service name is always defined (since if it is not we do default it to the current worker service)
365+
assert(serviceName);
366+
// Get or create `Map` mapping class name to optional unsafe unique key
367+
let classNames = serviceClassNames.get(serviceName);
368+
if (classNames === undefined) {
369+
classNames = new Map();
370+
serviceClassNames.set(serviceName, classNames);
371+
}
372+
373+
if (classNames.has(className)) {
374+
// If we've already seen this class in this service, make sure the
375+
// unsafe unique keys and unsafe prevent eviction values match
376+
const existingInfo = classNames.get(className);
377+
378+
const isDoUnacceptableDiff = (
379+
field: Extract<
380+
keyof typeof doConfigs,
381+
"enableSql" | "unsafeUniqueKey" | "unsafePreventEviction"
382+
>
383+
) => {
384+
if (!existingInfo) {
385+
return false;
373386
}
374-
if (existingInfo?.unsafePreventEviction !== unsafePreventEviction) {
375-
throw new MiniflareCoreError(
376-
"ERR_DIFFERENT_PREVENT_EVICTION",
377-
`Multiple unsafe prevent eviction values defined for Durable Object "${className}" in "${serviceName}": ${JSON.stringify(
378-
unsafePreventEviction
379-
)} and ${JSON.stringify(existingInfo?.unsafePreventEviction)}`
380-
);
387+
388+
const same = existingInfo[field] === doConfigs[field];
389+
if (same) {
390+
return false;
381391
}
382-
} else {
383-
// Otherwise, just add it
384-
classNames.set(className, {
385-
enableSql,
386-
unsafeUniqueKey,
387-
unsafePreventEviction,
388-
container,
389-
});
392+
393+
const oneIsUndefined =
394+
existingInfo[field] === undefined || doConfigs[field] === undefined;
395+
396+
// If one of the configurations is `undefined` (either the current one or the existing one) then there we
397+
// want to consider this as an acceptable difference since we might be in a potentially valid situation in
398+
// which worker A defines a DO with a config, while worker B simply uses the DO from worker A but without
399+
// providing the configuration (thus leaving it `undefined`) (this for example is exactly what Wrangler does
400+
// with the implicitly defined `enableSql` flag)
401+
if (oneIsUndefined) {
402+
return false;
403+
}
404+
405+
return true;
406+
};
407+
408+
if (isDoUnacceptableDiff("enableSql")) {
409+
throw new MiniflareCoreError(
410+
"ERR_DIFFERENT_STORAGE_BACKEND",
411+
`Different storage backends defined for Durable Object "${className}" in "${serviceName}": ${JSON.stringify(
412+
doConfigs.enableSql
413+
)} and ${JSON.stringify(existingInfo?.enableSql)}`
414+
);
415+
}
416+
417+
if (isDoUnacceptableDiff("unsafeUniqueKey")) {
418+
throw new MiniflareCoreError(
419+
"ERR_DIFFERENT_UNIQUE_KEYS",
420+
`Multiple unsafe unique keys defined for Durable Object "${className}" in "${serviceName}": ${JSON.stringify(
421+
doConfigs.unsafeUniqueKey
422+
)} and ${JSON.stringify(existingInfo?.unsafeUniqueKey)}`
423+
);
390424
}
425+
426+
if (isDoUnacceptableDiff("unsafePreventEviction")) {
427+
throw new MiniflareCoreError(
428+
"ERR_DIFFERENT_PREVENT_EVICTION",
429+
`Multiple unsafe prevent eviction values defined for Durable Object "${className}" in "${serviceName}": ${JSON.stringify(
430+
doConfigs.unsafePreventEviction
431+
)} and ${JSON.stringify(existingInfo?.unsafePreventEviction)}`
432+
);
433+
}
434+
} else {
435+
// Otherwise, just add it
436+
classNames.set(className, {
437+
enableSql: doConfigs.enableSql,
438+
unsafeUniqueKey: doConfigs.unsafeUniqueKey,
439+
unsafePreventEviction: doConfigs.unsafePreventEviction,
440+
container,
441+
});
391442
}
392443
}
393444
return serviceClassNames;

‎packages/miniflare/test/plugins/do/index.spec.ts‎

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -482,3 +482,124 @@ test("colo-local actors", async (t) => {
482482
res = await stub.fetch("http://localhost");
483483
t.is(await res.text(), "body:thing2");
484484
});
485+
486+
test("multiple workers with DO conflicting useSQLite booleans cause options error", async (t) => {
487+
const mf = new Miniflare({
488+
workers: [
489+
{
490+
modules: true,
491+
name: "worker-a",
492+
script: "export default {}",
493+
},
494+
],
495+
});
496+
497+
t.teardown(() => mf.dispose());
498+
499+
await t.throwsAsync(
500+
async () => {
501+
await mf.setOptions({
502+
workers: [
503+
{
504+
modules: true,
505+
name: "worker-c",
506+
script: "export default {}",
507+
durableObjects: {
508+
MY_DO: {
509+
className: "MyDo",
510+
scriptName: "worker-a",
511+
useSQLite: false,
512+
},
513+
},
514+
},
515+
{
516+
modules: true,
517+
name: "worker-a",
518+
script: `
519+
import { DurableObject } from "cloudflare:workers";
520+
521+
export class MyDo extends DurableObject {}
522+
523+
export default { }
524+
`,
525+
durableObjects: {
526+
MY_DO: {
527+
className: "MyDo",
528+
scriptName: undefined,
529+
useSQLite: true,
530+
},
531+
},
532+
},
533+
{
534+
modules: true,
535+
name: "worker-b",
536+
script: "export default {}",
537+
durableObjects: {
538+
MY_DO: {
539+
className: "MyDo",
540+
scriptName: "worker-a",
541+
useSQLite: false,
542+
},
543+
},
544+
},
545+
],
546+
});
547+
},
548+
{
549+
instanceOf: Error,
550+
message:
551+
'Different storage backends defined for Durable Object "MyDo" in "core:user:worker-a": false and true',
552+
}
553+
);
554+
});
555+
556+
test("multiple workers with DO useSQLite true and undefined does not cause options error", async (t) => {
557+
const mf = new Miniflare({
558+
workers: [
559+
{
560+
modules: true,
561+
name: "worker-a",
562+
script: "export default {}",
563+
},
564+
],
565+
});
566+
567+
t.teardown(() => mf.dispose());
568+
569+
await t.notThrowsAsync(async () => {
570+
await mf.setOptions({
571+
workers: [
572+
{
573+
modules: true,
574+
name: "worker-a",
575+
script: `
576+
import { DurableObject } from "cloudflare:workers";
577+
578+
export class MyDo extends DurableObject {}
579+
580+
export default { }
581+
`,
582+
durableObjects: {
583+
MY_DO: {
584+
className: "MyDo",
585+
scriptName: undefined,
586+
useSQLite: true,
587+
},
588+
},
589+
},
590+
{
591+
modules: true,
592+
name: "worker-b",
593+
script: "export default {}",
594+
durableObjects: {
595+
MY_DO: {
596+
className: "MyDo",
597+
scriptName: "worker-a",
598+
useSQLite: undefined,
599+
},
600+
},
601+
},
602+
],
603+
});
604+
});
605+
});

0 commit comments

Comments
 (0)