[GitHub] [kafka] C0urante commented on pull request #11130: KAFKA-13138: FileConfigProvider#get should keep failure exception

[GitBox <gi...@apache.org>]

C0urante commented on pull request #11130:
URL: https://github.com/apache/kafka/pull/11130#issuecomment-982141473


   @cmccabe @hachikuji this uses the [two-argument constructor of `ConfigException`](https://kafka.apache.org/30/javadoc/org/apache/kafka/common/config/ConfigException.html#%3Cinit%3E(java.lang.String,java.lang.Object)) as a (message, cause) pair but in reality, that constructor is supposed to accept a (name, value) pair. The usage in this PR can lead to confusing stack traces (see https://github.com/apache/kafka/pull/11349 for one example of how this type of usage looks when logged to users).
   
   I can appreciate the desire to preserve the cause of the exception for debugging purposes; would it be alright if we reverted this change to the `ConfigException` that's constructed in this case and, instead, log at `ERROR` level the causing exception before throwing our `ConfigException`? Thinking something like:
   
   ```java
           } catch (IOException e) {
               log.error("Could not read properties from file {}", path, e);
               throw new ConfigException("Could not read properties from file " + path);
           }
   ```


-- 
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-unsubscribe@kafka.apache.org

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