fix: O3-5419 Add comprehensive error handling for sessionStorage operations#585
Conversation
…ations - Add try-catch blocks around JSON.parse() in search-history.utils.ts - Implement LRU cache (max 50 items) to prevent unbounded growth - Add QuotaExceededError handling with fallback to 10 items - Validate input data and parsed JSON structure - Limit patient data to 100 per search to prevent storage bloat - Add user notifications via showToast for storage errors - Clear corrupted sessionStorage data automatically - Add JSDoc documentation for error handling behavior Fixes crash when sessionStorage contains corrupted JSON or quota is exceeded. Implements graceful degradation - searches continue working even if history fails.
|
@sanks011 The core error handling is solid, but there are some concerns that need addressing:
|
…builder-app into O3-5419-bug-unhandled-session-storage-errors-crash-search-history-component st ":wq" | clip ; Write-Host "Type :wq and press Enter in the editor to complete the merge"
…console statements - Add error handling to composition.utils.ts (third file with same bug) - Remove all 9 console.log/warn/error statements (codebase has zero console logs) - Keep only user-facing showToast notifications for actionable errors - Silent error recovery for corrupted storage data
067cac6 to
aecd2a0
Compare
@UjjawalPrabhat Thanks for the feedback! I've addressed both concerns:
Changes pushed. Please let me know if any further changes are required. |
…-crash-search-history-component
|
As @UjjawalPrabhat pointed out, src/components/composition/composition.utils.ts also touches sessionStorage and therefore runs the exact same risk of throwing quota errors or reading corrupted JSON. To prevent duplicating these large try/catch, Array.isArray(), and JSON.parse blocks across three different codebases, would it make sense to extract this into an app-level utility? (e.g., creating a safelySetSessionStorage(key, value) and safelyGetSessionStorage(key) helper function). Building a shared utility wrapper would protect the entire app universally and cover composition.utils.ts cleanly without redundant code. |
|
@sanks011 and one more tiny thing also in your addToHistory function, you process inputs with: if (!Array.isArray(patients) || typeof parameters !== 'object') { return false; } Just a quick heads up: in Javascript, typeof null === 'object'. Therefore, if parameters somehow gets accidentally passed as null by an upstream function, it will bypass this check and potentially cause null-reference errors downstream when you serialize it. We can easily plug this leak by strictly adding a null check: if (!Array.isArray(patients) || typeof parameters !== 'object' || parameters === null) { return false; } |
Requirements
Summary
This PR fixes a critical bug where the search history component crashes when
sessionStoragecontains corrupted JSON data or when the storage quota is exceeded.Problem
The search history functionality used
sessionStoragewithout error handling, causing application crashes in production when:Solution
Implemented comprehensive error handling with:
File 1:
src/components/search-history/search-history.utils.tsJSON.parse()operationsFile 2:
src/cohort-builder.utils.tsQuotaExceededErrorhandling with fallback to 10 most recent itemsshowToastfor quota errorsImpact
Testing
Screenshots
Before Fix:
OpenMRS.-.Brave.2026-02-11.13-34-13.mp4
SyntaxError: Expected property name or '}' in JSON at position 1After Fix:
OpenMRS.-.Brave.2026-02-11.13-47-00.mp4
[Cohort Builder] Corrupted history data, resettingRelated Issue
https://issues.openmrs.org/browse/O3-5419
Other
Reproduction Steps (Before Fix)
window.sessionStorage.setItem('openmrsHistory', '{invalid json}');After Fix Behavior
Technical Details