CodeSmell commented on code in PR #11920: URL: https://github.com/apache/camel/pull/11920#discussion_r1384905892
########## components/camel-kafka/src/main/java/org/apache/camel/component/kafka/consumer/support/KafkaRecordProcessor.java: ########## @@ -112,30 +115,48 @@ public ProcessingResult processExchange( exchange.setException(e); } if (exchange.getException() != null) { - boolean breakOnErrorExit = processException(exchange, partition, lastResult.getPartitionLastOffset(), + + LOG.debug("an exception was thrown for record at partition {} and offset {}", + record.partition(), record.offset()); + + boolean breakOnErrorExit = processException(exchange, topicPartition, record, lastResult, exceptionHandler); - return new ProcessingResult(breakOnErrorExit, lastResult.getPartitionLastOffset(), true); + + return new ProcessingResult(breakOnErrorExit, lastResult.getPartition(), lastResult.getPartitionLastOffset(), true); } else { - return new ProcessingResult(false, record.offset(), exchange.getException() != null); + return new ProcessingResult(false, record.partition(), record.offset(), exchange.getException() != null); } } private boolean processException( - Exchange exchange, TopicPartition partition, long partitionLastOffset, + Exchange exchange, TopicPartition topicPartition, + ConsumerRecord<Object, Object> record, ProcessingResult lastResult, ExceptionHandler exceptionHandler) { // processing failed due to an unhandled exception, what should we do if (configuration.isBreakOnFirstError()) { + + if (lastResult.getPartition() != -1 && + lastResult.getPartition() != record.partition()) { + LOG.error("about to process an exception with UNEXPECTED partition & offset. Got topic partition {}. " + + " The last result was on partition {} with offset {} but was expecting partition {} with offset {}", + topicPartition.partition(), lastResult.getPartition(), lastResult.getPartitionLastOffset(), + record.partition(), record.offset()); + } + // we are failing and we should break out if (LOG.isWarnEnabled()) { - LOG.warn("Error during processing {} from topic: {}", exchange, partition.topic(), exchange.getException()); - LOG.warn("Will seek consumer to offset {} and start polling again.", partitionLastOffset); + LOG.warn("Error during processing {} from topic: {}", exchange, topicPartition.topic(), exchange.getException()); + LOG.warn("Will seek consumer to offset {} on partition {} and start polling again.", + lastResult.getPartitionLastOffset(), lastResult.getPartition()); } - // force commit, so we resume on next poll where we failed except when the failure happened - // at the first message in a poll - if (partitionLastOffset != AbstractCommitManager.START_OFFSET) { - commitManager.forceCommit(partition, partitionLastOffset); + // force commit, so we resume on next poll where we failed + // except when the failure happened at the first message in a poll + if (lastResult.getPartitionLastOffset() != AbstractCommitManager.START_OFFSET) { + // should we use record.offset ? + //commitManager.forceCommit(topicPartition, record.offset() - 1); Review Comment: this change that is commented out is what mostly addresses the issues found in CAMEL-20044 and CAMEL-19894 I can submit this as a separate PR I commented it out here b/c I had not added unit tests and b/c I wasn't sure if that would impact other ways that the Camel riders would want breakOnFirstError to work I did see CAMEL-20089 but hope this fix can move forward as we it does fix 2 other issues -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@camel.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org