Stats: prevent _.merge from crashing reducer. #10326
Merged
+7
−2
Conversation
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
This was referenced Dec 30, 2016
jblz
approved these changes
Dec 30, 2016
There was a problem hiding this comment.
To clarify RE: that screenshot, it looks like the array is actually the size of a UNIX timestamp (🙀) & not the siteId -- and the index containing the 1 is that timestamp minus one.
So, here's what I did to reproduce:
const badArray = new Array(1482340821);
badArray[1482340820] = 1;
indexedDB.open('calypso').onsuccess = (e) => {
const db = e.target.result;
const transaction = db.transaction( 'calypso_store', 'readwrite' );
const store = transaction.objectStore( 'calypso_store' ); store.get( 'redux-state' ).onsuccess = (ee) => {
const state = ee.target.result;
state.stats.lists.items[siteId].statsStreak['[["endDate","2016-12-31"],["gmtOffset",0],["max",3000],["startDate","2015-12-01"]]'].data = badArray;
store.put( state, 'redux-state' );
};
};
Maybe that will help figure out what might have happened vis-a-vis the API.
When I tested this, it took a couple / few times reloading to clear out the bad array from the persisted state & stop crashing, but it does eventually clear up the issue.
I'm going to go ahead and click "Approve" because this is definitely an improvement!
Thanks 🥇
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes #9529
Today @jblz did some troubleshooting with his local state tree that was causing calypso to crash in Chrome. At the end of the troubleshooting, he discovered the issue to be caused by a massive array in the
statsStreakdata that was persisted inindexDB.A screen grab of the malformed data in Jeff's persisted state tree:
Steps to Recreate
With the ability to reproduce the bug locally, I was able to find the crash was happening when
_.mergeis called against the persisted statsStreak data, and the latest data from the API.This
_.mergecrashes the browser tab, and subsequently, the "bad" data is still inindexDb. Additionally, the query params forstatsStreakdata do not change but once per year, as the dates are based upon the calendar year... as such this problem will continually happen until the calypsoindexDBis purged manually, or if the persisted data is too old.The question still remains of how did this malformed array get created in the first place. @jblz tested the api response via the developer console, and in other browsers/incognito tabs and we could not re-create the massive array. I plan on investigating a bit more on the API side of things to see if I can reproduce the issue there.
The solution in this PR is to create a clone of the existing
stats.list.itemsstate tree, and_.unsetthe existing value before calling_.merge. I'm open to other suggestions so feel free to comment with other ideas!To Test
Repeat the steps above locally, and verify the browser tab no longer crashes when using this branch.