Skip to content

馃Ч Implement LocalStorage Fallback for Sync Failure #6

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
hashexplaindata wants to merge 1 commit into from
Closed

馃Ч Implement LocalStorage Fallback for Sync Failure #6

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion code/experiment.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -475,7 +475,7 @@ async function executeBatchPayload() {
} catch (error) {
console.error("Critical Sync Failure:", error);
DOM.syncStatus.innerHTML = `<span style="color:#ff453a">鈿狅笍 Sync Failed. Error: ${error.code || 'Network'}</span>`;
// Potential fallback: Save to localStorage for later recovery
localStorage.setItem('failed_sync', JSON.stringify(STATE.results));

@gemini-code-assist gemini-code-assist Bot Mar 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

This implementation risks data loss by overwriting previously saved failed syncs. If a user has a failed sync, and then runs another session that also fails to sync, the data from the first session will be lost. The data should be appended to any existing failed sync data.

Additionally, localStorage.setItem can fail if storage is full or disabled. It's crucial to wrap this in a try...catch block to handle such errors gracefully, especially since this is a data safety feature. My suggestion includes this, along with a user-facing message for this failure case.

        try {
            const FAILED_SYNC_KEY = 'failed_sync_payloads';
            const existingPayloads = JSON.parse(localStorage.getItem(FAILED_SYNC_KEY)) || [];
            existingPayloads.push(STATE.results);
            localStorage.setItem(FAILED_SYNC_KEY, JSON.stringify(existingPayloads));
        } catch (storageError) {
            console.error('Fallback to localStorage failed:', storageError);
            DOM.syncStatus.innerHTML += '<br><span style="color:#ff453a; font-size: 0.8em;">Critical: Could not save a local backup. Data may be lost.</span>';
        }

@chatgpt-codex-connector chatgpt-codex-connector Bot Mar 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve each failed payload under a unique localStorage key

Using a fixed key (failed_sync) means every new sync failure overwrites the previous unsent participant data, so running multiple offline/failed sessions on the same device will silently discard earlier telemetry. If this fallback is meant for recovery, the key should be namespaced (for example by participant ID and timestamp) or appended to a stored queue.

Useful? React with 馃憤聽/ 馃憥.

@chatgpt-codex-connector chatgpt-codex-connector Bot Mar 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Guard localStorage writes inside the sync-failure handler

localStorage.setItem can throw (for example QuotaExceededError or SecurityError in private/restricted browsing), and this call is inside the outer catch without its own guard. In those environments the fallback path itself throws, which can surface as an unhandled rejection and still lose the payload during the exact failure mode this code is trying to protect.

Useful? React with 馃憤聽/ 馃憥.

Copilot AI Mar 12, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

localStorage.setItem(...) can throw (e.g., SecurityError when storage is blocked in iframes/private mode, or QuotaExceededError). Since this is inside the catch for the sync failure, an exception here would mask the original failure and still lose the data. Wrap the fallback write in its own try/catch (and ideally guard on typeof localStorage !== 'undefined') and surface/log a secondary error if persisting locally fails.

Suggested change
localStorage.setItem('failed_sync', JSON.stringify(STATE.results));
// Best-effort local persistence of failed sync; avoid masking original error
if (typeof localStorage !== 'undefined') {
try {
localStorage.setItem('failed_sync', JSON.stringify(STATE.results));
} catch (storageError) {
console.error("Secondary failure: unable to persist failed sync to localStorage:", storageError);
}
} else {
console.warn("localStorage is not available; failed sync data was not persisted locally.");
}

Copilot uses AI. Check for mistakes.

Copilot AI Mar 12, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using a single fixed key (failed_sync) will overwrite any prior failed payloads (including retries or multiple participants on the same device). Consider namespacing the key with STATE.pid and/or a timestamp, and consider clearing the stored payload after a successful sync so the browser doesn鈥檛 retain stale telemetry indefinitely.

Copilot uses AI. Check for mistakes.

Copilot AI Mar 12, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This persists STATE.results (including free-form semantic_justification) to localStorage, which is long-lived and readable by any JS running on this origin (increasing impact of any future XSS). If persistence is required, consider storing the minimum necessary fields and/or using sessionStorage with an explicit user download/export flow, and document/communicate the retention/cleanup behavior.

Suggested change
localStorage.setItem('failed_sync', JSON.stringify(STATE.results));
// Persist a reduced, session-scoped payload without free-form justification
const failedSyncPayload = STATE.results.map(({ semantic_justification, ...rest }) => rest);
sessionStorage.setItem('failed_sync', JSON.stringify(failedSyncPayload));

Copilot uses AI. Check for mistakes.
}
}

Expand Down
Loading