fix: guard NaN in useLeveling XP load and HLC string parsing#908
Open
Anexus5919 wants to merge 1 commit into
Open
fix: guard NaN in useLeveling XP load and HLC string parsing#908Anexus5919 wants to merge 1 commit into
Anexus5919 wants to merge 1 commit into
Conversation
|
@Anexus5919 is attempting to deploy a commit to the somiljain2024-4175's projects Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
Author
|
@Somil450 @diksha78dev Kindly have a review on this pr. Thanks! |
useLeveling read XP from localStorage with parseInt and never checked the result, so a corrupt value made xp NaN, which poisoned the level and progress (calculateLevel returned NaN) and was persisted back as "NaN" permanently. hlcFromString likewise parseInt'd wallTime/counter without guarding, yielding a NaN HLC that breaks comparison/ordering on malformed input. Guard both: fall back to 0 (and level 1) when a parsed value is not a finite number, in the XP load, the rep-based XP add, calculateLevel, and the HLC parser.
Anexus5919
force-pushed
the
fix/nan-guard-leveling-hlc
branch
from
653a8e0 to
870ab73
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📌 Related Issue
Fixes #871
📝 Description
Two unguarded
parseIntpaths could produceNaNthat poisoned downstream values:useLevelingloaded XP from localStorage withparseInt(corrupt value →xp = NaN→calculateLevelreturnsNaN, persisted back as"NaN"permanently), andhlcFromStringparsedwallTime/counterwithout guarding (malformed string →NaNclock that breakscompareHLCordering).🔹 What has been changed?
src/hooks/useLeveling.ts: the XP load falls back to0when the parsed value is not a finite, non-negative number;addXpFromRepsignores a non-finiterepsand treats a non-finite previous XP as0;calculateLevelreturns level1for non-finite input.src/utils/hybridLogicalClock.ts:hlcFromStringfalls back to0forwallTime/counterwhen the parse is not a finite number.src/hooks/__tests__/useLeveling.test.tsandsrc/utils/__tests__/hybridLogicalClock.test.ts.🔹 Why are these changes needed?
NaNthat propagates into the visible level/progress (and persists), or into clock ordering. Falling back to a safe default keeps the values finite.🛠️ Type of Change
🧪 Testing
✅ Tests Performed
npx vitest run src/hooks/__tests__/useLeveling.test.ts src/utils/__tests__/hybridLogicalClock.test.ts-> 5 tests pass.calculateLevel(NaN)returnsNaN;hlcFromString("garbage")returnsNaNfields), and they pass with the guards.npx tsc --noEmit: both files clean;npx eslint: 0 problems.🌐 Browsers Tested
Not applicable (pure logic; verified via unit tests).
📷 Screenshots / Demo (if applicable)
Not applicable.
📋 Checklist
💬 Additional Notes
Branched from latest
upstream/main(5464425). TheuseLevelingXP poisoning is the higher-impact part (it persists); thehlcFromStringguard is defensive against corrupt CRDT data.