You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@rocketmq.apache.org by "zjncs (via GitHub)" <gi...@apache.org> on 2026/09/20 07:35:20 UTC
[PR] fix(proxy): keep the request staleness counter monotonic across effect runs [rocketmq-dashboard]
zjncs opened a new pull request, #4591:
URL: https://github.com/apache/rocketmq-dashboard/pull/4591
### Which Issue(s) This PR Fixes
- Fixes #<issue-id>
### Brief Description
The Proxy page guards async responses with a monotonically increasing request id (`loadRequestId`): every caller captures an id and drops responses whose id no longer matches. However, the load effect's cleanup **rewound** the counter:
```tsx
const requestId = loadRequestId.current;
void loadProxyNodes();
return () => {
loadRequestId.current = requestId + 1;
};
```
Because `loadProxyNodes` itself increments the counter (`++loadRequestId.current`), the cleanup's assignment could move the counter **backwards** to an id that was already issued. After a re-run of the effect (e.g. a language switch, since the callback closes over the translated strings), the new load and an older still-in-flight mutation could both pass the staleness guard, letting the stale response overwrite the fresh list. The same rewind also un-matched the `finally` blocks of `handleAddProxyAddress` / `handleRemoveProxyAddress`, which clear their button spinner only when their captured id still matches — so the add/remove spinner could stay wedged in the loading state.
The fix makes the cleanup advance the counter (`loadRequestId.current += 1`), so ids are never reused and every in-flight request from the previous effect run is invalidated. Additionally, the per-mutation spinner state (`addressMutationLoading`, `removingProxyAddress`) now clears unconditionally in `finally`, because it tracks that mutation's own lifecycle rather than the page's load lifecycle — leaving it tied to the id would wedge the button when the response arrives stale.
### How Did You Test This Change?
New test `does not let a stale add overwrite the list reloaded after a language switch`: it starts an add (deferred), switches the language to re-run the load effect, resolves the fresh load with the post-add list, then resolves the older add with the pre-add list. Before the fix, the stale add overwrote the list (the just-added address disappeared) and the success toast fired; after the fix the list keeps the fresh state and no toast is shown. A test-only `LangSwitcher` inside the same `LangProvider` reproduces the effect re-run that a real language switch triggers.
```
$ NODE_OPTIONS=--no-experimental-webstorage npx --prefix web vitest run --root web src/pages/studio/__tests__/Proxy.test.tsx
Test Files 1 passed (1)
Tests 16 passed (16) # 15 before this change; the new test fails on master's code
$ npx --prefix web eslint --fix web/src/pages/studio/Proxy.tsx web/src/pages/studio/__tests__/Proxy.test.tsx
$ npx --prefix web tsc -p web --noEmit # clean
```
(`NODE_OPTIONS=--no-experimental-webstorage` is only needed on Node ≥ 25, where native `localStorage` shadows jsdom's.) The full web suite (122 files / 1035 tests) was also run; the Proxy file passed, and the handful of unrelated failures were re-run individually and all passed (timeout flakes under heavy machine load, ~15 min per test vs 91 s for the whole suite normally).
### Checklist
- [x] One coherent change; unrelated modifications are not bundled in
- [x] Commit subject follows Conventional Commits (`feat:` / `fix:` / `refactor:` / `chore:` / `docs:` / `perf:`)
- [x] Tests added or updated for non-trivial changes, test methods named `...Test`
- [x] New UI text has both Chinese and English entries under `web/src/i18n/` (no new UI text)
- [x] Architecture constraints stay green (`mvn test` runs the ArchUnit checks) (frontend-only change)
- [x] New source files carry the ASF license header (no new files)
- [x] Documentation touched where behaviour changed (bug fix only, no behaviour change visible to document)
--
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(proxy): keep the request staleness counter monotonic across effect runs [rocketmq-dashboard]
Posted by "zjncs (via GitHub)" <gi...@apache.org>.
zjncs commented on PR #4591:
URL: https://github.com/apache/rocketmq-dashboard/pull/4591#issuecomment-5903668795
GitHub refuses to reopen this PR (the branch was force-pushed after the close, which permanently locks the state). Re-submitted as #5194 with the same change rebased onto current master.
--
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(proxy): keep the request staleness counter monotonic across effect runs [rocketmq-dashboard]
Posted by "zjncs (via GitHub)" <gi...@apache.org>.
zjncs commented on PR #4591:
URL: https://github.com/apache/rocketmq-dashboard/pull/4591#issuecomment-5903639427
Reopening after the stale-bot close. Rebased the branch on current master (no conflicts — `Proxy.tsx` untouched since d05c1b97) and re-ran the full web suite locally: all green. The fix is unchanged and still applies as-is.
--
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(proxy): keep the request staleness counter monotonic across effect runs [rocketmq-dashboard]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #4591:
URL: https://github.com/apache/rocketmq-dashboard/pull/4591#issuecomment-5861192559
This pull request has been automatically closed because there has been no activity for 7 days. If you would like to continue working on this, please reopen the PR with updated changes.
--
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(proxy): keep the request staleness counter monotonic across effect runs [rocketmq-dashboard]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #4591: fix(proxy): keep the request staleness counter monotonic across effect runs
URL: https://github.com/apache/rocketmq-dashboard/pull/4591
--
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