Skip to content

[fix][cpp-client] Pass seek error to callback when SEEK command fails#549

Merged
BewareMyPower merged 8 commits intoapache:mainfrom
bigo-sg:fix/issue-528
Apr 15, 2026
Merged

[fix][cpp-client] Pass seek error to callback when SEEK command fails#549
BewareMyPower merged 8 commits intoapache:mainfrom
bigo-sg:fix/issue-528

Conversation

@zhanglistar
Copy link
Copy Markdown
Contributor

@zhanglistar zhanglistar commented Mar 8, 2026

Fixes #528

Motivation

In ConsumerImpl::seekAsyncInternal, when the broker responds to the SEEK request with a non-OK result, we reset seek state and invoke the user seekCallback_ on the executor. The failure path incorrectly called callback(ResultOk), so callers could observe success even though the seek had failed.

Modifications

  • In the result != ResultOk branch of the SEEK response handler, change the posted callback from callback(ResultOk) to callback(result) (capture result in the lambda and forward the actual error code).

Verifying this change

  • CI passes.

Seek failure is now surfaced with the correct Result code to seekAsync / synchronous seek callers.

Documentation

  • doc-not-needed

Internal bug fix; no API or user-facing behavior contract change beyond reporting the correct error on seek failure.

Comment thread lib/ConsumerImpl.cc Outdated
@BewareMyPower BewareMyPower added this to the 4.1.0 milestone Mar 17, 2026
Comment thread lib/ConsumerImpl.cc Outdated
zhangzhibiao added 2 commits March 21, 2026 21:28
Comment thread lib/ConsumerImpl.cc
@zhanglistar zhanglistar changed the title [Test] Fix flaky test: ReaderSeekTest.testHasMessageAvailableAfterSeekToEnd [fix][cpp-client] Pass seek error to callback when SEEK command fails Mar 27, 2026
@BewareMyPower BewareMyPower modified the milestones: 4.1.0, 4.2.0 Mar 29, 2026
@BewareMyPower
Copy link
Copy Markdown
Contributor

You can merge the latest main branch to get CI running

@BewareMyPower
Copy link
Copy Markdown
Contributor

Could you rebase or merge the latest master branch?

@BewareMyPower BewareMyPower merged commit f9995da into apache:main Apr 15, 2026
14 checks passed
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.

[Bug] Flaky test: ReaderSeekTest.testHasMessageAvailableAfterSeekToEnd

2 participants