lizhimins commented on PR #5835: URL: https://github.com/apache/rocketmq-dashboard/pull/5835#issuecomment-6120329512
> Thanks for the careful write-up. The defect this PR describes was already fixed on > `rocketmq-studio` by `579918565` (#5032, which folded in #5256): the ownership check was moved > out of the `try` block to *after* the `catch`, so `Proxy.tsx:134-135` now runs > `if (requestId !== loadRequestId.current) return false;` before `setProxyNodes` / > `persistProxyAddress` on both the success and the probe-failure path. Since your branch has been > merged with the latest trunk, the guard you added inside `catch` is an exact duplicate of the very > next statement (same condition, same `return false`) and is a behavioural no-op — I could not > reproduce "fails before the fix, passes after" against `5e4c39b05`. > > The test itself is worth keeping, though: `ProxyStaleTopology.test.tsx` does go red if the guard > from #5032 is removed, and `Proxy.test.tsx` currently has no stale/superseded coverage. If you > re-file it as a test-only PR (`test(proxy): cover stale probe failure`, dropping the `Proxy.tsx` > hunk and rewording the body as a regression lock for #5032 rather than a new fix), I'm happy to > take it. Closing this one as already fixed. -- 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]
