gensericghiro commented on code in PR #20491:
URL: https://github.com/apache/kafka/pull/20491#discussion_r2326111084
##########
clients/src/test/java/org/apache/kafka/clients/consumer/internals/AsyncKafkaConsumerTest.java:
##########
@@ -2029,6 +2030,27 @@ public void
testRecordBackgroundEventQueueSizeAndBackgroundEventQueueTime() {
assertEquals(10, (double)
metrics.metric(metrics.metricName("background-event-queue-time-max",
CONSUMER_METRIC_GROUP)).metricValue());
}
+ @Test
+ public void testFailConstructor() {
+ final Properties props = requiredConsumerConfig();
+ props.put(ConsumerConfig.GROUP_ID_CONFIG, "group-id");
Review Comment:
`groupMetadata` is always initialized
([ref](https://github.com/apache/kafka/blob/0a12eaa80e9f3ff763172f7026b0785fb31c3dbd/clients/src/main/java/org/apache/kafka/clients/consumer/internals/AsyncKafkaConsumer.java#L326)),
so my understanding was what you said, i.e. "you can ensure that the test pass
because it hits they newly added null check on appEventHandler, and not because
of the existing early return on groupMetadata.isEmpty".
> creation happens after the metrics anyways, so it will end up always being
null for this test anyways, right?
Good point. The only place we could add something after the initialization
of `groupMetadata` but before the initialization of the other handlers/invokers
that are checked for nullity is in `RequestManagers.supplier`, but that doesn't
throw any exception. Now I'm wondering how these NPEs were obtained in the
first place 🤔? Only place `groupMetadata` isn't checked for emptiness is in
`awaitPendingAsyncCommitsAndExecuteCommitCallbacks`, and
`lastPendingAsyncCommit` is always null unless there's already been an
asyncCommit, which I assume is impossible if the consumer isn't yet built?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
