Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: stop client from repeatedly calling endpoint #669

Merged
merged 1 commit into from
Oct 1, 2020
Merged

Conversation

Oxiang
Copy link
Contributor

@Oxiang Oxiang commented Sep 30, 2020

Problem

To stop runaway API calls to /api/stats in development environment on fresh install

Reasons:

  1. server returns wrong values
  2. client having unlimited retries

Steps involved:

  1. opening homepage
  2. in StatisticsSilver.jsx > useEffect runs on mount
  3. loadStats() was called
  4. checks for any NULL values in userCount, linkCount and clickCount.
  5. confirmed that all values are NULL > call /api/stats endpoint
  6. received userCount = 0, linkCount = 0 and clickCount = NULL
  7. apply changes and re-renders
  8. re-rendering triggers useEffect
  9. loadstats() called again
  10. checks that confirmed that clickCount is NULL > call /api/stats/ endpoint
  11. repeat step 6

#659

Solution

To address both the client side and server side issues.

For the client side, we will prevent it from calling the endpoint multiple times.

  • useEffect in StatisticSilver.js will only be called once on mount.

For the server side, we will ensure clickCount returns 0 instead of NaN.

  • The function, getGlobalStatistics, in StatisticsRepository.ts will return 0 for clickCount instead of NaN

- set useEffect to be called only once, under client, in StatisticSilver.jsx
- set default value to be 0 when clickCount returns  NaN, under server, in StatisticRepository.ts
Copy link
Contributor

@LoneRifle LoneRifle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants