fix(storage): prevent project data loss with backup and auto-save (#2… by parthdagia05 · Pull Request #6539 · sugarlabs/musicblocks

parthdagia05 · 2026-04-10T23:20:53Z

Summary

Students have reported losing all their locally saved projects after a browser crash or page refresh. The root cause is that all project data is stored under a single key (ProjectData) in IndexedDB via localforage. If that entry gets corrupted during a crash, restore() silently returns null, and initialiseStorage() overwrites everything with an empty object permanently destroying all projects with no way to recover.

This PR addresses the problem with four targeted changes to ProjectStorage.js:

Changes

Backup before write
Every call to save() now persists a copy of the current stored data under a separate ProjectData_backup key before overwriting the primary. This ensures a known-good snapshot always exists.
Corruption recovery in restore()
If the primary data fails to parse (corrupted JSON), the system now automatically attempts to restore from the backup key. Only if both primary and backup are unavailable does it initialize a fresh empty state.
Error handling and concurrency guard in save()
save() is wrapped in try/catch so a storage failure (quota exceeded, disk error) no longer propagates as an unhandled exception that could crash the app.
A _saveInProgress flag prevents overlapping writes that could cause race conditions.
Periodic auto-save (60s interval)
A setInterval timer calls save() every 60 seconds, so if the browser crashes between user-triggered saves, at most one minute of work is lost instead of everything.
Safe initialization
initialiseStorage() now skips calling save() when data was null/corrupted, preventing it from overwriting the backup with empty data during recovery.

How it was tested

Added 43 unit tests covering:
- save/restore
- backup creation
- corruption recovery
- concurrent save guard
- auto-save lifecycle
- CRUD operations
- two end-to-end data-loss-and-recovery scenarios
Ran the full test suite (4232 tests pass, no regressions introduced)
Verified with integration tests that simulate the exact Student Loses Project Twice #2994 scenario:
- corrupt primary storage → refresh → project recovered from backup

Category

-[X] Bug Fix
-[X] Tests

github-actions · 2026-04-10T23:22:02Z

❌ Some Jest tests failed. Please check the logs and fix the issues before merging.

Failed Tests:

help.test.js

parthdagia05 · 2026-04-10T23:37:15Z

@walterbender Can you review this PR

parthdagia05 · 2026-04-11T08:59:20Z

@Ashutoshx7

parthdagia05 · 2026-04-11T08:59:41Z

❌ Some Jest tests failed. Please check the logs and fix the issues before merging.

Failed Tests:
help.test.js

could you also check this test which is failing on the main branch

Ashutoshx7 · 2026-04-11T09:02:15Z

hy @parthdagia05 sorry can't review on my own i am in collage
but i asked ai to review this and then i verified what he said ( i don't agree with the solution he said but the problem is real
and also help.test.js is problem on the master
nothing from your side
pr_6539_review.md

github-actions · 2026-04-11T11:17:10Z

❌ Some Jest tests failed. Please check the logs and fix the issues before merging.

Failed Tests:

help.test.js

parthdagia05 · 2026-04-11T11:17:49Z

made a little changes, could you check it out again ?

hy @parthdagia05 sorry can't review on my own i am in collage but i asked ai to review this and then i verified what he said ( i don't agree with the solution he said but the problem is real and also help.test.js is problem on the master nothing from your side pr_6539_review.md

walterbender · 2026-04-11T14:02:36Z

            pitchDuration = toFraction(pitchDuration);
            const adjustedNote = _adjustPitch(pitch.name, keySignature).toUpperCase();
-            if (triplet != null) {
+            if (triplet !== null && triplet !== undefined) {


Please revert this change.

walterbender · 2026-04-11T14:04:35Z

+                } catch (e) {
+                    console.error("[AutoSave] Failed:", e);
+                }
+            }, 60000);


Maybe every 5 minutes? One reason we never did this in the past was because we didn't want to interrupt playback. So maybe defer is the project is actively running? (Esp. because we'll have saved right before running.)

