vamossagar12 commented on code in PR #16418:
URL: https://github.com/apache/kafka/pull/16418#discussion_r1705300148


##########
clients/src/main/java/org/apache/kafka/common/network/Selector.java:
##########
@@ -350,6 +350,10 @@ private KafkaChannel 
buildAndAttachKafkaChannel(SocketChannel socketChannel, Str
             // Ideally, these resources are closed by the KafkaChannel but if 
the KafkaChannel is not created to an
             // error, this builder should close the resources it has created 
instead.
             Utils.closeQuietly(metadataRegistry, "metadataRegistry");
+            // If an Authentication Failure is encountered, throw that error 
directly.
+            if (e instanceof AuthenticationException) {
+                throw e;

Review Comment:
   @rajinisivaram , i made the changes as you suggested (added UT's as well). 
Regarding the Integration test, I tried adding them in 
`kafka.api.SaslClientsWithInvalidCredentialsTest` but I am not able to 
replicate the exact scenario. In the producer test 
`testProducerWithAuthenticationFailure` in that class, sending the record fails 
till we don't setup the client credentials. I have been wanting to add a test 
where the credentials expire and the producer also fails immediately(instead of 
the current behaviour of blocking for max.block.ms). Is there any existing test 
which mimics this ?



-- 
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: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to