From 3d8de782d7db373cd3c536c6817e1b47fc279123 Mon Sep 17 00:00:00 2001 From: Gary Russell Date: Tue, 12 Feb 2019 13:20:14 -0500 Subject: [PATCH] Fix ErrorHandler for Consumer Errors When a container-aware error handler is called from a `ConcurrentMessageListenerContainer`, when the consumer throws an exception (rather than the listener), the wrong container was passed in the container argument. The child container was passed in instead of the concurrent container. --- .../KafkaMessageListenerContainer.java | 2 +- ...rentMessageListenerContainerMockTests.java | 75 +++++++++++++++++++ 2 files changed, 76 insertions(+), 1 deletion(-) create mode 100644 spring-kafka/src/test/java/org/springframework/kafka/listener/ConcurrentMessageListenerContainerMockTests.java diff --git a/spring-kafka/src/main/java/org/springframework/kafka/listener/KafkaMessageListenerContainer.java b/spring-kafka/src/main/java/org/springframework/kafka/listener/KafkaMessageListenerContainer.java index 048ca791..f8adc256 100644 --- a/spring-kafka/src/main/java/org/springframework/kafka/listener/KafkaMessageListenerContainer.java +++ b/spring-kafka/src/main/java/org/springframework/kafka/listener/KafkaMessageListenerContainer.java @@ -851,7 +851,7 @@ public class KafkaMessageListenerContainer // NOSONAR comment density try { if (!this.isBatchListener && this.errorHandler != null) { this.errorHandler.handle(e, Collections.emptyList(), this.consumer, - KafkaMessageListenerContainer.this); + KafkaMessageListenerContainer.this.container); } else if (this.isBatchListener && this.batchErrorHandler != null) { this.batchErrorHandler.handle(e, new ConsumerRecords(Collections.emptyMap()), this.consumer, diff --git a/spring-kafka/src/test/java/org/springframework/kafka/listener/ConcurrentMessageListenerContainerMockTests.java b/spring-kafka/src/test/java/org/springframework/kafka/listener/ConcurrentMessageListenerContainerMockTests.java new file mode 100644 index 00000000..b15ec11c --- /dev/null +++ b/spring-kafka/src/test/java/org/springframework/kafka/listener/ConcurrentMessageListenerContainerMockTests.java @@ -0,0 +1,75 @@ +/* + * Copyright 2019 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.kafka.listener; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.BDDMockito.given; +import static org.mockito.BDDMockito.willAnswer; +import static org.mockito.Mockito.mock; + +import java.util.Collections; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicReference; + +import org.apache.kafka.clients.consumer.Consumer; +import org.apache.kafka.clients.consumer.ConsumerRecords; +import org.junit.jupiter.api.Test; + +import org.springframework.kafka.core.ConsumerFactory; + +/** + * @author Gary Russell + * @since 2.2.4 + * + */ +public class ConcurrentMessageListenerContainerMockTests { + + @SuppressWarnings({ "rawtypes", "unchecked" }) + @Test + public void testCorrectContainerForConsumerError() throws InterruptedException { + ConsumerFactory consumerFactory = mock(ConsumerFactory.class); + final Consumer consumer = mock(Consumer.class); + AtomicBoolean first = new AtomicBoolean(true); + willAnswer(invocation -> { + if (first.getAndSet(false)) { + throw new RuntimeException("planned"); + } + Thread.sleep(100); + return new ConsumerRecords<>(Collections.emptyMap()); + }).given(consumer).poll(any()); + given(consumerFactory.createConsumer("grp", "", "-0", null)).willReturn(consumer); + ContainerProperties containerProperties = new ContainerProperties("foo"); + containerProperties.setGroupId("grp"); + containerProperties.setMessageListener((MessageListener) record -> { }); + ConcurrentMessageListenerContainer container = new ConcurrentMessageListenerContainer<>(consumerFactory, + containerProperties); + CountDownLatch latch = new CountDownLatch(1); + AtomicReference errorContainer = new AtomicReference<>(); + container.setErrorHandler((ContainerAwareErrorHandler) (thrownException, records, consumer1, ec) -> { + errorContainer.set(ec); + latch.countDown(); + }); + container.start(); + assertThat(latch.await(10, TimeUnit.SECONDS)).isTrue(); + assertThat(errorContainer.get()).isSameAs(container); + container.stop(); + } + +}