ENG-1324 - Initial Cloudflare-backed tldraw sync (single feature flag, PoC)#749
ENG-1324 - Initial Cloudflare-backed tldraw sync (single feature flag, PoC)#749
Conversation
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
…grate Cloudflare sync store. Update TldrawCanvasCloudflareSync to utilize new asset store and improve store management with custom shape and binding utilities.
…d various Cloudflare packages. Added peer dependencies for esbuild plugins and updated existing package versions for improved compatibility.
…s to utilize Roam's native asset store. Update TldrawCanvasCloudflareSync to integrate the new asset management approach.
…re types and adding workers-types. Refactor TldrawDurableObject to specify type for schedulePersistToR2. Adjust router type definitions in worker.ts for improved clarity.
- Introduced `getSyncRoomId` function to create a unique room ID based on the page UID and graph name. - Updated `useCloudflareSyncStore` to utilize the new room ID for WebSocket connections. - Added comments in `Tldraw.tsx` to clarify canvas identity handling in Roam.
- Updated `getSyncRoomId` to encode the room ID as a base64url token, enhancing security and compatibility. - Clarified comments in `Tldraw.tsx` regarding canvas identity handling in Roam.
- Introduced a set of functions to manage CORS headers and validate allowed origins. - Replaced the previous CORS setup with a more flexible approach, allowing specific origins and Vercel preview URLs. - Updated the router to enforce CORS checks and handle preflight requests for OPTIONS method. - Enhanced the /connect and /unfurl routes to include CORS headers in responses.
|
Switching to ready for review to trigger bot reviews. This is not intended to be merged to main |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| const isAllowedOrigin = (origin: string): boolean => | ||
| ALLOWED_ORIGINS.includes(origin) || | ||
| ALLOWED_ORIGINS.some((allowedOrigin) => origin.startsWith(allowedOrigin)) || | ||
| isVercelPreviewUrl(origin); |
There was a problem hiding this comment.
🔴 CORS origin bypass via startsWith allows attacker-controlled origins
The isAllowedOrigin function in the Cloudflare sync worker uses origin.startsWith(allowedOrigin) to validate request origins, which allows any origin that merely begins with an allowed origin string to pass the CORS check.
Root Cause and Impact
The check at apps/tldraw-sync-worker/worker/worker.ts:19 applies origin.startsWith(allowedOrigin) for each entry in ALLOWED_ORIGINS. This means an attacker-controlled domain like https://roamresearch.com.evil.com passes validation because it starts with https://roamresearch.com.
Verified with:
isAllowedOrigin('https://roamresearch.com.evil.com') // => true
isAllowedOrigin('http://localhost:3000.evil.com') // => trueAdditionally, the startsWith check is entirely redundant with the ALLOWED_ORIGINS.includes(origin) check on line 18 for exact matches — the only additional origins it matches are malicious substrings.
Impact: An attacker can host a malicious page on a domain like roamresearch.com.evil.com and connect to the sync worker's WebSocket rooms, potentially reading/writing canvas data for any room whose ID they can guess or obtain.
| const isAllowedOrigin = (origin: string): boolean => | |
| ALLOWED_ORIGINS.includes(origin) || | |
| ALLOWED_ORIGINS.some((allowedOrigin) => origin.startsWith(allowedOrigin)) || | |
| isVercelPreviewUrl(origin); | |
| const isAllowedOrigin = (origin: string): boolean => | |
| ALLOWED_ORIGINS.includes(origin) || | |
| isVercelPreviewUrl(origin); | |
Was this helpful? React with 👍 or 👎 to provide feedback.
📝 WalkthroughWalkthroughThis PR introduces Cloudflare-based synchronization for Tldraw canvases alongside the existing Roam integration. It adds a new dependency, creates a new Cloudflare Worker with Durable Object and R2 persistence, implements a unified store adapter pattern supporting both sync paths, and enhances error handling and performance through memoization. Changes
Sequence Diagram(s)sequenceDiagram
participant RoamApp as Roam App
participant CFWorker as Cloudflare Worker
participant DurObj as Durable Object<br/>(TldrawDurableObject)
participant R2 as R2 Storage
participant Client as WebSocket Client
RoamApp->>RoamApp: useCloudflareSync flag ON
RoamApp->>RoamApp: useCloudflareSyncStore initialized
RoamApp->>CFWorker: GET /connect/:roomId<br/>(pageUid, sessionId, schema config)
CFWorker->>DurObj: Route to Durable Object instance
DurObj->>DurObj: Load or initialize room
DurObj->>R2: Load snapshot + schema config
R2-->>DurObj: Room state & schema
DurObj->>DurObj: Merge schema<br/>(custom shapes/bindings)
DurObj->>DurObj: Create TLSocketRoom<br/>(with merged schema)
DurObj->>CFWorker: Create WebSocket pair
CFWorker-->>RoamApp: WebSocket connection<br/>(store ready)
RoamApp->>Client: Sync active
Client->>DurObj: Send canvas updates
DurObj->>DurObj: Update in-memory state
DurObj->>R2: Persist (throttled ~10s)
R2-->>DurObj: Persist ACK
DurObj->>Client: Broadcast update
Client-->>RoamApp: Receive sync
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@apps/roam/src/components/canvas/Tldraw.tsx`:
- Around line 151-165: The component TldrawCanvas currently chooses Cloudflare
sync when useCloudflareSync (computed from TLDRAW_CLOUDFLARE_SYNC_ENABLED,
TLDRAW_CLOUDFLARE_SYNC_WS_BASE_URL and hasRoamPersistedCanvasData(pageUid)) is
true, which causes all new canvases to use TldrawCanvasCloudflare with no
fallback; change this by wrapping the Cloudflare initialization in a guarded
try/catch with a timeout/failure path that falls back to rendering
TldrawCanvasRoam, and move the hasRoamPersistedCanvasData check into an effect
or async memo so the component can re-evaluate and switch to TldrawCanvasRoam if
Roam-persisted data appears after mount; reference the symbols TldrawCanvas,
useCloudflareSync, TldrawCanvasCloudflare, TldrawCanvasRoam,
hasRoamPersistedCanvasData, TLDRAW_CLOUDFLARE_SYNC_ENABLED, and
TLDRAW_CLOUDFLARE_SYNC_WS_BASE_URL when implementing the guarded init and
re-check logic.
In `@apps/roam/src/components/canvas/TldrawCanvasCloudflareSync.tsx`:
- Around line 25-38: getSyncRoomId deterministically encodes {graphName,
pageUid} into a predictable base64url room id allowing unauthenticated access;
update the implementation to document the security boundary by adding a clear
TODO comment in getSyncRoomId (and near any sync connection code) that an
authentication/authorization token must be added before production (e.g., "TODO:
Add auth token / verify user permissions for sync rooms"), and, if possible, add
a short inline note in the connect/subscribe logic referencing getSyncRoomId so
future reviewers know this is intentionally insecure for PoC only and must be
replaced with an authenticated room-id or signed token.
In `@apps/tldraw-sync-worker/worker/worker.ts`:
- Around line 17-20: The isAllowedOrigin function currently uses
ALLOWED_ORIGINS.includes and .some(... origin.startsWith(...)) which allows
subdomain spoofing (e.g., "https://roamresearch.com.evil.com"); remove the
startsWith-based check and the redundant includes call and instead perform
strict origin equality against ALLOWED_ORIGINS or, if you must support prefix
matches, ensure you compare allowedOrigin followed only by end-of-string or a
"/" (i.e., check origin === allowedOrigin || origin === allowedOrigin + "/" or
verify the character after allowedOrigin is "/" or absent); keep the
isVercelPreviewUrl(origin) call as-is.
In `@apps/tldraw-sync-worker/wrangler.toml`:
- Line 20: Fix the typo in the comment within wrangler.toml: change the word
"assest" to "assets" in the comment that reads "# We store room snapshots
(assest are stored in native apps) in an R2 bucket" so it correctly reads
"assets".
🧹 Nitpick comments (14)
apps/roam/src/components/canvas/useRoamStore.ts (1)
41-50: Duplicated prop-extraction logic — consider extracting a shared helper.The pattern of reading
getBlockProps(pageUid)→ extractingroamjs-query-builder→ accessing.tldrawis repeated here and in theuseEffectat Lines 125–129. A small shared helper (e.g.,getRoamTldrawData(pageUid)) would reduce duplication and keep the two call sites in sync.Not blocking for a PoC, but worth noting for follow-up.
apps/roam/src/components/canvas/TldrawCanvasCloudflareSync.tsx (2)
14-17: Hardcoded feature flag and worker URL — use environment variables or runtime config.
TLDRAW_CLOUDFLARE_SYNC_ENABLEDis unconditionallytrueand the worker URL is a hardcoded string. For a PoC this is understandable, but before any broader rollout:
- The feature flag should be driven by a runtime setting (e.g., Roam extension settings or an env var) so it can be toggled without a code change.
- The worker URL should come from an environment variable (e.g.,
process.env.TLDRAW_SYNC_WORKER_URL) to support different environments (dev/staging/prod).
40-42: Fragile markdown-image stripping — share with the identical logic in Tldraw.tsx.This exact regex pair (
replace(/^!\[\]\(/, "").replace(/\)$/, "")) also appears in Tldraw.tsx (Lines 1193, 1272). Extract it into a shared utility to keep the parsing logic consistent and DRY.apps/roam/src/components/canvas/Tldraw.tsx (3)
263-271: Passing a hook as a component prop is unconventional — add a clarifying comment.
useStoreAdapteris a custom hook passed as a prop, which is valid React but non-obvious to readers. The critical constraint is that it must be called unconditionally (not behind a conditional), which is satisfied at Line 714. A brief comment documenting this contract would help future maintainers.Suggested comment
const TldrawCanvasShared = ({ title, pageUid, useStoreAdapter, }: { title: string; pageUid: string; + /** Custom hook — must be called unconditionally to satisfy Rules of Hooks. */ useStoreAdapter: (args: CanvasStoreAdapterArgs) => CanvasStoreAdapterResult; }) => {
898-903: Error message leaks implementation details to end users.The
invalidRecorderror message references "Cloudflare sync worker schema" and "DG custom shapes and bindings" — these are developer-facing concepts that won't help an end user. Consider a more user-friendly message with the technical details kept in the console log (which you already have at Lines 819–831).Suggested change
- ? "Cloudflare sync rejected a custom Discourse Graph record (invalidRecord). The sync worker schema must include DG custom shapes and bindings." - : "There was a problem loading the Tldraw canvas."}{" "} - {error?.message ? `Details: ${error.message}` : ""} + ? "There was a sync configuration error. Please contact support if this persists." + : "There was a problem loading the canvas."}{" "}
919-927:storeprop type mismatch between Roam and Cloudflare paths.When using the Roam adapter,
storeisTLStore | null(null is guarded out by Line 885). When using Cloudflare,storeisTLStoreWithStatus.TldrawEditoraccepts bothTLStoreandTLStoreWithStatus, so this works at runtime. However, theCanvasStoreAdapterResulttype at Line 177 declaresstore: TLStore | TLStoreWithStatus | null— this wide union can mask type errors. Consider narrowing the type or using a discriminated union.apps/tldraw-sync-worker/worker/types.ts (1)
3-6: Prefertypeoverinterfaceper coding guidelines.♻️ Suggested change
-export interface Environment { - TLDRAW_BUCKET: R2Bucket - TLDRAW_DURABLE_OBJECT: DurableObjectNamespace -} +export type Environment = { + TLDRAW_BUCKET: R2Bucket; + TLDRAW_DURABLE_OBJECT: DurableObjectNamespace; +};As per coding guidelines: "Prefer
typeoverinterfacefor TypeScript".apps/tldraw-sync-worker/tsconfig.json (1)
2-14: Consider adding an explicittargetcompiler option.
targetdefaults toES3when omitted. While wrangler handles bundling and this may not manifest as a real issue, setting"target": "ES2020"(matchinglib) would be more consistent and avoids unnecessary down-level transpilation of syntax like optional chaining or nullish coalescing during type-checking.apps/tldraw-sync-worker/worker/TldrawDurableObject.ts (5)
19-33: Missing explicit return type oncreateRoomSchema.♻️ Suggested change
-const createRoomSchema = ({ shapeTypes, bindingTypes }: RoomSchemaConfig) => { +const createRoomSchema = ({ shapeTypes, bindingTypes }: RoomSchemaConfig): ReturnType<typeof createTLSchema> => {As per coding guidelines: "Use explicit return types for functions in TypeScript".
92-97: Useconsole.errorinstead ofconsole.logfor error handling.The outer worker router (in
worker.ts) usesconsole.error(e), but this inner DO router usesconsole.log(e). This inconsistency could cause errors to be missed when filtering logs by severity.private readonly router = AutoRouter({ catch: (e) => { - console.log(e) + console.error(e) return error(e) }, })
144-157: Schema evolution disconnects all existing clients — document this trade-off.When
ensureSchemaConfigdetects a schema change, it closes the existing room (line 154), which disconnects all current WebSocket clients. They must reconnect and will see the new schema. This is fine for a PoC but should be called out as a known limitation, since a new client connecting with an additional custom shape type will disrupt all active sessions.As per coding guidelines: "Document limitations, known bugs, or edge cases where behavior may not align with expectations".
170-191: Add an explicit return type togetRoom.This is a public method but is missing its return type annotation.
- getRoom() { + getRoom(): Promise<TLSocketRoom<TLRecord, void>> {As per coding guidelines: "Use explicit return types for functions in TypeScript".
193-201: Throttled persistence can lose up to 10 seconds of data on DO eviction.If the Durable Object is evicted (hibernation, crash, or Cloudflare recycling) between
onDataChangefiring and the throttle flushing, the latest snapshot won't be persisted. For a PoC this is acceptable, but document the limitation. In a production version, consider usingctx.waitUntil()to extend the DO lifetime until the persist completes, or flush onwebSocketClose/webSocketErrorevents.As per coding guidelines: "Document limitations, known bugs, or edge cases where behavior may not align with expectations".
apps/tldraw-sync-worker/package.json (1)
1-25: Dependency versions confirmed compatible. Consider upgradingwranglerfrom v3 to v4.The date-based versioning scheme for
@cloudflare/workers-types(4.YYYYMMDD.N) is standard and valid. Version4.20240208.0(Feb 8, 2024) is compatible with thecompatibility_date = "2024-07-01"inwrangler.toml. Similarly,wrangler ^3.64.0is compatible with that date.However,
wranglerv4 became the current major version in March 2025 and is now at v4.63.0. While v3 still functions, upgrading to v4 would keep the project on the supported major version line and future-proof it as a PoC that may grow.
| const TldrawCanvas = ({ title }: { title: string }) => { | ||
| // In Roam, canvas identity is currently keyed by the page UID. | ||
| // Room sync is graph/page encoded as an opaque base64url token. | ||
| const pageUid = useMemo(() => getPageUidByPageTitle(title), [title]); | ||
| const useCloudflareSync = | ||
| TLDRAW_CLOUDFLARE_SYNC_ENABLED && | ||
| !!TLDRAW_CLOUDFLARE_SYNC_WS_BASE_URL && | ||
| !hasRoamPersistedCanvasData(pageUid); | ||
|
|
||
| if (useCloudflareSync) { | ||
| return <TldrawCanvasCloudflare title={title} pageUid={pageUid} />; | ||
| } | ||
|
|
||
| return <TldrawCanvasRoam title={title} pageUid={pageUid} />; | ||
| }; |
There was a problem hiding this comment.
Cloudflare sync is active for all new canvases — verify this is intentional for the PoC.
With TLDRAW_CLOUDFLARE_SYNC_ENABLED hardcoded to true, every canvas page without existing Roam-persisted data will route through Cloudflare sync. If the worker is down or unreachable, those canvases will fail to load with no fallback. Consider adding a try/catch or timeout-based fallback to the Roam path for resilience.
Also, hasRoamPersistedCanvasData(pageUid) is called synchronously during render. This is fine given Roam's synchronous API, but it means the decision is made once per mount — if data is persisted after mount (e.g., by another tab), the component won't switch paths.
🤖 Prompt for AI Agents
In `@apps/roam/src/components/canvas/Tldraw.tsx` around lines 151 - 165, The
component TldrawCanvas currently chooses Cloudflare sync when useCloudflareSync
(computed from TLDRAW_CLOUDFLARE_SYNC_ENABLED,
TLDRAW_CLOUDFLARE_SYNC_WS_BASE_URL and hasRoamPersistedCanvasData(pageUid)) is
true, which causes all new canvases to use TldrawCanvasCloudflare with no
fallback; change this by wrapping the Cloudflare initialization in a guarded
try/catch with a timeout/failure path that falls back to rendering
TldrawCanvasRoam, and move the hasRoamPersistedCanvasData check into an effect
or async memo so the component can re-evaluate and switch to TldrawCanvasRoam if
Roam-persisted data appears after mount; reference the symbols TldrawCanvas,
useCloudflareSync, TldrawCanvasCloudflare, TldrawCanvasRoam,
hasRoamPersistedCanvasData, TLDRAW_CLOUDFLARE_SYNC_ENABLED, and
TLDRAW_CLOUDFLARE_SYNC_WS_BASE_URL when implementing the guarded init and
re-check logic.
| const getSyncRoomId = ({ pageUid }: { pageUid: string }): string => { | ||
| const graphName = window.roamAlphaAPI.graph.name; | ||
| const payload = JSON.stringify({ graphName, pageUid }); | ||
| const bytes = new TextEncoder().encode(payload); | ||
| let binary = ""; | ||
| for (const byte of bytes) { | ||
| binary += String.fromCharCode(byte); | ||
| } | ||
|
|
||
| return btoa(binary) | ||
| .replace(/\+/g, "-") | ||
| .replace(/\//g, "_") | ||
| .replace(/=+$/g, ""); | ||
| }; |
There was a problem hiding this comment.
Room IDs are unauthenticated and predictable — document the security boundary.
getSyncRoomId deterministically encodes {graphName, pageUid} into a base64url token. Anyone who can guess or learn a graph name and page UID can connect to the sync room and read/write canvas state. This is fine for a PoC, but the lack of authentication should be called out explicitly (e.g., a // TODO: Add auth token comment) so it isn't forgotten before production use.
🤖 Prompt for AI Agents
In `@apps/roam/src/components/canvas/TldrawCanvasCloudflareSync.tsx` around lines
25 - 38, getSyncRoomId deterministically encodes {graphName, pageUid} into a
predictable base64url room id allowing unauthenticated access; update the
implementation to document the security boundary by adding a clear TODO comment
in getSyncRoomId (and near any sync connection code) that an
authentication/authorization token must be added before production (e.g., "TODO:
Add auth token / verify user permissions for sync rooms"), and, if possible, add
a short inline note in the connect/subscribe logic referencing getSyncRoomId so
future reviewers know this is intentionally insecure for PoC only and must be
replaced with an authenticated room-id or signed token.
| const isAllowedOrigin = (origin: string): boolean => | ||
| ALLOWED_ORIGINS.includes(origin) || | ||
| ALLOWED_ORIGINS.some((allowedOrigin) => origin.startsWith(allowedOrigin)) || | ||
| isVercelPreviewUrl(origin); |
There was a problem hiding this comment.
startsWith check allows CORS bypass via subdomain spoofing.
origin.startsWith("https://roamresearch.com") would also match https://roamresearch.com.evil.com. An attacker-controlled domain that starts with an allowed origin string will pass this check and receive CORS headers granting access. The includes check on line 18 is also redundant since startsWith is a superset of exact match.
Remove the startsWith check entirely, or compare against origin + / to ensure a full domain match.
🔒 Proposed fix
const isAllowedOrigin = (origin: string): boolean =>
- ALLOWED_ORIGINS.includes(origin) ||
- ALLOWED_ORIGINS.some((allowedOrigin) => origin.startsWith(allowedOrigin)) ||
- isVercelPreviewUrl(origin);
+ ALLOWED_ORIGINS.includes(origin) || isVercelPreviewUrl(origin);If you need prefix matching for path-based origins in the future, validate that the character immediately after the allowed origin is / or end-of-string.
🤖 Prompt for AI Agents
In `@apps/tldraw-sync-worker/worker/worker.ts` around lines 17 - 20, The
isAllowedOrigin function currently uses ALLOWED_ORIGINS.includes and .some(...
origin.startsWith(...)) which allows subdomain spoofing (e.g.,
"https://roamresearch.com.evil.com"); remove the startsWith-based check and the
redundant includes call and instead perform strict origin equality against
ALLOWED_ORIGINS or, if you must support prefix matches, ensure you compare
allowedOrigin followed only by end-of-string or a "/" (i.e., check origin ===
allowedOrigin || origin === allowedOrigin + "/" or verify the character after
allowedOrigin is "/" or absent); keep the isVercelPreviewUrl(origin) call as-is.
| tag = "v1" | ||
| new_sqlite_classes = ["TldrawDurableObject"] | ||
|
|
||
| # We store room snapshots (assest are stored in native apps) in an R2 bucket |
There was a problem hiding this comment.
Typo: "assest" → "assets".
-# We store room snapshots (assest are stored in native apps) in an R2 bucket
+# We store room snapshots (assets are stored in native apps) in an R2 bucket📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # We store room snapshots (assest are stored in native apps) in an R2 bucket | |
| # We store room snapshots (assets are stored in native apps) in an R2 bucket |
🤖 Prompt for AI Agents
In `@apps/tldraw-sync-worker/wrangler.toml` at line 20, Fix the typo in the
comment within wrangler.toml: change the word "assest" to "assets" in the
comment that reads "# We store room snapshots (assest are stored in native apps)
in an R2 bucket" so it correctly reads "assets".
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
|
|
||
| const isAllowedOrigin = (origin: string): boolean => | ||
| ALLOWED_ORIGINS.includes(origin) || | ||
| ALLOWED_ORIGINS.some((allowedOrigin) => origin.startsWith(allowedOrigin)) || |
There was a problem hiding this comment.
The startsWith check for allowed origins is insecure as it allows any domain that shares the same prefix (e.g., https://roamresearch.com.attacker.com). This misconfiguration permits unauthorized cross-origin access to the sync worker.
Update the check to enforce an exact match or properly validate subdomains (e.g., check if it equals the allowed origin or ends with .{allowedOrigin}).
Spotted by Graphite Agent
Is this helpful? React 👍 or 👎 to let us know.
This comment came from an experimental review—please leave feedback if it was helpful/unhelpful. Learn more about experimental comments here.
- Integrated a visual indicator for Cloudflare sync status in the TldrawCanvasShared component. - Updated props to include `isCloudflareSync` for both Roam and Cloudflare canvas components. - Enhanced user experience by displaying a tooltip with a cloud icon when Cloudflare sync is enabled.
| schedulePersistToR2: () => void = throttle(async () => { | ||
| if (!this.roomPromise || !this.roomId) return | ||
| const room = await this.getRoom() | ||
|
|
||
| // convert the room to JSON and upload it to R2 | ||
| const snapshot = JSON.stringify(room.getCurrentSnapshot()) | ||
| await this.r2.put(`rooms/${this.roomId}`, snapshot) | ||
| }, 10_000) |
There was a problem hiding this comment.
🔴 lodash.throttle wrapping an async function silently swallows R2 persistence errors
schedulePersistToR2 uses lodash.throttle around an async callback. throttle is not promise-aware — it discards the returned promise, so any rejection from this.r2.put(...) becomes an unhandled promise rejection that is silently lost.
Detailed Explanation
At TldrawDurableObject.ts:194-201, the throttled function is typed as () => void but the inner function is async. When throttle invokes it, the returned Promise is never awaited or caught:
schedulePersistToR2: () => void = throttle(async () => {
// ...
await this.r2.put(`rooms/${this.roomId}`, snapshot) // rejection is lost
}, 10_000)If the R2 put fails (network issue, bucket error, etc.), the error is silently swallowed. The Durable Object believes data was persisted, but it was not. When the DO is eventually evicted from memory, all unpersisted room state is lost permanently.
Impact: Silent data loss — canvas edits that users believe are saved may be dropped if R2 writes fail during the throttle window, with no error logging or retry.
| schedulePersistToR2: () => void = throttle(async () => { | |
| if (!this.roomPromise || !this.roomId) return | |
| const room = await this.getRoom() | |
| // convert the room to JSON and upload it to R2 | |
| const snapshot = JSON.stringify(room.getCurrentSnapshot()) | |
| await this.r2.put(`rooms/${this.roomId}`, snapshot) | |
| }, 10_000) | |
| schedulePersistToR2: () => void = throttle(async () => { | |
| if (!this.roomPromise || !this.roomId) return | |
| try { | |
| const room = await this.getRoom() | |
| const snapshot = JSON.stringify(room.getCurrentSnapshot()) | |
| await this.r2.put(`rooms/${this.roomId}`, snapshot) | |
| } catch (e) { | |
| console.error('Failed to persist room to R2', { roomId: this.roomId, error: e }) | |
| } | |
| }, 10_000) |
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Summary by CodeRabbit
New Features
Chores