Skip to content

fix(studio): ignore stale consumer progress responses - #4546

Open
X-LightYear wants to merge 1 commit into
apache:masterfrom
X-LightYear:fix/4545-consumer-progress-race
Open

X-LightYear wants to merge 1 commit into
apache:masterfrom
X-LightYear:fix/4545-consumer-progress-race

Conversation

@X-LightYear

Copy link
Copy Markdown

Summary

  • prevent stale consumer progress requests from overwriting newer diagnostic results
  • track request ownership per instance/group diagnostic context
  • add deterministic regression coverage for overlapping progress requests

Root cause

ConsumerPage.loadProgress allowed overlapping requests for the same diagnostic context to update progressByGroup in completion order.

An older request could therefore complete after a newer "Re-diagnose" request and overwrite the newer result.

Fix

Track the latest progress request generation per existing cacheKey.

Only the request that still owns the latest generation is allowed to update progress or surface an error.

Testing

  • added deterministic deferred-Promise regression coverage for stale progress responses
  • ConsumerPageDiagnosticsRace.test.tsx: 1 passed
  • ConsumerPage.test.tsx: 32 passed
  • frontend lint: 0 errors
  • npm run build: passed

Fixes #4545

@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

This PR fixes a race condition in the consumer progress diagnostic UI where stale async requests could overwrite newer diagnostic results. The fix uses a request ID pattern to track request ownership, preventing out-of-order completions from corrupting the UI state.

Verdict: APPROVED — Clean, minimal fix with excellent test coverage.

Review Notes

Correctness

  • The progressRequestIdRef pattern correctly prevents stale responses from overwriting newer data
  • Both success and error paths check the request ID before updating state
  • Consistent with the existing groupRequestIdRef pattern in the same component

Performance

  • Minimal overhead: just a counter increment and comparison
  • No unnecessary re-renders or state updates

Tests

  • The regression test ConsumerPageDiagnosticsRace.test.tsx is well-structured
  • Uses deferred promises to deterministically control async timing
  • Tests both the success path (stale response ignored) and error path (stale error ignored)
  • Proper cleanup with afterEach

Compatibility

  • No API changes, purely internal state management
  • No breaking changes to component behavior

Suggestions

None — this is a solid fix. The request ID pattern is a standard approach for handling async race conditions in React.


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.

[Studio][Bug] Stale consumer progress response can overwrite a newer diagnostic result

2 participants