CAMEL-24191: camel-aws2-kinesis - fix KCL consumer session credentials and checkpoint/error handling#24835
Conversation
…s and checkpoint/error handling Two defects in the KCL-based Kinesis consumer: - In both the DynamoDB and CloudWatch async-client builders the credential branches tested accessKey+secretKey before accessKey+secretKey+sessionToken, so the session-token branch was unreachable and those clients never used STS temporary credentials. The session-credentials branch is now checked first. - processRecords wrapped processing in catch(Throwable) that logged without the exception and swallowed it, so the scheduler later checkpointed past records that failed to process (data loss), and it never checkpointed after a successful batch (a crash reprocessed the whole lease). It now checkpoints only after the batch is processed successfully, and on failure routes the exception to the consumer ExceptionHandler without checkpointing, so KCL redelivers the batch from the last checkpoint. Adds KclKinesis2ConsumerRecordProcessingTest asserting the checkpoint is taken after a successful batch and skipped when processing fails. Related to CAMEL-24156. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Andrea Cosentino <ancosen@gmail.com>
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
gnodet
left a comment
There was a problem hiding this comment.
Claude Code on behalf of gnodet
Clean, well-scoped fix for two real bugs in the KCL-based Kinesis consumer. Both issues have been present since the original session-credential support was added (CAMEL-20912/20913, commit 8a2116f).
Fix 1 — Session credentials branch ordering ✅
The if/else-if chain checked accessKey && secretKey before the more specific accessKey && secretKey && sessionToken. Since the 2-param condition is a strict superset of the 3-param one, the session-token branch was dead code — STS temporary credentials silently fell back to basic credentials, dropping the session token. The fix correctly reorders to most-specific-first in both the DynamoDB and CloudWatch builders.
Fix 2 — Checkpoint/error handling ✅
The old processRecords() had catch (Throwable t) { LOG.error("..."); } — three problems in one line:
- Exception swallowed (no stack trace logged)
- No checkpoint on success → crash = reprocess entire lease (duplication)
- No rethrow/routing on failure → KCL could advance past failed records (data loss)
The new code:
- Checkpoints only after a successful batch — correct at-least-once semantics
- Routes processing failures to
ExceptionHandlerwithout checkpointing → KCL redelivers from last checkpoint - Catches
ShutdownException | InvalidStateExceptionfor checkpoint failures — consistent withshardEnded()andshutdownRequested()handlers in the same class
The forEach → plain for loop change is a nice simplification — the lambda wrapping Exception → RuntimeException is no longer needed since the for-loop naturally propagates to the surrounding try-catch.
Test ✅
KclKinesis2ConsumerRecordProcessingTest directly constructs the inner CamelKinesisRecordProcessor (package-private, so accessible from same-package test) and verifies both the success-checkpoint and failure-no-checkpoint behaviors with mocked RecordProcessorCheckpointer.
No concerns. Good fix for a genuine data-safety issue.
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 9 tested, 29 compile-only — current: 9 all testedMaveniverse Scalpel detected 38 affected modules (current approach: 9).
|
The test started the endpoint, which built a real Kinesis client and failed in CI with "Unable to load region" (no AWS_REGION configured). Provide a mock KinesisClient on the configuration so the endpoint does not build a real client on start, matching the sibling consumer tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Andrea Cosentino <ancosen@gmail.com>
…s and checkpoint/error handling (#24848) Backport of #24835 to camel-4.18.x. Reorder credential branches so session tokens are used (both DynamoDB and CloudWatch builders); checkpoint only after a successful batch and route processing failures to the ExceptionHandler without checkpointing so KCL redelivers. Adds KclKinesis2ConsumerRecordProcessingTest. Related to CAMEL-24156. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…s and checkpoint/error handling (#24849) Backport of #24835 to camel-4.14.x. Reorder credential branches so session tokens are used (both DynamoDB and CloudWatch builders); checkpoint only after a successful batch and route processing failures to the ExceptionHandler without checkpointing so KCL redelivers. Adds KclKinesis2ConsumerRecordProcessingTest. Related to CAMEL-24156. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Description
Fixes CAMEL-24191 (split out of the AWS review umbrella CAMEL-24156). Two defects in the KCL-based Kinesis consumer (
KclKinesis2Consumer):Session credentials silently ignored. In both the DynamoDB and CloudWatch async-client builders the branch order tested
accessKey && secretKeybeforeaccessKey && secretKey && sessionToken, so the session-token branch was unreachable and those clients never used STS temporary credentials. The session-credentials branch is now checked first in both builders.Checkpoint/error handling lost or reprocessed data.
processRecordswrapped processing incatch (Throwable t)that logged a message without the exception and did not rethrow, so the scheduler later checkpointed past records that failed to process (data loss); it also never checkpointed after a successful batch, so a crash reprocessed the whole lease. It now:ExceptionHandler(with stack trace) and does not checkpoint, so KCL redelivers the batch from the last checkpoint.Tests
Adds
KclKinesis2ConsumerRecordProcessingTest— asserts the checkpoint is taken after a successful batch and is never taken when processing throws. Existing kinesis tests continue to pass.Backport
Same code on
camel-4.18.xandcamel-4.14.x; will be backported after merge (fixVersions 4.22.0 / 4.18.4 / 4.14.9).Related to CAMEL-24156.
Claude Code on behalf of oscerd