unbridled-41 opened a new pull request, #5121:
URL: https://github.com/apache/rocketmq-dashboard/pull/5121
### Issue linkage
No existing issue or open PR covers this path. `gh search prs --repo
apache/rocketmq-dashboard "subscriptionLoadingByGroup"` returns nothing, and
the 211 open PR bodies contain no reference to it. The nearest artifacts are
the loading-flag fixes on other pages (#4984 ACL + system alerts, #4983 metrics
profile) and #4968 (mine), which introduced the id guard this PR completes.
This is a regression from #4968, which fixed #4961 (stale subscription
responses overwriting newer diagnostic results).
### Problem / Evidence
`loadSubscriptions` (`web/src/pages/instance/consumer.tsx:392-421`) raised
the per-group loading flag for a user-visible check and cleared it only when
the finishing request was still current **and** non-silent:
```ts
if (subscriptionRequestIdRef.current[cacheKey] === requestId && !silent) {
setSubscriptionLoadingByGroup((prev) => ({ ...prev, [cacheKey]: false }));
}
```
The detail modal refreshes the same data silently every 2s (`:459-476`,
`await loadSubscriptions(groupName, true, true)`), so a tick that lands while
the user-visible check is still in flight takes over the current-request slot.
After that, the older request's `finally` is skipped by the id check and the
silent request's by the `!silent` guard — in **either** resolution order.
`subscriptionLoadingByGroup[cacheKey]` then stays `true` forever and the
订阅一致性检查 panel keeps its spinner (`:1817`) and can keep the alert at "正在检查订阅一致性"
(`:1841-1843`) until the operator triggers a non-silent check again.
Observed **before** the fix — the new test, run against the pre-fix
component:
```text
× stops the subscription check spinner when a silent refresh supersedes it
AssertionError: expected ... <button class="ant-btn ... ant-btn-loading">
not to have class "ant-btn-loading"
Test Files 1 failed (1)
Tests 1 failed | 39 skipped (40)
```
Observed **after** the fix: the same test passes;
`subscriptionLoadingByGroup` is cleared once the newest request for that group
finishes.
### Root cause / Fix
The flag belongs to whichever request is current, not to the request that
raised it. Let the current request clear it regardless of how it was started,
and leave the state object untouched when the flag is already down so no extra
render happens.
### Priority / fix confidence
- PRIORITY = **63** — impact 22/40 (a permanently spinning diagnostic panel
that hides whether the check finished), scope 13/20 (consumer group diagnostics
modal), reproducibility 18/20 (deterministic test: modal open + one 2s tick),
maintenance value 10/20 (completes the staleness guard from #4968, whose
silent-refresh semantics this restores).
- FIX_CONFIDENCE = **88** — the failure is fully explained by the two
guards, and the fix only removes the condition that made the newest request
unable to clear its own flag.
### Tests
| Command | Result |
|---|---|
| `npx vitest run src/pages/instance/__tests__/ConsumerPage.test.tsx -t
'stops the subscription check spinner when a silent refresh supersedes it'` | 1
failed before the change, passes after |
| `npx vitest run src/pages/instance/__tests__/ConsumerPage.test.tsx
src/pages/instance/__tests__/ConsumerPageDiagnosticsRace.test.tsx` | 41 passed |
| `npx eslint` + `npx prettier --check` + `npx tsc -b` | clean |
| `npx vitest run` (whole web suite, branch) | 145 files, 4 failed; 1362
tests, 8 failed |
| `npx vitest run` (whole web suite, untouched base `4ee173ad`, same
machine, run sequentially) | 145 files, **the same 4** failed; 1362 tests, 16
failed |
The whole suite is load-flaky in this environment: the four failing files
are identical on the branch and on the untouched base (`MetricsExplorer`,
`ConsumerPage`, `TopicPage`, `AlertsPage`), every test that fails on the branch
also fails on the base in that comparison run, and the one divergence (`keeps
failed consumer groups selected after a partially successful batch deletion`,
7.1s under whole-suite load) passes on the branch in isolation in 5.95s. Both
runs used `npx vitest run` with no extra flags, one after the other.
New test: `ConsumerPage.test.tsx#stops the subscription check spinner when a
silent refresh supersedes it`. It opens the modal with a deferred subscription
response, lets the modal's 2s auto-refresh issue the silent request, resolves
both and asserts the 重新检查 button drops `ant-btn-loading`.
### Risk
A silent refresh now clears the loading flag when it is the newest request,
so the diagnostic spinner can stop while an older user-triggered request is
still in flight — that older response is by definition stale and is already
ignored by the id guard, and the newer data has landed by then. Nothing else
reads `subscriptionLoadingByGroup`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
