diff --git a/frontend/.claude/skills/code-standards/SKILL.md b/frontend/.claude/skills/code-standards/SKILL.md index a75c0539b..5e3a008c9 100644 --- a/frontend/.claude/skills/code-standards/SKILL.md +++ b/frontend/.claude/skills/code-standards/SKILL.md @@ -40,6 +40,13 @@ Most issues are auto-fixed: bun x ultracite fix ``` +## Console Logging + +- **Never** use `console.log`, `console.debug`, or `console.info` in production code +- Only use `console.error` and `console.warn` for actionable errors +- For debug logging, wrap in `if (IsDev) { ... }` +- Don't use `biome-ignore` to suppress console warnings - remove the logs instead + ## Rules See `rules/` directory for detailed guidance. diff --git a/frontend/.claude/skills/code-standards/rules/no-legacy.md b/frontend/.claude/skills/code-standards/rules/no-legacy.md index ce0612026..321a720ec 100644 --- a/frontend/.claude/skills/code-standards/rules/no-legacy.md +++ b/frontend/.claude/skills/code-standards/rules/no-legacy.md @@ -88,6 +88,15 @@ When modifying files with legacy patterns: 3. **Keep changes minimal** if just fixing bugs 4. **New features** must use modern patterns +### MobX → React State: Check Side-Effects + +When converting observables to React state, automatic behaviors must become explicit: +- MobX observable arrays auto-update UI when mutated +- React state requires explicit `setState()` calls +- Clearing, resetting, or initializing state needs manual handling + +**Checklist**: List all observable mutations → add equivalent `setState` calls. + ## Legacy Locations - MobX stores: `src/state/` (do not add new files) diff --git a/frontend/.claude/skills/testing/SKILL.md b/frontend/.claude/skills/testing/SKILL.md index 36e5dabd6..4741c8d4e 100644 --- a/frontend/.claude/skills/testing/SKILL.md +++ b/frontend/.claude/skills/testing/SKILL.md @@ -40,6 +40,14 @@ bun run test:coverage # Coverage report - `.test.ts` = unit (Node.js), `.test.tsx` = integration (JSDOM) - Always use `test-utils/test-utils.tsx` for React component tests +- Test that features are fully wired: UI elements must connect to actual functionality + +### Feature Completeness Testing + +When implementing interactive features (buttons, forms, etc.): +- Verify event handlers call the correct functions with proper parameters +- Test that AbortSignals, callbacks, and other "plumbing" are passed through +- Don't assume UI presence means functionality works - test the connection ## When to Use This Skill diff --git a/frontend/src/components/pages/topics/Tab.Messages/index.tsx b/frontend/src/components/pages/topics/Tab.Messages/index.tsx index 2dd583f4c..f83c46f42 100644 --- a/frontend/src/components/pages/topics/Tab.Messages/index.tsx +++ b/frontend/src/components/pages/topics/Tab.Messages/index.tsx @@ -485,93 +485,96 @@ export const TopicMessageView: FC = (props) => { // Convert executeMessageSearch to useCallback // biome-ignore lint/complexity/noExcessiveCognitiveComplexity: complex business logic - const executeMessageSearch = useCallback(async (): Promise => { - const canUseFilters = - (api.topicPermissions.get(props.topic.topicName)?.canUseSearchFilters ?? true) && !isServerless(); + const executeMessageSearch = useCallback( + async (abortSignal?: AbortSignal): Promise => { + const canUseFilters = + (api.topicPermissions.get(props.topic.topicName)?.canUseSearchFilters ?? true) && !isServerless(); - // Get current search params from Zustand store for filters - const currentSearchParams = getSearchParams(props.topic.topicName); + // Get current search params from Zustand store for filters + const currentSearchParams = getSearchParams(props.topic.topicName); - let filterCode = ''; - if (canUseFilters) { - const functionNames: string[] = []; - const functions: string[] = []; + let filterCode = ''; + if (canUseFilters) { + const functionNames: string[] = []; + const functions: string[] = []; - // Use filters from URL state instead of localStorage - const filteredSearchParams = filters.filter( - (searchParam) => searchParam.isActive && searchParam.code && searchParam.transpiledCode - ); + // Use filters from URL state instead of localStorage + const filteredSearchParams = filters.filter( + (searchParam) => searchParam.isActive && searchParam.code && searchParam.transpiledCode + ); - for (const searchParam of filteredSearchParams) { - const name = `filter${functionNames.length + 1}`; - functionNames.push(name); - functions.push(`function ${name}() { + for (const searchParam of filteredSearchParams) { + const name = `filter${functionNames.length + 1}`; + functionNames.push(name); + functions.push(`function ${name}() { ${wrapFilterFragment(searchParam.transpiledCode)} }`); - } + } - if (functions.length > 0) { - filterCode = `${functions.join('\n\n')}\n\nreturn ${functionNames.map((f) => `${f}()`).join(' && ')}`; - if (IsDev) { - // biome-ignore lint/suspicious/noConsole: intentional console usage - console.log(`constructed filter code (${functions.length} functions)`, `\n\n${filterCode}`); + if (functions.length > 0) { + filterCode = `${functions.join('\n\n')}\n\nreturn ${functionNames.map((f) => `${f}()`).join(' && ')}`; + if (IsDev) { + // biome-ignore lint/suspicious/noConsole: intentional console usage + console.log(`constructed filter code (${functions.length} functions)`, `\n\n${filterCode}`); + } } } - } - const request = { - topicName: props.topic.topicName, - partitionId: partitionID, - startOffset, - startTimestamp: currentSearchParams?.startTimestamp ?? uiState.topicSettings.searchParams.startTimestamp, - maxResults, - filterInterpreterCode: encodeBase64(sanitizeString(filterCode)), - includeRawPayload: true, - keyDeserializer, - valueDeserializer, - } as MessageSearchRequest; + const request = { + topicName: props.topic.topicName, + partitionId: partitionID, + startOffset, + startTimestamp: currentSearchParams?.startTimestamp ?? uiState.topicSettings.searchParams.startTimestamp, + maxResults, + filterInterpreterCode: encodeBase64(sanitizeString(filterCode)), + includeRawPayload: true, + keyDeserializer, + valueDeserializer, + } as MessageSearchRequest; - try { - setFetchError(null); - setSearchPhase('Searching...'); + try { + setFetchError(null); + setSearchPhase('Searching...'); + + const messageSearch = createMessageSearch(); + const startTime = Date.now(); + + const result = await messageSearch.startSearch(request, abortSignal).catch((err: Error) => { + const msg = err.message ?? String(err); + // biome-ignore lint/suspicious/noConsole: intentional console usage + console.error(`error in searchTopicMessages: ${msg}`); + setFetchError(err); + setSearchPhase(null); + return []; + }); - const messageSearch = createMessageSearch(); - const startTime = Date.now(); + const endTime = Date.now(); + setMessages(result); + setSearchPhase(null); + setElapsedMs(endTime - startTime); + setBytesConsumed(messageSearch.bytesConsumed); + setTotalMessagesConsumed(messageSearch.totalMessagesConsumed); - const result = await messageSearch.startSearch(request).catch((err: Error) => { - const msg = err.message ?? String(err); + return result; + } catch (error: unknown) { // biome-ignore lint/suspicious/noConsole: intentional console usage - console.error(`error in searchTopicMessages: ${msg}`); - setFetchError(err); + console.error(`error in searchTopicMessages: ${(error as Error).message ?? String(error)}`); + setFetchError(error as Error); setSearchPhase(null); return []; - }); - - const endTime = Date.now(); - setMessages(result); - setSearchPhase(null); - setElapsedMs(endTime - startTime); - setBytesConsumed(messageSearch.bytesConsumed); - setTotalMessagesConsumed(messageSearch.totalMessagesConsumed); - - return result; - } catch (error: unknown) { - // biome-ignore lint/suspicious/noConsole: intentional console usage - console.error(`error in searchTopicMessages: ${(error as Error).message ?? String(error)}`); - setFetchError(error as Error); - setSearchPhase(null); - return []; - } - }, [ - props.topic.topicName, - partitionID, - startOffset, - maxResults, - getSearchParams, - keyDeserializer, - valueDeserializer, - filters, - ]); + } + }, + [ + props.topic.topicName, + partitionID, + startOffset, + maxResults, + getSearchParams, + keyDeserializer, + valueDeserializer, + filters, + ] + ); // Convert searchFunc to useCallback const searchFunc = useCallback( @@ -582,12 +585,6 @@ export const TopicMessageView: FC = (props) => { const searchParams = `${startOffset} ${maxResults} ${partitionID} ${currentSearchParams?.startTimestamp ?? uiState.topicSettings.searchParams.startTimestamp} ${keyDeserializer} ${valueDeserializer} ${filtersSignature}`; if (searchParams === currentSearchRunRef.current && source === 'auto') { - // biome-ignore lint/suspicious/noConsole: intentional console usage - console.log('ignoring search, search params are up to date, and source is auto', { - newParams: searchParams, - oldParams: currentSearchRunRef.current, - trigger: source, - }); return; } @@ -597,19 +594,15 @@ export const TopicMessageView: FC = (props) => { abortControllerRef.current = null; } - // biome-ignore lint/suspicious/noConsole: intentional console usage - console.log('starting a new message search', { - newParams: searchParams, - oldParams: currentSearchRunRef.current, - trigger: source, - }); - // Start new search currentSearchRunRef.current = searchParams; abortControllerRef.current = new AbortController(); + // Clear messages immediately when starting new search + setMessages([]); + try { - executeMessageSearch() + executeMessageSearch(abortControllerRef.current?.signal) // biome-ignore lint/suspicious/noConsole: intentional console usage .catch(console.error) .finally(() => { diff --git a/frontend/src/state/backend-api.ts b/frontend/src/state/backend-api.ts index 43f1166de..0bf9c1f22 100644 --- a/frontend/src/state/backend-api.ts +++ b/frontend/src/state/backend-api.ts @@ -14,6 +14,7 @@ import { create, type Registry } from '@bufbuild/protobuf'; import type { ConnectError } from '@connectrpc/connect'; import { Code } from '@connectrpc/connect'; +import { createLinkedAbortController } from '@connectrpc/connect/protocol'; import { createStandaloneToast, redpandaTheme, redpandaToastOptions } from '@redpanda-data/ui'; import { consoleHasEnterpriseFeature, @@ -2765,7 +2766,7 @@ export function createMessageSearch() { messages: observable([] as TopicMessage[], { deep: false }), // biome-ignore lint/complexity/noExcessiveCognitiveComplexity: complexity 64, refactor later - async startSearch(_searchRequest: MessageSearchRequest): Promise { + async startSearch(_searchRequest: MessageSearchRequest, externalSignal?: AbortSignal): Promise { // https://connectrpc.com/docs/web/using-clients // https://github.com/connectrpc/connect-es // https://github.com/connectrpc/examples-es @@ -2800,8 +2801,11 @@ export function createMessageSearch() { this.messages.length = 0; this.elapsedMs = null; - const messageSearchAbortController = new AbortController(); - this.abortController = messageSearchAbortController; + // Links external signal (component control) with internal controller - both can abort the same stream + // Ensures cleanup works from component unmount AND direct stopSearch() calls + const linkedController = createLinkedAbortController(externalSignal); + this.abortController = linkedController; + const abortSignal = linkedController.signal; // do it const req = create(ListMessagesRequestSchema); @@ -2826,23 +2830,19 @@ export function createMessageSearch() { try { for await (const res of client.listMessages(req, { - signal: messageSearchAbortController.signal, + signal: abortSignal, timeoutMs, })) { - if (messageSearchAbortController.signal.aborted) { + if (abortSignal.aborted) { break; } try { switch (res.controlMessage.case) { case 'phase': - // biome-ignore lint/suspicious/noConsole: intentional console usage - console.log(`phase: ${res.controlMessage.value.phase}`); this.searchPhase = res.controlMessage.value.phase; break; case 'progress': - // biome-ignore lint/suspicious/noConsole: intentional console usage - console.log(`progress: ${res.controlMessage.value.messagesConsumed}`); this.bytesConsumed = Number(res.controlMessage.value.bytesConsumed); this.totalMessagesConsumed = Number(res.controlMessage.value.messagesConsumed); break; @@ -3094,19 +3094,22 @@ export function createMessageSearch() { } } } catch (e) { - this.abortController = null; this.searchPhase = 'Done'; this.bytesConsumed = 0; this.totalMessagesConsumed = 0; this.searchPhase = null; // https://connectrpc.com/docs/web/errors - if (messageSearchAbortController.signal.aborted) { + if (abortSignal.aborted) { // Do not throw, this is a user cancellation } else { // biome-ignore lint/suspicious/noConsole: intentional console usage console.error('startMessageSearchNew: error in await loop of client.listMessages', { error: e }); throw e; } + } finally { + // Always cleanup the abort controller reference to prevent memory leaks + // This ensures cleanup happens whether the stream completes, errors, or is cancelled + this.abortController = null; } // one done