Re: [PR] fix(services): normalize import and cleanup results from partial envelopes [rocketmq-dashboard]
github-actions[bot] commented on PR #2942: URL: https://github.com/apache/rocketmq-dashboard/pull/2942#issuecomment-5593749779 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] fix(services): normalize import and cleanup results from partial envelopes [rocketmq-dashboard]
github-actions[bot] closed pull request #2942: fix(services): normalize import and cleanup results from partial envelopes URL: https://github.com/apache/rocketmq-dashboard/pull/2942 -- 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]
Re: [PR] fix(services): normalize import and cleanup results from partial envelopes [rocketmq-dashboard]
RockteMQ-AI commented on code in PR #2942:
URL:
https://github.com/apache/rocketmq-dashboard/pull/2942#discussion_r3909107449
##
web/src/api/producer.ts:
##
@@ -152,7 +152,12 @@ export function buildProducerConnectionSummary(
/** Fetch topic names for a managed instance. */
export async function fetchTopicList(instanceId: string): Promise {
const res = await client.get('/topics', { params: {
instanceId } });
- const topics = res.data.data?.map((topic) => topic.name) ??
res.data.topicList ?? [];
+ const rawTopics = res.data.data?.map((topic) => topic?.name) ??
res.data.topicList ?? [];
Review Comment:
The Array.isArray guard only protects the res.data.topicList fallback. If
data itself is a non-null non-array (e.g. a string or an object),
res.data.data?.map() still throws before the guard is reached — the same class
of crash this PR fixes for topicList, so the function isn't fully crash-proof
against malformed code-0 envelopes. Consider `const rawTopics =
Array.isArray(res.data.data) ? res.data.data.map((t) => t?.name) :
res.data.topicList;` and add a test with a non-array data payload. Minor note:
the filter also silently drops empty-string names, which is reasonable (invalid
topic names) but is a behavior change with no test coverage.
##
web/src/api/metadata.ts:
##
@@ -225,7 +225,15 @@ export async function exportTopics(params?:
TopicExportQuery) {
export async function importTopics(data: ImportTopicsRequest) {
const res = await client.post<{ data: ImportTopicsResult
}>('/topics/import', data);
- return res.data.data;
+ const result = res.data.data;
+ // A success envelope may still carry a null or partial result; normalize it
so the
+ // import UI can always iterate the reported groups and failures.
+ return {
Review Comment:
The normalization whitelists exactly four fields, so any additional field
the backend later adds to the import result (e.g. warnings, skipped counts)
will be silently dropped here and read as undefined in the UI once the type is
extended — the literal won't fail tsc for optional fields. Spreading the
original first (`{ ...result, imported: result?.imported ?? 0, failed: ...,
topics: ..., failures: ... }`) preserves the same null/partial guarantees while
staying forward-compatible with newer server versions. Same applies to
importConsumerGroups around line 435.
--
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]
[PR] fix(services): normalize import and cleanup results from partial envelopes [rocketmq-dashboard]
yyqdbngt opened a new pull request, #2942: URL: https://github.com/apache/rocketmq-dashboard/pull/2942 ## Summary - `api/metadata.importTopics` and `api/metadata.importConsumerGroups` normalize their result: a success envelope with `data: null` or a partial result now resolves to zeroed counters and empty `topics`/`groups`/`failures` arrays - `api/producer.fetchTopicList` tolerates a non-array topic-list payload and drops non-string entries before sorting - `opsService.cleanupAuditLogs` treats a null API result as zero deleted records - Adds regression tests for every case (null payloads, missing arrays, non-array topic lists, plus happy paths) ## Why The client interceptor validates the business `code`, but a `code: 0` envelope can still carry `data: null` or a result object with missing fields. The import endpoints returned `res.data.data` verbatim, so the import UI's `result.topics.map(...)` / `result.groups.map(...)` crashed on a null or partial payload. `fetchTopicList` fell back to `topicList` and called `.sort()` unconditionally — a string payload threw `topics.sort is not a function`. `cleanupAuditLogs` read `result.deleted` off an unguarded response. ## Testing - `./node_modules/.bin/vitest run src/api/metadata.test.ts src/api/producer.test.ts src/services/opsService.cleanup.test.ts` → 24 passed (8 new; verified 7 fail with the pre-fix code, the remaining one is the happy-path check that passes both before and after) - `./node_modules/.bin/tsc --noEmit` → clean - `./node_modules/.bin/eslint` on the 6 changed files → 0 errors -- 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]