Changed from 60s to 5 minutes (5 * 60 * 1000). Added a guard: if (this.logo && this.logo._alreadyRunning) return; so auto-save is skipped during playback.

walterbender · 2026-04-11T14:05:34Z

        //  prevent unnecessary crashing

-        if (savedjsonobj == null) throw new Error("Failed to save project data");
+        if (savedjsonobj === null || savedjsonobj === undefined)


does == null cover both cases?

does == null covers both the cases reverting the changes

walterbender · 2026-04-11T14:06:31Z

In general, this looks very good.

github-actions · 2026-04-11T14:33:53Z

✅ All Jest tests passed! This PR is ready to merge.

…to-save (sugarlabs#2994) Addresses the root causes of student project data loss: - Add backup-on-save: persists a copy under a separate key before each write - Add corruption recovery: restore() falls back to backup when primary is corrupt - Add error handling in save() with concurrency guard to prevent race conditions - Prevent initialiseStorage() from overwriting backup when data is null - Add 60-second auto-save in activity.js that captures live workspace blocks via prepareExport() + saveLocally(), not just a storage-layer flush - Clean up auto-save interval on beforeunload - Fix pre-existing eslint eqeqeq warnings in touched files - Add module.exports for testability - Add 43 comprehensive tests covering all storage protection mechanisms Closes sugarlabs#2994

github-actions · 2026-04-11T14:39:10Z

✅ All Jest tests passed! This PR is ready to merge.

parthdagia05 · 2026-04-11T14:41:37Z

@walterbender made the changes could you recheck

github-actions Bot added bug fix Fixes a bug or incorrect behavior tests Adds or updates test coverage size/XL Extra large: 500-999 lines changed area/tests Changes to test files labels Apr 10, 2026

parthdagia05 force-pushed the fix/project-data-loss-prevention-2994 branch from 706e569 to 5e0db6b Compare April 11, 2026 11:16

github-actions Bot added the area/javascript Changes to JS source files label Apr 11, 2026

walterbender reviewed Apr 11, 2026

View reviewed changes

parthdagia05 force-pushed the fix/project-data-loss-prevention-2994 branch from 5e0db6b to 0586623 Compare April 11, 2026 14:32

parthdagia05 force-pushed the fix/project-data-loss-prevention-2994 branch from 0586623 to 80f5a70 Compare April 11, 2026 14:38

walterbender merged commit 9f7dd5d into sugarlabs:master Apr 11, 2026
9 checks passed

Uh oh!

Conversation

parthdagia05 commented Apr 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

How it was tested

Category

Uh oh!

github-actions Bot commented Apr 10, 2026

Uh oh!

parthdagia05 commented Apr 10, 2026

Uh oh!

parthdagia05 commented Apr 11, 2026

Uh oh!

parthdagia05 commented Apr 11, 2026

Uh oh!

Ashutoshx7 commented Apr 11, 2026

Uh oh!

github-actions Bot commented Apr 11, 2026

Uh oh!

parthdagia05 commented Apr 11, 2026

Uh oh!

walterbender Apr 11, 2026

Choose a reason for hiding this comment

Uh oh!

parthdagia05 Apr 11, 2026

Choose a reason for hiding this comment

Uh oh!

walterbender Apr 11, 2026

Choose a reason for hiding this comment

Uh oh!

parthdagia05 Apr 11, 2026

Choose a reason for hiding this comment

Uh oh!

walterbender Apr 11, 2026

Choose a reason for hiding this comment

Uh oh!

parthdagia05 Apr 11, 2026

Choose a reason for hiding this comment

Uh oh!

walterbender commented Apr 11, 2026

Uh oh!

github-actions Bot commented Apr 11, 2026

Uh oh!

github-actions Bot commented Apr 11, 2026

Uh oh!

parthdagia05 commented Apr 11, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

parthdagia05 commented Apr 10, 2026 •

edited

Loading