Skip to content

fix(web): harden producer group AutoComplete option filtering - #2872

Open
yyqdbngt wants to merge 1 commit into
apache:rocketmq-studiofrom
yyqdbngt:codex/yy-studio-producer-filter
Open

fix(web): harden producer group AutoComplete option filtering#2872
yyqdbngt wants to merge 1 commit into
apache:rocketmq-studiofrom
yyqdbngt:codex/yy-studio-producer-filter

Conversation

@yyqdbngt

Copy link
Copy Markdown

Summary

  • Replace the fragile inline AutoComplete filterOption in the studio Producer page (pages/studio/Producer.tsx) with an exported matchesProducerGroupOption helper.
  • The old expression was option?.value.toLowerCase().includes(inputValue.toLowerCase()) ?? false. The ?? false only protects the case where option itself is nullish (the optional chain short-circuits); it does NOT protect the cases that actually throw:
    • option.value is null/undefined (e.g. a null group in the API response) → TypeError: Cannot read properties of null (reading 'toLowerCase') inside the dropdown render;
    • a non-string option.value (number, etc.) → same TypeError.
  • The new helper coerces both sides with String(...) ?? '' so any option value or search string is compared safely, case-insensitively.

Why

The producer-group selector builds its options from live API data (fetchProducerGroups), unlike the static topic/instance selectors. A single malformed payload entry therefore used to throw inside antd's filter path and break the whole suggestion dropdown. The same defensive String(option?.label ?? '') pattern is already used in components/InstanceSelect.tsx; this brings the Producer selector in line with it.

Testing

  • cd web && ./node_modules/.bin/vitest run src/pages/studio/__tests__/producerGroupFilter.test.ts — 5 new regression tests passed (case-insensitive match, empty input, null/undefined/non-string option values, undefined input).
  • cd web && ./node_modules/.bin/vitest run src/pages/studio/__tests__/Producer.test.tsx — 13 pre-existing tests still pass.
  • cd web && ./node_modules/.bin/tsc --noEmit — clean.
  • cd web && ./node_modules/.bin/eslint src/pages/studio/Producer.tsx src/pages/studio/__tests__/producerGroupFilter.test.ts — 0 errors (1 react-refresh warning for the exported helper, same precedent as other page-level helper exports).

@RockteMQ-AI RockteMQ-AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

Clean defensive fix — the old ?? false only guarded against a nullish option, not a null/non-string option.value. The new helper correctly coerces both sides with String(...) and covers the edge cases with regression tests.

LGTM.


Automated review by github-manager-bot

@RockteMQ-AI RockteMQ-AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

Defensive fix that improves input validation and error handling. Code looks clean and follows existing patterns.

LGTM


Automated review by "github-manager-bot"

@RockteMQ-AI RockteMQ-AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — defensive fix improving input validation and error handling.


Automated review by "github-manager-bot"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants