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/04 16:00:41 UTC
[PR] fix(message): keep the trace tab responsive after an empty trace query [rocketmq-dashboard]
unbridled-41 opened a new pull request, #3156:
URL: https://github.com/apache/rocketmq-dashboard/pull/3156
## Problem / Evidence
Found by code inspection (no issue tracks it; no existing PR covers it).
In `web/src/pages/instance/message.tsx`, `runTraceQuery` bumps `traceGenerationRef` **before** validating its input, and the empty-input branch returns without touching `traceLoading`:
1. Open a message's 消息轨迹 tab (or click 轨迹) — `loadMessageTrace` starts with generation G and `traceLoading = true`.
2. While that request is in flight, clear the Message ID input and click 查询轨迹 — the generation becomes G+1, the branch sets the validation error and returns.
3. The in-flight request's `finally { if (traceGenerationRef.current === requestGeneration) setTraceLoading(false); }` compares G ≠ G+1 and skips, and nothing else ever resets `traceLoading`.
Result: the 消息轨迹 tab shows 正在加载轨迹数据… indefinitely, and because the loading text takes render precedence over `traceError`, the validation hint (请输入 Message ID) that was set is never displayed. The tab only recovers if the user issues another non-empty trace query or reopens the dialog.
## Root cause / Fix
The generation bump orphaned the in-flight load, so its guarded `finally` can no longer reset `traceLoading`; the empty-input branch must own that cleanup. It now stops the spinner, clears stale trace data, and surfaces the validation error.
## Priority & scoring
- PRIORITY 72/100: impact 28/40 (trace tab hangs with an eternal spinner on a very plausible user sequence — click 轨迹, clear input, click 查询轨迹; recovery requires extra actions), scope 12/20 (message trace tab), reproducibility 17/20 (deterministic), maintenance value 15/20 (small fix inside the existing generation-guard pattern).
- FIX_CONFIDENCE 88/100: cleanup placed at the exact divergence point between the bump and the guarded finally; covered by a red→green regression test.
## Tests
- New: `MessagePageAsyncState.test.tsx` — `keeps the trace tab responsive when an empty trace query supersedes an in-flight load`: opens the trace tab against a pending `getMessageTrace`, submits an empty query, asserts the validation error is visible and the spinner is gone, then resolves the pending request and asserts the late response does not resurrect the spinner. Red on the unfixed source (error text unreachable behind the stuck spinner), green after.
- `npx vitest run src/pages/instance/__tests__/MessagePageAsyncState.test.tsx src/pages/instance/__tests__/MessagePage.test.tsx`: 28/28 pass.
- `npx tsc -p tsconfig.app.json --noEmit`, `eslint` on changed files: clean.
## Risk
Low: only the empty-input branch changed; non-empty trace queries and the in-flight load lifecycle are untouched. The late in-flight trace response remains discarded after an empty query — consistent with the user having superseded it.
--
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(message): keep the trace tab responsive after an empty trace query [rocketmq-dashboard]
Posted by "lizhimins (via GitHub)" <gi...@apache.org>.
lizhimins merged PR #3156:
URL: https://github.com/apache/rocketmq-dashboard/pull/3156
--
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(message): keep the trace tab responsive after an empty trace query [rocketmq-dashboard]
Posted by "unbridled-41 (via GitHub)" <gi...@apache.org>.
unbridled-41 commented on PR #3156:
URL: https://github.com/apache/rocketmq-dashboard/pull/3156#issuecomment-5543785810
## Verification evidence (supplementary)
**Why CI is red on this PR:** repo-wide pre-existing CI breakage — every `ci.yml` run has failed at startup (`startup_failure`, zero jobs executed) since 2026-08-28, including upstream's own pushes to `rocketmq-studio` and all contributor PRs ("This run likely failed because of a workflow file issue"; last `ci.yml` change was PR #2621, 2026-08-27). This branch does not modify `ci.yml` (diff vs `origin/rocketmq-studio` is empty). CI-equivalent verification was run locally:
**Regression test (red → green)**
- Red, before the fix: `npx vitest run src/pages/instance/__tests__/MessagePageAsyncState.test.tsx -t 'empty trace query'` → `keeps the trace tab responsive when an empty trace query supersedes an in-flight load` failed with `TestingLibraryElementError: Unable to find an element with the text: 请输入 Message ID` — exactly the defect: the stuck `traceLoading` spinner takes render precedence, so the validation error set by the empty-input branch was never rendered.
- Green, after the fix: same command passes; the test asserts the error is visible, the spinner is gone, and the late in-flight trace response does not resurrect the spinner.
**Module / related suites**
- `npx vitest run src/pages/instance/__tests__/MessagePageAsyncState.test.tsx src/pages/instance/__tests__/MessagePage.test.tsx` → 28/28 pass.
- `npx tsc -p tsconfig.app.json --noEmit` → clean; `eslint` on both changed files → clean.
**Full suite and build (CI frontend equivalents)**
- Full `npx vitest run --testTimeout=60000 --hookTimeout=60000` on this branch → 923 tests (922 baseline + 1 new), 1 failed / 922 passed. The single failure is `ConsumerPage.test.tsx > shows group health diagnostics from subscriptions, progress and clients` — a file this PR does not touch. The full suite is load-flaky in this sandbox (repeated pristine runs produce different failure sets: 9/4 files, 15/7, then 3; all near the configured 20s `testTimeout`), and this exact test passes in isolation both on pristine (53/53 across ConsumerPage/AclPage/NotificationDeliveriesPage) and on a sibling branch (ConsumerPage 50/50). No deterministic failure exists on this branch.
- `npm run build` (CI frontend-build equivalent) → succeeds.
No new commits were added for this; the code and tests were already complete — this comment documents the verification the broken CI could not run.
--
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