Project Stats: Project Stats numbers (Classifications Today / Classifications Total) don't show on initial Next.js page fetch

[shaunanoordin]

Inconsistent UI Bug?

Package: app-project and/or lib-react-components
Confirmed as of commit:
Also see: #5974 (previous fix for AnimatedNumbers), #5971 (similar issue)
Possibly related Talk issue: https://www.zooniverse.org/talk/17/3402644 ?

In certain cases when the Next.JS server first fetches a page, it's possible for the Project Stats ("Classifications Today" and "Classifications Total" count on the Classify page) to not display accurate numbers, despite fetching the correct data.

Replicating this issue is a faff, but can be done on localhost:

• Tested using macOS + Chrome 126 non-Incognito, targeting app-project, with a logged-in session saved. (i.e. when I open the target URL, I'm already logged in with an account that has already made 5+ classifications today.)
• Start a FRESH instance of the local dev server (yarn dev)
• Open a new window in Chrome.
• Open target URL: https://local.zooniverse.org:3000/projects/fulsdavid/the-daily-minor-planet/classify/workflow/22354?env=production (Daily Minor Planets)
  • A hydration warning should appear.
  • Scroll down to the Project Stats; it should display inconsistent numbers (see screenshot below)
• Refresh the page. Project stats should be normal.

Screenshot: Project Stats show 0 Classifications Today and 0 Classifications Total, but the bar chart indicates otherwise.

Extended Testing

Asides from the issue seemingly stemming from the "initial Next.JS page fetch", additional testing shows that the Project Stats otherwise works fine.

• Tested with macOS + Chrome 126 on https://www.zooniverse.org/projects/fulsdavid/the-daily-minor-planet/classify/workflow/22354 and local (app-project, master)
• I'm able to load the FEM Classify page (while already logged in) and see the initial Classifications Today and Total counts.
  While logged in, I can make classifications, and both the Classifications Today and Total counts update as expected. (About 5 classifications per test)
• I can log out, and the counts will reset to 0
• I can login to the same account, or to a different account, and the Classifications Today and Total counts update as expected.

Dev Notes

Thoughts 1: I don't have enough expertise on the Next.JS infrastructure to understand why/how the fetch process would be a problem. Is it a hydration thing?

Even if it is, I can't tell if it's anything user-facing (i.e. if it affects the non-dev servers), and how I'd confirm/check that.

Thoughts 2: could it be possible that in certain scenarios, the animation of the <AnimatedNumbers> used in the project stats is interrupted, causing the numbers to "freeze" at 0 (for small counts) or appear to "lag" (for larger counts)?

I'm starting to think if it's worth removing AnimatedNumbers altogether, if it's a potential cause of problems.

Thoughts 3: this may issue or may not be related to the Talk post that initiated this investigation, since that volunteer said that despite refreshing their page, the lag still persisted.

But since then it has started to fall behind the 'classifications today' counter, until it was lagging behind by 55 classifications. I tried refreshing the page, clearing my cache, waiting for an hour and refreshing again, none of which made any difference

But then again:

Logging out of my account and back in again did seem to do something, though - the total classifications counter is now off by just two classifications. Better, but still not quite what I wanted.

Status

Unknown priority. Impact level unclear, but some volunteers are understandably upset when this occurs. Replicability/consistency of the issue is dubious since it relies on some Next.JS infrastructure shenanigans that I can only consistently replicate on my localhost.

UPDATE (1 Aug): now set to low priority.

[eatyourgreens]

If it helps, classification stats aren’t fetched server-side. They’re fetched in the browser, after you’ve been logged in.

[shaunanoordin]

Update

I'm filing this investigation under low priority for now, as the original volunteer has informed us that they couldn't replicate the problem at the moment. I'll still keep this issue open until we know for sure what's been happening.

Also, of interest, the volunteer also added something else that may either be related to this issue, or is tangential to it:

PS: I have come across the inconsistent UI problem as well, for what it's worth, although for me it usually manifests slightly differently: when I start a fresh session after booting my computer, both stats counters will initially be at 0, with the bar chart for the preceding days also incorrectly at 0. The problem mostly goes away after doing a few classifications and refreshing the page, although occasionally I need to open the classify page in a new tab for the bar chart to be displayed correctly. I have always assumed that this is connected to the fact that I normally stay logged into my zooniverse account.

[eatyourgreens]

It might also be worth noting that user project classification counts are cached in session storage, to prevent them being lost when you go to Talk.

#5134 (comment)

[eatyourgreens]

@shaunanoordin I can reproduce this bug on the Zooniverse site:

1. Open a Classify page in a tab and do some classifications:
   https://www.zooniverse.org/projects/fulsdavid/the-daily-minor-planet/classify/workflow/22354

2. Open the same page in a new tab. Today's total is correct but total project classifications are now 0:

The project user snapshot, which includes personal stats, is saved to session storage, to preserve it when you visit Talk, or your inbox, or recents, or collections. Session storage isn't shared across tabs. You could maybe change that to use local storage, which is shared across tabs. I'm not sure what the other side effects of that might be. For example, if you have two tabs, each on a different project, then the user snapshots for each of those tabs will be different and you wouldn't want the snapshot from project A to be loaded into the Classify page for project B (see #5134 and #5964.)

[eatyourgreens]

I’m not 100% sure this is relevant, but each tab has its own, independent Panoptes session. See #4892 and zooniverse/panoptes-javascript-client#207.

So, in theory, the new tab should log you in, with your secure Panoptes cookie, then request your classification count from the API. I think there might be a bug in the code that makes that request, which is in the user store. TBH, making network requests from a MobX store isn’t a recommended pattern so that code probably needs to be replaced.

EDIT: after looking at the AnimatedNumber component, I realised the bug was actually missing dependencies for a useEffect hook, plus an animation function that will animate even if the initial and final values are both 0. Data-fetching seems to be working as expected. However, if data-fetching from the stats API fails, I don't think that error is communicated to a volunteer. The bug has nothing to do with Next.js.

The next few comments are me trying to figure out where AnimatedNumber is failing. You can skip forward to this comment, #6190 (comment), if you want to know how to reliably reproduce the bug.

Skip forward to this comment, #6190 (comment), if you want to get straight to an explanation of the bug. It's to do with closures and stale variable scopes being captured.

[eatyourgreens]

Here's the action that fetches your personal stats, when the React app mounts in the browser. If newUser is false, then daily counts should have been loaded from a user snapshot in browser session storage.

front-end-monorepo/packages/app-project/stores/User/UserPersonalization/UserPersonalization.js

Lines 75 to 83 in 094daf5

	load(newUser = true) {
	self.notifications.fetchAndSubscribe()
	if (newUser) {
	self.stats.fetchDailyCounts()
	self.projectPreferences.fetchResource()
	} else {
	self.projectPreferences.refreshSettings()
	}
	},

[eatyourgreens]

I just refeshed my browser tab from one hour ago. Project stats have changed, but today's 4 classifications are missing from the overall total. The correct numbers should be; 4 classifications today, 50 classifications overall.

I think what's going here is the bug that I described in #5134 (comment), where session storage loads stale classification counts. The 4 classifications that I made today were all made in a different tab, so they haven't been added to the total count cached in browser storage in this tab.

If mutabilitie is using long-running sessions in a single tab, refreshing that tab to log back in, then a combination of #5134 and zooniverse/panoptes-javascript-client#207 could explain why she's seeing stale classification counts.

[eatyourgreens]

I added console logs for the counts prop, and ran a production build of Daily Minor Planet on local.zooniverse.org. The counts prop updates as expected, so counts are being fetched properly from the API, but the YourStats component doesn't reflect changes to counts.total. counts.today does update when it changes.

[eatyourgreens]

Logging the props for the Stat component shows that the displayed value doesn't match the prop either.

[eatyourgreens]

The AnimatedNumber component is being passed a value of 51, but showing a value of 0. That's the same issue I saw in #5971. I find that the bug triggers if I load my classification counts offscreen, so the bug might be in the intersection observer that animates the number.

Screen.Recording.2024-08-02.at.23.19.51.mov

Screen.Recording.2024-08-02.at.23.21.21.mov

[eatyourgreens]

If I load my stats offscreen, animated is always false in the AnimatedNumber component, when the value first changes from 0 to 5.

When animated is false, AnimatedNumber shows 0, so maybe there's a bug here? Should initialValue here store the previous value of the animated number, rather than 0?

front-end-monorepo/packages/lib-react-components/src/AnimatedNumber/AnimatedNumber.js

Line 80 in 094daf5

return <span ref={numRef}>{!animated ? initialValue : formatValue(value)}</span>

[eatyourgreens]

NB. that code also means that project stats are always rendered as 0 during SSR, even though they've been set. Here are the Node console logs from that same page, running locally.

animated is always false in Node, so total Classifications, Volunteers, and Completed Subjects will all be rendered as 0.

counts: { today: 0, total: 0 }
Stat: test-classify-your-stats-today-count Classifications today 0
{ animated: false, value: 0 }
Stat: test-classify-your-stats-total-count Classifications total 0
{ animated: false, value: 0 }
Stat: undefined Volunteers 6190
{ animated: false, value: 6190 }
Stat: undefined Classifications 810216
{ animated: false, value: 810216 }
Stat: undefined Subjects 71804
{ animated: false, value: 71804 }
Stat: undefined Completed subjects 28715
{ animated: false, value: 28715 }

[eatyourgreens]

Now that I know how to reliably reproduce this bug, here's a screenshot showing the AnimatedNumber component in production, with the value prop highlighted in the React component inspector. value is 51, but the displayed value is 0, which is pretty much the same issue as #5971.

To reproduce this bug:

1. Use a new tab, so that user preferences (including activity_count) aren't already cached in session storage and must be fetched from Panoptes. Alternatively, open session storage and delete the stored user, if present.
2. Make sure the YourStats component is offscreen, so that its displayed value is still 0 after stats have loaded from the API.
3. Load the Classify page for a project that you've classified on today.
4. Wait for user stats to load offscreen.
5. Scroll down to your stats. One will animate to its loaded value, but I find that one will stay stuck at 0 most of the time. Sometimes today's count is stuck at 0, sometimes the total count is stuck at 0.
6. Inspect AnimatedNumber in the React component inspector to see that its value prop doesn’t match the displayed number.

Those steps reliably reproduce the bug for me in desktop Firefox. Mobile Safari seems to be unaffected.

https://www.zooniverse.org/projects/fulsdavid/the-daily-minor-planet/classify/workflow/22354

[eatyourgreens]

I think I see a bug. The effect hook here captures the animateValue closure when value is 0. It doesn’t update when animateValue changes, so I think it’s always going to animate to 0, even though a new animateValue closure is generated on every render. I think the effect hook executes a stale reference to an older value of animateValue, from a previous render.

The linter should really be warning about missing useEffect dependencies here. I’d also be careful about generating callback functions on every render, like you’re doing here, since it’s really easy to accidentally capture a stale variable scope inside a closure. If possible, move your callbacks outside the render function, or declare them inside the useEffect hook.

front-end-monorepo/packages/lib-react-components/src/AnimatedNumber/AnimatedNumber.js

Line 65 in 094daf5

animateValue()

[eatyourgreens]

I've opened #6192 to fix the useEffect hook, so that the animated value always uses the latest version of the value prop.

I’ve also added a check so that it won’t animate from 0 to 0, then set animated to true, while the value is still loading from an external API.

[eatyourgreens]

@shaunanoordin this isn't particularly exhaustive testing, but I'm seeing stats show up correctly in dev mode, with the changes in #6192. Screen recording from Firefox:

Screen.Recording.2024-08-03.at.17.30.49.mov

[eatyourgreens]

Here’s an explanation of the bug.

1. When the AnimatedNumber component mounts, value is 0 and animated is false. useEffect runs and creates an intersection observer which will animate the displayed number from 0 to 0.
2. value changes to your actual count and a new animateValue function is created, which will animate the span to that value, but useEffect doesn’t run, so the intersection observer doesn’t update to use the new animateValue. It has captured the value of value when it was created, which is 0.
3. When you scroll the number into view, it runs an outdated animateValue from the first render, animates the displayed number to 0, then sets animated to true.

So the component will always show 0, if it loads offscreen and value loads asynchronously. There’s no story or test for the case where value is set asynchronously but the code is written to display the value of value that’s captured when the component mounts.

For a more technical explanation, I recommend MDN’s explanation of lexical scoping in closures. Each time the useEffect hook runs, it's capturing animateValue and lessAnimation from the current render. They, in turn, are capturing the current values of value and duration. TBH, the easiest way to avoid bugs like this is to avoid nested closures. Also, set up ESLint to warn when it detects nested closures, and look out for nested closures as a code smell in PR reviews.

[eatyourgreens]

Here's a story that reproduces the bug. On the main branch, this story always shows 0. I've included it in #6192 so that you can reproduce the bug and check the fix.

I think the bug was introduced in #5974, or possibly #5947. Neither of those PRs included value or duration in the dependencies for useEffect, so the effect hook only captured value on component mount.

export const DeferredAnimation = () => {
  const [value, setValue] = useState(0)
  setTimeout(() => setValue(700000000), 2000)
  return (
    <Box pad={{ vertical: '120vh' }}>
      // this will always display 0, even if value changes.
      <AnimatedNumber duration={4000} value={value} />
    </Box>
  )
}

production-release:

Screen.Recording.2024-08-04.at.17.04.55.mov

PR #6192:

Screen.Recording.2024-08-04.at.17.06.02.mov

[eatyourgreens]

@shaunanoordin this might not be a low priority bug after all, given that volunteer classification counts are always displayed as 0 when they load from Zooniverse APIs. They display fine if they load from session storage, which is why refreshing the page is a workaround.

[eatyourgreens]

For anyone who's made it this far, congratulations, and the title of this issue is wrong. The bug has nothing to to do with Next.js.

[eatyourgreens]

I've posted a link to Talk that reproduces this bug every time you click it:
https://www.zooniverse.org/talk/17/3402644?comment=5592887

[eatyourgreens]

Here's a quick screen recording in Chrome, where I click that link three times and get the wrong counts each time. Weirdly, the daily stats sometimes show up, sometimes show as 0. Total counts are always 0.

Screen.Recording.2024-08-06.at.18.50.11.mov