Main page stats failover improvements - #3673
Conversation
tom2drum
left a comment
tom2drum
left a comment
There was a problem hiding this comment.
| ID | Severity | Axis |
|---|---|---|
| F1 | blocker | correctness |
| F2 | blocker | correctness |
| F3 | blocker | correctness |
| F4 | major | correctness |
| F5 | major | correctness |
No spec.md for this task — spec axis skipped.
Not anchorable
HomeRpcDataContextProvider.enable still does setIsEnabled(isEnabled) (unchanged hunk). F3 is anchored on the new subscription ids that make that last-write-wins path concurrent.
|
|
||
| return React.useMemo(() => ({ | ||
| isLoading: (isStatsFeatureEnabled && statsApiQuery.isPlaceholderData) || coreApiQuery.isLoading, | ||
| isError: coreApiQuery.isError && (isStatsFeatureEnabled && statsApiQuery.isError && !statsApiQuery.isRefetchError), |
There was a problem hiding this comment.
F1 · blocker — isError: coreApiQuery.isError && (isStatsFeatureEnabled && statsApiQuery.isError && !statsApiQuery.isRefetchError) is always false when the stats microservice is off, so a core stats failure never trips RPC / mdash failover. Widgets then format missing fields as NaN.
Fix: isError: coreApiQuery.isError && (!isStatsFeatureEnabled || (statsApiQuery.isError && !statsApiQuery.isRefetchError)).
— Reviewed by Cursor Grok 4.6
| const coreApiQuery = useStatsQuery(); | ||
|
|
||
| return React.useMemo(() => ({ | ||
| isLoading: (isStatsFeatureEnabled && statsApiQuery.isPlaceholderData) || coreApiQuery.isLoading, |
There was a problem hiding this comment.
F2 · blocker — isLoading uses coreApiQuery.isLoading, but useStatsQuery sets placeholderData: STATS. In React Query v5 that makes isLoading false while the stub is showing, so placeholder numbers render as live. The stats-microservice branch already uses isPlaceholderData; deleted Stats.tsx used apiQuery.isPlaceholderData for the same reason.
Fix: (isStatsFeatureEnabled && statsApiQuery.isPlaceholderData) || coreApiQuery.isPlaceholderData.
— Reviewed by Cursor Grok 4.6
|
|
||
| export type SubscriptionId = 'latest-blocks' | 'latest-txs' | 'stats-widgets'; | ||
| export type SubscriptionId = 'latest-blocks' | 'latest-txs' | 'stats-widgets' | | ||
| 'stats-widgets-latest-block' | 'stats-widgets-average-block-time' | 'stats-widgets-gas-tracker'; |
There was a problem hiding this comment.
F3 · blocker — New ids 'stats-widgets-latest-block' | 'stats-widgets-average-block-time' | 'stats-widgets-gas-tracker' are meant for independent subscribers, but enable() still setIsEnabled(isEnabled) with no refcount. One widget's effect cleanup sets isEnabled false and stops the RPC watch for the other (and for latest-blocks / latest-txs). 'stats-widgets-gas-tracker' is never passed in.
Fix: derive enabled from remaining subscriptions.length > 0 (or subscribe once from a parent). Drop the unused gas-tracker id.
— Reviewed by Cursor Grok 4.6
| total_blocks: statsApiQuery.data?.total_blocks?.title, | ||
| average_block_time: statsApiQuery.data?.average_block_time?.title, | ||
| total_transactions: statsApiQuery.data?.total_transactions?.title, | ||
| total_operational_transactions: statsApiQuery.data?.op_stack_total_operational_transactions?.title, |
There was a problem hiding this comment.
F4 · major — labels.total_operational_transactions is statsApiQuery.data?.op_stack_total_operational_transactions?.title. Deleted Stats.tsx used total_operational_transactions.title for Arbitrum and the op-stack title only for Optimistic. HomeStatsTotalOperationalTxs reads only labels.total_operational_transactions.
Fix: map total_operational_transactions to statsApiQuery.data?.total_operational_transactions?.title (keep the op-stack key for Optimistic, or pick in the widget by rollup type).
— Reviewed by Cursor Grok 4.6
| if (statsQuery.isError) { | ||
| return mdash; | ||
| } | ||
| return Number(statsQuery.data?.last_output_root_size).toLocaleString(); |
There was a problem hiding this comment.
F5 · major — Number(statsQuery.data?.last_output_root_size).toLocaleString() (and the same Number(undefined) pattern in HomeStatsTotalTxs, HomeStatsTotalAddresses, HomeStatsTotalOperationalTxs, HomeStatsAverageBlockTime, HomeStatsLatestBlock) yields "NaN" / "NaNs" when the field is absent. if (value === undefined) never hits. Deleted Stats.tsx omitted those widgets when the source was missing; HomeStatsBtcLocked already returns null.
Fix: return null when the source value is missing, same as BtcLocked / CurrentEpoch.
— Reviewed by Cursor Grok 4.6
1 participant
Description
Resolves #3655