fix(@Persist): prevent null auto-vivification on read operations

examples/persist/src/index.ts

@@ -6,14 +6,18 @@ import { Actor, handler, Persist } from '../../../packages/core/src'
 export class MyPersistActor extends Actor<Env> {
     @Persist
     public myCustomNumber: number = 0;
-    
+
     @Persist
     public myCustomObject: Record<string, any> = {
         customKey: {
             customDeepKey: []
         }
     };
 
+    // Nullable fields are preserved correctly - reading them returns null, not {}
+    @Persist
+    public optionalOwner: string | null = null;
+
     static async nameFromRequest(request: Request): Promise<string | undefined> {
         // Pause for 500 milliseconds to mimic async operation
         await new Promise(resolve => setTimeout(resolve, 500));
@@ -26,6 +30,7 @@ export class MyPersistActor extends Actor<Env> {
         // you could use this as a replacement to `constructor`.
         console.log('Previous value: ', this.myCustomNumber);
         console.log('Previous object: ', JSON.stringify(this.myCustomObject));
+        console.log('Optional owner: ', this.optionalOwner); // null is preserved, not converted to {}
     }
 
     async fetch(request: Request): Promise<Response> {

packages/core/src/actor-configuration.test.ts

@@ -0,0 +1,66 @@
+import { describe, expect, it, vi } from "vitest";
+
+vi.mock("cloudflare:workers", () => {
+  class DurableObject {
+    constructor(_state?: unknown, _env?: unknown) {}
+  }
+
+  class WorkerEntrypoint {}
+
+  return {
+    DurableObject,
+    WorkerEntrypoint,
+    env: {},
+  };
+});
+
+import { Actor, type ActorConfiguration } from "./index";
+
+describe("Actor configuration overrides", () => {
+  it("respects custom upgrade paths defined by subclasses", async () => {
+    let upgradeCalls = 0;
+
+    class CustomPathActor extends Actor<Record<string, never>> {
+      static override configuration(): ActorConfiguration {
+        return {
+          sockets: {
+            upgradePath: "/custom",
+          },
+        };
+      }
+
+      protected override async shouldUpgradeWebSocket(
+        request: Request,
+      ): Promise<boolean> {
+        return request.headers.get("Upgrade")?.toLowerCase() === "websocket";
+      }
+
+      protected override onWebSocketUpgrade(_request: Request): Response {
+        upgradeCalls += 1;
+        return new Response("upgraded", { status: 200 });
+      }
+
+      protected override onRequest(): Promise<Response> {
+        return Promise.resolve(new Response("fallback", { status: 418 }));
+      }
+    }
+
+    const actor = new CustomPathActor(undefined, undefined);
+    (actor as Record<string, unknown>)["_setNameCalled"] = true;
+
+    const upgradeResponse = await actor.fetch(
+      new Request("https://example.com/custom/game", {
+        headers: { Upgrade: "websocket" },
+      }),
+    );
+    // Node/undici Response objects cannot emit 101, so we just ensure the response we returned flows through.
+    expect(upgradeResponse.status).toBe(200);
+    expect(upgradeCalls).toBe(1);
+
+    const fallbackResponse = await actor.fetch(
+      new Request("https://example.com/ws/game"),
+    );
+    expect(fallbackResponse.status).toBe(418);
+    expect(upgradeCalls).toBe(1);
+  });
+});

packages/core/src/index.ts

@@ -107,10 +107,11 @@ export abstract class Actor<E> extends DurableObject<E> {
 
     /**
      * Static method to configure the actor.
-     * @param options 
-     * @returns 
+     * Subclasses can override this to customize upgrade paths or other options.
+     * @param request - Incoming request (optional for subclasses that need context)
+     * @returns Actor configuration values
      */
-    static configuration = (request: Request): ActorConfiguration => {
+    static configuration(request?: Request): ActorConfiguration {
         return { 
             locationHint: undefined, 
             sockets: { 
@@ -237,7 +238,8 @@ export abstract class Actor<E> extends DurableObject<E> {
     async fetch(request: Request): Promise<Response> {
         // If the request route is `/ws` then we should upgrade the connection to a WebSocket
         // Get configuration from the static property
-        const config = (this.constructor as typeof Actor).configuration(request);
+        const ActorClass = this.constructor as typeof Actor;
+        const config = ActorClass.configuration(request);
 
         // Parse the URL to check if the path component matches the upgradePath
         const url = new URL(request.url);
@@ -562,7 +564,8 @@ export function getActor<T extends Actor<any>>(
 ): DurableObjectStub<T> {
     const className = ActorClass.name;
     const envObj = env as unknown as Record<string, DurableObjectNamespace>;
-    const locationHint = (ActorClass as any).configuration().locationHint;
+    const actorConfig = (ActorClass as unknown as typeof Actor).configuration();
+    const locationHint = actorConfig.locationHint;
 
     const bindingName = Object.keys(envObj).find(key => {
         const binding = (env as any).__DURABLE_OBJECT_BINDINGS?.[key];

packages/core/src/persist.test.ts

@@ -0,0 +1,169 @@
+import { describe, expect, it } from "vitest";
+import { __test } from "./persist";
+
+const { createDeepProxy } = __test;
+
+// No-op trigger for testing (we don't need persistence in these tests)
+const noopTrigger = () => {};
+
+/**
+ * Helper to create a proxy using the real createDeepProxy function.
+ * Uses minimal mock instance and property key for testing purposes.
+ */
+function createTestProxy<T extends object>(value: T): T {
+  return createDeepProxy(value, {}, "testProp", noopTrigger);
+}
+
+describe("persist proxy - null auto-vivification bug", () => {
+  it("should NOT mutate null properties to {} when reading them", () => {
+    // This is the bug: reading a null property through the proxy
+    // should return null, not auto-vivify it to {}
+    const original = {
+      ownerId: null as string | null,
+      name: "test",
+    };
+
+    const proxied = createTestProxy(original);
+
+    // Read the null property
+    const ownerId = proxied.ownerId;
+
+    // Should return null as-is without mutation
+    expect(ownerId).toBe(null);
+    expect(original.ownerId).toBe(null);
+  });
+
+  it("should preserve null values in object spread", () => {
+    const original = {
+      id: "match_123",
+      ownerId: null as string | null,
+      firstMoveMadeAt: null as number | null,
+      completedAt: null as number | null,
+describe("persist proxy - error handler heap overflow bug", () => {
+  it("should return undefined on error instead of creating infinite proxy chain", () => {
+    // Create an object with a throwing getter
+    const throwingObj = {
+      get badProp(): never {
+        throw new Error("This getter throws");
+      },
+      normalProp: "hello",
+    };
+
+    const proxied = createTestProxy(throwingObj);
+
+    // Accessing the throwing getter should return undefined, not create {} and recurse
+    // Before the fix, this would cause a heap overflow from infinite proxy recursion
+    const result = proxied.badProp;
+
+    expect(result).toBe(undefined);
+    // The normal prop should still work
+    expect(proxied.normalProp).toBe("hello");
+  });
+
+  it("should not corrupt the original object on error", () => {
+    const original: Record<string, any> = {
+      get explosive(): never {
+        throw new Error("boom");
+      },
+    };
+
+    const proxied = createTestProxy(original);
+
+    // Spread the proxied object (this is what our code does)
+    const copy = { ...proxied };
+
+    // Spread should preserve null values
+    expect(copy.ownerId).toBe(null);
+    expect(copy.firstMoveMadeAt).toBe(null);
+    expect(copy.completedAt).toBe(null);
+
+    // The original should NOT be mutated by a read operation
+    expect(original.ownerId).toBe(null);
+  });
+
+  it("should NOT mutate the underlying object on property read", () => {
+    const original = { nullProp: null as null };
+    const proxied = createTestProxy(original);
+
+    // Just reading should not mutate
+    void proxied.nullProp;
+
+    // The underlying object should remain unchanged
+    expect(original.nullProp).toBe(null);
+  });
+
+  it("real-world: D1 journaling with nullable fields", () => {
+    // Real-world scenario: match state with nullable fields
+    // When serialized to D1, null values must be preserved
+    interface MatchState {
+      id: string;
+      ownerId: string | null;
+      firstMoveMadeAt: number | null;
+      completedAt: number | null;
+    }
+
+    const matchState: MatchState = {
+      id: "match_123",
+      ownerId: null,
+      firstMoveMadeAt: null,
+      completedAt: null,
+    };
+
+    const proxied = createTestProxy(matchState);
+
+    // When we journal to D1, null values should be preserved
+    expect(proxied.ownerId).toBe(null);
+    expect(proxied.firstMoveMadeAt).toBe(null);
+    expect(proxied.completedAt).toBe(null);
+
+    // Verify the original wasn't corrupted
+    expect(matchState.ownerId).toBe(null);
+    expect(matchState.firstMoveMadeAt).toBe(null);
+    expect(matchState.completedAt).toBe(null);
+  });
+
+  it("should preserve undefined values without mutation", () => {
+    const original = {
+      definedProp: undefined as string | undefined,
+      anotherProp: "value",
+    };
+
+    const proxied = createTestProxy(original);
+
+    // Read the undefined property
+    const value = proxied.definedProp;
+
+    // Should return undefined as-is without mutation
+    expect(value).toBe(undefined);
+    expect(original.definedProp).toBe(undefined);
+  });
+
+  it("should preserve falsy but valid values (0, false, empty string)", () => {
+    const original = {
+      zero: 0,
+      falseValue: false,
+      emptyString: "",
+    };
+
+    const proxied = createTestProxy(original);
+
+    // All falsy values should pass through unchanged
+    expect(proxied.zero).toBe(0);
+    expect(proxied.falseValue).toBe(false);
+    expect(proxied.emptyString).toBe("");
+
+    // Original should remain unchanged
+    expect(original.zero).toBe(0);
+    expect(original.falseValue).toBe(false);
+    expect(original.emptyString).toBe("");
+  });
+
+    // Access the throwing getter
+    void proxied.explosive;
+
+    // The original should not be mutated with {}
+    // Check that 'explosive' is still a getter, not {}
+    const descriptor = Object.getOwnPropertyDescriptor(original, "explosive");
+    expect(descriptor?.get).toBeDefined();
+  });
+});

packages/core/src/persist.ts

@@ -77,16 +77,12 @@ function createDeepProxy(value: any, instance: any, propertyKey: string, trigger
                 }
 
                 const prop = Reflect.get(target, key);
-
-                // If the property is null or undefined but is being accessed as an object,
-                // automatically convert it to an object
-                if ((prop === null || prop === undefined) && 
-                    typeof key === 'string' && 
-                    !key.startsWith('_') && 
-                    key !== 'length') {
-                    const newObj = {};
-                    Reflect.set(target, key, newObj);
-                    return createDeepProxy(newObj, instance, propertyKey, triggerPersist);
+
+                // Return null/undefined as-is - these are intentional values
+                // Do NOT auto-vivify on read operations as this mutates the
+                // underlying object and corrupts domain values
+                if (prop === null || prop === undefined) {
+                    return prop;
                 }
 
                 // Special handling for array methods that modify the array
@@ -123,10 +119,10 @@ function createDeepProxy(value: any, instance: any, propertyKey: string, trigger
                 return prop;
             } catch (e) {
                 console.error(`Error accessing property ${String(key)}:`, e);
-                // Return an empty object proxy for error recovery
-                const newObj = {};
-                Reflect.set(target, key, newObj);
-                return createDeepProxy(newObj, instance, propertyKey, triggerPersist);
+                // Return undefined on error - don't auto-vivify as it causes:
+                // 1. Silent data corruption (replaces original value with {})
+                // 2. Infinite proxy recursion leading to heap overflow
+                return undefined;
             }
         },
         set(target, key, newValue) {
@@ -515,6 +511,11 @@ function safeParse(json: string): any {
     });
 }
 
+// Test exports - expose internal functions for unit testing
+export const __test = {
+    createDeepProxy,
+};
+
 /**
  * Helper function to persist a property value to storage.
  */