feat: implement safe JSON parsing for localStorage with validation and error handling#330
Conversation
|
@arnavkirti is attempting to deploy a commit to the shopstr-eng Team on Vercel. A member of the Team first needs to authorize it. |
|
Hey will you replace the remaining raw JSON.parse in checkout-card with the new helper, also either add real shape validation for signer or narrow the PR description so it doesn’t claim recovery from invalid shapes there. |
|
@GautamBytes I have replaced the remaining raw JSON.parse and added real shape validation for signer. Please give it a look. |
|
Hey @arnavkirti, just a small suggestion, could you make the description a bit more concise and add some spacing between the points? It already has good formatting, but improving the spacing would make it much easier to read. |
kaihere14
left a comment
There was a problem hiding this comment.
Solid fix for a real crash vector raw JSON.parse on localStorage without a try/catch is a ticking bomb. The
abstraction makes sense.
A few things:
- removeOnError silently deletes the key on any parse failure or validation mismatch. For a cart or discount
key, that's destructive the user loses their data with no warning. At minimum log the error; ideally only
remove on known corruption, not validator mismatches (which could be a schema version mismatch). - parseJsonWithFallback swallows all errors and returns the fallback. If the caller passes a wrong fallback
type (e.g. [] when the data is an object), it silently proceeds with bad state. The generic typing helps but
doesn't fully prevent this at runtime. - The nostr-helper-functions.ts change is a large diff for a "safe JSON" PR worth confirming that the signer
type refactor is actually needed here or if it's scope creep. - getLocalStorageJson does an SSR guard (typeof window === "undefined") but returns the fallback silently.
Fine for now, but if this ever gets called server-side intentionally, it'll be hard to debug.
Tests are good. LGTM with the removeOnError behavior reconsidered.
|
@kaihere14 Thank you for the review! I have addressed it in the latest commits. |
|
LGTM Now!! |
All good now |
Description
Introduce
parseJsonWithFallbackandgetLocalStorageJsoninutils/safe-json.tsto centralize typed, safe JSON parsing forlocalStorage, with optional validation and automatic key cleanup viaremoveOnError.Migrate cart, discount, and Nostr
localStorageusage (includingrelays,mints,tokens,history,signer, etc.) to these helpers so malformed or invalid JSON no longer crashes rendering and instead recovers via validators and safe defaults.Broaden selected
localStorage-backed types where needed while still validating shapes at the boundary, and add unit tests covering valid/malformed JSON, validator failures, key removal, and non-throwing recovery paths.Resolved or fixed issue
none
Affirmation
For more details on what to include, see:
Bug Report Template
Feature Request Template
Documentation Issue Template
Security Issue Template