Skip to content

fix(inkless:fetch): properly propagate exception back on failing FetchOffset#608

Merged
giuseppelillo merged 1 commit into
mainfrom
jeqo/fix-fetch-offset-error-handling
May 22, 2026
Merged

fix(inkless:fetch): properly propagate exception back on failing FetchOffset#608
giuseppelillo merged 1 commit into
mainfrom
jeqo/fix-fetch-offset-error-handling

Conversation

@jeqo

@jeqo jeqo commented May 22, 2026

Copy link
Copy Markdown
Contributor

The previous code threw a bare RuntimeException() without cause or message when TopicIdEnricher failed. This propagated to the request handler, which could log the full request context (all topic names) producing oversized log entries that get silently dropped by the log pipeline (journalpump → Vector → ClickHouse).

Now completes all pending futures with the error, matching the existing pattern for control plane failures in queryControlPlane().

@jeqo jeqo marked this pull request as ready for review May 22, 2026 10:10
@jeqo jeqo requested a review from Copilot May 22, 2026 10:10

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR adjusts FetchOffsetHandler.Job.start() so that failures during topic ID enrichment (e.g., TopicIdEnricher can’t resolve a topic UUID) are propagated back by completing all pending offset futures with an error, rather than throwing a bare unchecked exception that can bubble to the request handler and contribute to oversized log entries.

Changes:

  • Replace throw new RuntimeException() on TopicIdNotFoundException with completion of all pending futures using a RuntimeException that includes message and cause.
  • Add unit tests covering single- and multi-partition batches when topic ID resolution fails (UUID is ZERO_UUID).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
storage/inkless/src/main/java/io/aiven/inkless/consume/FetchOffsetHandler.java Completes all queued futures with a descriptive error on topic-ID enrichment failure instead of throwing.
storage/inkless/src/test/java/io/aiven/inkless/consume/FetchOffsetHandlerTest.java Adds tests asserting futures complete with an error (and no executor/control-plane calls) when enrichment fails.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

…hOffset

The previous code threw a bare RuntimeException() without cause or message when
TopicIdEnricher failed. This propagated to the request handler, which could log
the full request context (all topic names) producing oversized log entries that
get silently dropped by the log pipeline (journalpump → Vector → ClickHouse).

Now completes all pending futures with the error, matching the existing pattern
for control plane failures in queryControlPlane().

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@jeqo jeqo force-pushed the jeqo/fix-fetch-offset-error-handling branch from ee1bd09 to 71bb186 Compare May 22, 2026 10:44
@giuseppelillo giuseppelillo merged commit 17fafe8 into main May 22, 2026
4 checks passed
@giuseppelillo giuseppelillo deleted the jeqo/fix-fetch-offset-error-handling branch May 22, 2026 12:24
giuseppelillo pushed a commit that referenced this pull request May 29, 2026
…hOffset (#608)

The previous code threw a bare RuntimeException() without cause or message when
TopicIdEnricher failed. This propagated to the request handler, which could log
the full request context (all topic names) producing oversized log entries that
get silently dropped by the log pipeline (journalpump → Vector → ClickHouse).

Now completes all pending futures with the error, matching the existing pattern
for control plane failures in queryControlPlane().

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
giuseppelillo pushed a commit that referenced this pull request May 29, 2026
…hOffset (#608)

The previous code threw a bare RuntimeException() without cause or message when
TopicIdEnricher failed. This propagated to the request handler, which could log
the full request context (all topic names) producing oversized log entries that
get silently dropped by the log pipeline (journalpump → Vector → ClickHouse).

Now completes all pending futures with the error, matching the existing pattern
for control plane failures in queryControlPlane().

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
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