Feature/mv3 canvas worker cleanup #4
Conversation
WalkthroughThis release bumps versions from 2.0.0-alpha.18 to 2.0.0-alpha.19 across all packages and introduces pluggable ImageBitmap-to-DataURL processing. The change refactors canvas recording to replace worker-based processing with a flexible processor pattern, supporting both inline and worker-backed implementations with error handling and frame-change optimization. Changes
Sequence Diagram(s)sequenceDiagram
participant User as Recording Flow
participant RecordIndex as packages/rrweb/src/record/index.ts
participant CanvasManager as CanvasManager
participant Processor as ImageBitmapProcessor
participant Worker as Web Worker (optional)
User->>RecordIndex: recordOptions w/ imageBitmapProcessor
RecordIndex->>CanvasManager: Initialize with imageBitmapProcessor
CanvasManager->>Processor: Process ImageBitmap data
alt Inline Processor Path
Processor->>Processor: Convert to DataURL via OffscreenCanvas
Processor->>Processor: Cache transparent frames
Processor-->>CanvasManager: ImageBitmapDataURLWorkerResponse
else Worker-Backed Path
Processor->>Worker: Post ImageBitmapDataURLWorkerParams
Worker->>Worker: Process via createWorkerMessageHandler
Worker-->>Processor: ImageBitmapDataURLWorkerResponse
Processor-->>CanvasManager: Response (or fallback to inline)
end
CanvasManager->>CanvasManager: Handle response with base64 data
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Areas requiring extra attention:
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 2
🧹 Nitpick comments (1)
packages/rrweb/src/record/observers/canvas/canvas-manager.ts (1)
228-301: Close ImageBitmap instances to release GPU memory.After
createImageBitmap(canvas)returns, we never callbitmap.close(). These objects hold GPU-backed resources until explicitly closed, so sampling canvases repeatedly will leak memory and eventually destabilize long-running replays. Please close the bitmap in thefinallyblock once processing (and any fallbacks) finish. (developer.mozilla.org)- try { - const bitmap = await createImageBitmap(canvas); + let bitmap: ImageBitmap | null = null; + try { + bitmap = await createImageBitmap(canvas); let response: ImageBitmapDataURLWorkerResponse = await this.processImageBitmap({ id, bitmap, width: canvas.width, @@ - } catch (error) { + } catch (error) { console.warn('[rrweb] Failed to process canvas snapshot', error); - } finally { + } finally { + if (bitmap) { + bitmap.close(); + } snapshotInProgressMap.set(id, false); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
packages/all/package.json(2 hunks)packages/packer/package.json(2 hunks)packages/plugins/rrweb-plugin-canvas-webrtc-record/package.json(2 hunks)packages/plugins/rrweb-plugin-canvas-webrtc-replay/package.json(2 hunks)packages/plugins/rrweb-plugin-console-record/package.json(2 hunks)packages/plugins/rrweb-plugin-console-replay/package.json(2 hunks)packages/plugins/rrweb-plugin-sequential-id-record/package.json(2 hunks)packages/plugins/rrweb-plugin-sequential-id-replay/package.json(2 hunks)packages/record/package.json(2 hunks)packages/replay/package.json(2 hunks)packages/rrdom-nodejs/package.json(2 hunks)packages/rrdom/package.json(3 hunks)packages/rrvideo/package.json(3 hunks)packages/rrweb-player/package.json(2 hunks)packages/rrweb-snapshot/package.json(2 hunks)packages/rrweb/package.json(2 hunks)packages/rrweb/src/index.ts(1 hunks)packages/rrweb/src/record/index.ts(2 hunks)packages/rrweb/src/record/observers/canvas/canvas-manager.ts(6 hunks)packages/rrweb/src/record/workers/image-bitmap-data-url-processor.ts(1 hunks)packages/rrweb/src/record/workers/image-bitmap-data-url-worker.ts(1 hunks)packages/rrweb/src/replay/index.ts(1 hunks)packages/rrweb/src/types.ts(2 hunks)packages/types/package.json(1 hunks)packages/types/src/index.ts(3 hunks)packages/utils/package.json(1 hunks)packages/web-extension/package.json(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
packages/rrweb/src/types.ts (1)
packages/types/src/index.ts (1)
ImageBitmapDataURLProcessor(537-539)
packages/rrweb/src/record/observers/canvas/canvas-manager.ts (4)
packages/types/src/index.ts (3)
ImageBitmapDataURLProcessor(537-539)blockClass(184-184)ImageBitmapDataURLWorkerResponse(524-535)packages/rrweb/src/record/workers/image-bitmap-data-url-processor.ts (1)
createInlineImageBitmapProcessor(111-113)packages/rrweb/src/utils.ts (1)
isBlocked(206-236)packages/rrweb-snapshot/src/types.ts (1)
ICanvas(24-26)
packages/rrweb/src/record/workers/image-bitmap-data-url-processor.ts (2)
packages/types/src/index.ts (5)
DataURLOptions(853-856)ImageBitmapDataURLWorkerParams(516-522)ImageBitmapDataURLWorkerResponse(524-535)ImageBitmapDataURLProcessor(537-539)event(172-172)packages/rrweb/src/record/workers/image-bitmap-data-url-worker.ts (1)
ImageBitmapDataURLRequestWorker(7-31)
packages/rrweb/src/record/workers/image-bitmap-data-url-worker.ts (2)
packages/types/src/index.ts (3)
ImageBitmapDataURLWorkerParams(516-522)ImageBitmapDataURLWorkerResponse(524-535)event(172-172)packages/rrweb/src/record/workers/image-bitmap-data-url-processor.ts (1)
createWorkerMessageHandler(235-247)
🪛 ESLint
packages/rrweb/src/record/workers/image-bitmap-data-url-processor.ts
[error] 23-23: OffscreenCanvas is not supported in Safari 17.4, iOS Safari 15.6-15.8
(compat/compat)
[error] 51-51: OffscreenCanvas is not supported in Safari 17.4, iOS Safari 15.6-15.8
(compat/compat)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Tests
- GitHub Check: ESLint Check and Report Upload
🔇 Additional comments (18)
packages/plugins/rrweb-plugin-canvas-webrtc-record/package.json (1)
3-3: Consistent version bump to alpha.19 across package and dependencies.All three version references have been updated consistently as part of the coordinated release. The changes align with the PR objectives and maintain consistency between the main package version and its dependency versions.
Also applies to: 46-46, 52-52
packages/plugins/rrweb-plugin-canvas-webrtc-replay/package.json (1)
3-3: Version bump is consistent and correct.All three version references (package version, devDependencies, and peerDependencies) are properly synchronized to
2.0.0-alpha.19. The semver constraint syntax is valid.Also applies to: 46-46, 52-52
packages/record/package.json (1)
3-3: Coordinated version bump and dependency updates look correct.All dependencies are updated consistently to alpha.19 with caret constraints allowing compatible versions.
Also applies to: 58-60
packages/utils/package.json (1)
3-3: Version bump is consistent with the coordinated release.packages/rrdom/package.json (1)
3-3: Version and dependency updates are consistent with the coordinated release.Also applies to: 44-44, 55-55
packages/plugins/rrweb-plugin-sequential-id-record/package.json (1)
3-3: Version and peer dependency updates are correctly aligned.Both devDependency and peerDependency for @junify-app/rrweb are updated to the same version, maintaining the peer dependency contract.
Also applies to: 46-46, 52-52
packages/types/package.json (1)
3-3: Version bump is consistent with the coordinated release.packages/rrweb/src/replay/index.ts (1)
520-526: Logic-preserving refactor of the play condition looks correct.The single-line condition
this.config.useVirtualDom && Math.abs(timeOffset - currentTime) > 0correctly preserves the intent: disable virtual-dom fast-forward optimization only when seeking to a different time offset.packages/rrvideo/package.json (1)
3-3: Version and dependency updates are consistent with the coordinated release.Also applies to: 24-24, 33-33
packages/rrdom-nodejs/package.json (1)
3-3: Version and dependency updates are correctly coordinated.Both @junify-app/rrdom and @junify-app/types are updated to alpha.19, ensuring consistency with the new ImageBitmapDataURLProcessor features.
Also applies to: 55-56
packages/rrweb-snapshot/package.json (1)
3-3: LGTM! Version bump coordinated across the monorepo.The version and dependency updates are consistent with the PR's alpha.19 release.
Also applies to: 57-58
packages/all/package.json (1)
3-3: LGTM! Dependency versions properly aligned.All dependencies correctly updated to alpha.19.
Also applies to: 59-61
packages/rrweb/src/record/index.ts (1)
88-88: LGTM! New processor option properly threaded through.The
imageBitmapProcessoroption is correctly extracted fromrecordOptionsand passed toCanvasManager, enabling the new pluggable ImageBitmap processing pattern.Also applies to: 329-329
packages/replay/package.json (1)
3-3: LGTM! Version and dependencies updated consistently.Also applies to: 59-60
packages/plugins/rrweb-plugin-console-record/package.json (1)
3-3: LGTM! Plugin dependencies properly aligned with alpha.19.Also applies to: 48-48, 56-57
packages/rrweb/package.json (1)
3-3: LGTM! Core package and all dependencies updated to alpha.19.Also applies to: 83-86
packages/rrweb/src/types.ts (1)
41-41: LGTM! New processor option properly typed.The
ImageBitmapDataURLProcessortype is correctly imported and added as an optional property torecordOptions, enabling users to provide custom ImageBitmap processing implementations while maintaining backward compatibility.Also applies to: 78-78
packages/packer/package.json (1)
3-3: LGTM! Version and type dependency updated consistently.Also applies to: 80-80
| const id = `${width}-${height}`; | ||
| if (!('OffscreenCanvas' in globalThis)) { | ||
| return ''; | ||
| } | ||
| if (transparentBlobMap.has(id)) return transparentBlobMap.get(id)!; | ||
| const offscreen = new OffscreenCanvas(width, height); | ||
| // Creating the rendering context ensures convertToBlob runs correctly. | ||
| offscreen.getContext('2d'); | ||
| const blob = await offscreen.convertToBlob(dataURLOptions); | ||
| const arrayBuffer = await blob.arrayBuffer(); | ||
| const base64 = encode(arrayBuffer); | ||
| transparentBlobMap.set(id, base64); | ||
| return base64; |
There was a problem hiding this comment.
Include dataURL options in the transparent cache key
transparentBlobMap currently keys only on width-height. If two canvases share dimensions but request different dataURLOptions (different mime/quality), the first entry wins and later lookups reuse the wrong transparent base64. That breaks the “skip the first blank frame” optimisation for the later canvas and causes an unnecessary full frame emit. Fold the relevant option fields into the cache key so each distinct encoding gets its own entry.
- const id = `${width}-${height}`;
+ const typeKey = dataURLOptions?.type ?? '';
+ const qualityKey =
+ dataURLOptions && typeof dataURLOptions.quality === 'number'
+ ? String(dataURLOptions.quality)
+ : '';
+ const cacheKey = `${width}-${height}-${typeKey}-${qualityKey}`;
if (!('OffscreenCanvas' in globalThis)) {
return '';
}
- if (transparentBlobMap.has(id)) return transparentBlobMap.get(id)!;
+ if (transparentBlobMap.has(cacheKey)) {
+ return transparentBlobMap.get(cacheKey)!;
+ }
const offscreen = new OffscreenCanvas(width, height);
// Creating the rendering context ensures convertToBlob runs correctly.
offscreen.getContext('2d');
const blob = await offscreen.convertToBlob(dataURLOptions);
const arrayBuffer = await blob.arrayBuffer();
const base64 = encode(arrayBuffer);
- transparentBlobMap.set(id, base64);
+ transparentBlobMap.set(cacheKey, base64);
return base64;📝 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.
| const id = `${width}-${height}`; | |
| if (!('OffscreenCanvas' in globalThis)) { | |
| return ''; | |
| } | |
| if (transparentBlobMap.has(id)) return transparentBlobMap.get(id)!; | |
| const offscreen = new OffscreenCanvas(width, height); | |
| // Creating the rendering context ensures convertToBlob runs correctly. | |
| offscreen.getContext('2d'); | |
| const blob = await offscreen.convertToBlob(dataURLOptions); | |
| const arrayBuffer = await blob.arrayBuffer(); | |
| const base64 = encode(arrayBuffer); | |
| transparentBlobMap.set(id, base64); | |
| return base64; | |
| const typeKey = dataURLOptions?.type ?? ''; | |
| const qualityKey = | |
| dataURLOptions && typeof dataURLOptions.quality === 'number' | |
| ? String(dataURLOptions.quality) | |
| : ''; | |
| const cacheKey = `${width}-${height}-${typeKey}-${qualityKey}`; | |
| if (!('OffscreenCanvas' in globalThis)) { | |
| return ''; | |
| } | |
| if (transparentBlobMap.has(cacheKey)) { | |
| return transparentBlobMap.get(cacheKey)!; | |
| } | |
| const offscreen = new OffscreenCanvas(width, height); | |
| // Creating the rendering context ensures convertToBlob runs correctly. | |
| offscreen.getContext('2d'); | |
| const blob = await offscreen.convertToBlob(dataURLOptions); | |
| const arrayBuffer = await blob.arrayBuffer(); | |
| const base64 = encode(arrayBuffer); | |
| transparentBlobMap.set(cacheKey, base64); | |
| return base64; |
🧰 Tools
🪛 ESLint
[error] 23-23: OffscreenCanvas is not supported in Safari 17.4, iOS Safari 15.6-15.8
(compat/compat)
🤖 Prompt for AI Agents
In packages/rrweb/src/record/workers/image-bitmap-data-url-processor.ts around
lines 18 to 30, the transparentBlobMap key only uses width-height which causes
incorrect cache hits when different dataURLOptions (e.g., mime or quality) are
requested; include the relevant dataURLOptions in the cache key (for example
mime and quality or a stable serialization of the full dataURLOptions object) so
entries are unique per encoding, ensure you serialize deterministically (sort
keys or use a stable serializer) and handle undefined/null option fields
consistently when constructing the id before checking/setting the map.
| return new Promise<ImageBitmapDataURLWorkerResponse>((resolve, reject) => { | ||
| const queue = pending.get(params.id); | ||
| if (queue) { | ||
| queue.push({ resolve, reject }); | ||
| } else { | ||
| pending.set(params.id, [{ resolve, reject }]); | ||
| } | ||
|
|
||
| try { | ||
| worker.postMessage(params, [params.bitmap]); | ||
| } catch (error) { | ||
| const existing = pending.get(params.id); | ||
| if (existing) { | ||
| existing.pop(); | ||
| if (existing.length === 0) { | ||
| pending.delete(params.id); | ||
| } | ||
| } | ||
| workerFailed = true; | ||
| onError(error); | ||
| fallbackProcessor(params) | ||
| .then((response) => { | ||
| resolve(response); | ||
| }) | ||
| .catch((fallbackError) => { | ||
| reject(fallbackError); | ||
| }); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Failing worker leaves older requests hanging
When worker.postMessage throws you mark workerFailed = true, remove only the newest resolver, and fall back for that one request. Any earlier requests still sitting in pending keep their promises unresolved because handleMessage now bails out once it sees workerFailed, so their responses are ignored forever. This is a hard hang for callers waiting on those promises. Please flush the remainder of the queue (e.g. call rejectAll(error) before returning to the fallback) so every pending request either resolves or rejects deterministically.
} catch (error) {
const existing = pending.get(params.id);
if (existing) {
existing.pop();
if (existing.length === 0) {
pending.delete(params.id);
}
}
workerFailed = true;
+ rejectAll(error);
onError(error);
fallbackProcessor(params)
.then((response) => {
resolve(response);
})Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/rrweb/src/record/workers/image-bitmap-data-url-processor.ts around
lines 203-231, when worker.postMessage throws the code only removes the newest
resolver and leaves earlier pending promises unresolved; change the catch path
to flush the entire queue for params.id by rejecting every queued promise with
the thrown error (or a wrapped error) and deleting the map entry, then set
workerFailed = true and call onError(error), and finally run the
fallbackProcessor for the current call and resolve/reject only its promise; this
ensures no requests remain hanging and prevents memory leaks.
Summary by CodeRabbit
New Features
Improvements
Chores