You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@rocketmq.apache.org by "unbridled-41 (via GitHub)" <gi...@apache.org> on 2026/09/27 01:27:10 UTC
[PR] fix(web): clear the subscription check spinner when a silent refresh wins [rocketmq-dashboard]
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(web): clear the subscription check spinner when a silent refresh wins [rocketmq-dashboard]
Posted by "unbridled-41 (via GitHub)" <gi...@apache.org>.
unbridled-41 commented on PR #5121:
URL: https://github.com/apache/rocketmq-dashboard/pull/5121#issuecomment-5965573422
### CI status: the workflow run for this head ended in `startup_failure` — no checks were produced
```
gh run list --branch <this branch> --limit 1
2026-09-27T01:16:06Z completed startup_failure <head sha>
```
`startup_failure` means GitHub Actions could not start the workflow at all, so this PR has no
check results rather than passing or failing ones — the same period in which the repository's CI
rejected re-runs with "its workflow file may be broken". The verification for this change is
therefore the local one recorded in the Tests table above (red before the fix, green after, plus the
surrounding suite, `tsc`/`eslint`/`prettier`/`checkstyle` as listed).
The three other jobs, when they do run on this repository, are red on every branch including
`rocketmq-studio` itself: `Backend Build` fails in the `binary-license-gate` step (tracked by #5341)
and `Frontend Build` fails in the license-manifest check (tracked by #5346).
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(web): clear the subscription check spinner when a silent refresh wins [rocketmq-dashboard]
Posted by "lizhimins (via GitHub)" <gi...@apache.org>.
lizhimins merged PR #5121:
URL: https://github.com/apache/rocketmq-dashboard/pull/5121
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org