fix(chrome-extension): fix crash when generating script after stop recording#2172
fix(chrome-extension): fix crash when generating script after stop recording#2172
Conversation
…cording Multiple memory issues caused the extension to crash during script generation after clicking stop: - Remove duplicate persistEventToSession (addEvent already persists) - Debounce IndexedDB writes in addEvent to avoid O(n²) IO - Avoid reloading all sessions from IndexedDB after every updateSession - Limit concurrent AI description calls to 3 to reduce peak memory - Remove full events array (with base64 screenshots) from all logger calls - Delete dead saveEventsToStorage code Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Deploying midscene with
|
| Latest commit: |
5645be6
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://961c4044.midscene.pages.dev |
| Branch Preview URL: | https://fix-chrome-extension-stop-re.midscene.pages.dev |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6e9b7869dd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (state.isRecording) { | ||
| // Debounce IndexedDB writes to avoid O(n²) IO. | ||
| // In-memory state is updated immediately; DB write is batched. | ||
| const sessionId = useRecordingSessionStore.getState().currentSessionId; | ||
| if (sessionId) { | ||
| await dbManager.updateSession(sessionId, { | ||
| events: newEvents, | ||
| updatedAt: Date.now(), | ||
| }); | ||
| scheduleSessionPersist(sessionId, newEvents); | ||
| } |
There was a problem hiding this comment.
recording-events updated during active recording
Because useRecordStore.initialize() still restores in-progress recordings from dbManager.getRecordingEvents() in store.tsx, and Recorder reapplies that recovered data on reopen in extension/recorder/index.tsx, removing the per-event write here breaks the existing crash/reload recovery path. If the popup refreshes or crashes mid-recording, any events captured since the last debounced session flush will reopen as an empty or stale timeline instead of the live recording the user just made.
Useful? React with 👍 / 👎.
| pendingSessionPersist = { sessionId, events }; | ||
| if (sessionPersistTimer) { | ||
| clearTimeout(sessionPersistTimer); | ||
| } | ||
| sessionPersistTimer = setTimeout( |
There was a problem hiding this comment.
pendingSessionPersist now captures the event array exactly as it looked when addEvent ran, but updateEvent and setEvents still persist newer versions of the same session immediately. During recording, optimizeEvent() issues asynchronous updateEvent calls after addEvent (first the loading placeholder, then the final description/screenshot), so the 2s timer can write the older snapshot back to IndexedDB and silently revert those later mutations. Users who reopen the popup or read the session from storage before stopping can therefore see placeholder descriptions or otherwise stale event data.
Useful? React with 👍 / 👎.
Tests cover: - Debounced persistence: rapid writes coalesce into single DB call - Flush behavior: explicit flush writes immediately and clears state - In-memory session update: avoids full DB reload - Batched concurrency: peak concurrent AI calls bounded by limit - Navigation/scroll pre-processing: skips concurrency slots - Logger sanitization: events arrays never passed to loggers Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ests Adds vitest to chrome-extension devDependencies and a "test" script so unit tests can be run via `npx nx test chrome-extension`. Not added to the root `pnpm test` yet because the pre-existing event-memory-stress test is flaky (GC-dependent heap measurements). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2 participants
Summary
Changes
persistEventToSessionduplicatedaddEventpersistenceaddEventalready writes to IndexedDBaddEventwrote full events array to IndexedDB on every event (O(n²))updateSessioncalledgetAllSessions()after every update, reloading all screenshots from IndexedDBgenerateElementDescriptionsran all AI calls in parallel viaPromise.allyamlGenerator.ts,ProgressModal.tsx,useRecordingControl.ts,store.tsxeventsCountsaveEventsToStoragebecame dead code after removing duplicate persistence// const finalEvents = [...]Validation
pnpm run lintpassesTest plan
🤖 Generated with Claude Code