Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.
Comment: Address discussion comment

...

Current state: Under Discussion

Discussion thread: TODO https://lists.apache.org/thread/5gmns2pfw1gb0jvlrj13q7jd50x764x2

JIRA: TODO

Motivation

Java has a convention where classes extending Throwable accept the cause of the Throwable as a second argument to the constructor. org.apache.kafka.common.config.ConfigException  however does not follow that convention as it has a constructor ConfigException(String name, Object value), which delegates to ConfigException(String name, Object value, String message) that in turn delegates to KafkaException(String message) . This API is brittle as developers often misuse the constructor with the intention of passing a cause as the second argument which never makes it to the stacktrace [1][2][3][4]. The cause and the throwable chain often proves useful in debugging the misconfiguration. For instance, when debugging incorrectly configured TLS in [1], it can help identify the cause of misconfiguration.

...

This KIP proposes the introduction of another constructor ConfigException(String message, Throwable cause) to adhere to the convention and the addition of another constructor to capture cause.

Public Interfaces

Code Block
languagejava
titleConfigException.java
// Deprecate existing constructor
@Deprecated
public ConfigException(String name, Object value) {
	this(name, value, null);
}

// New Constructor
public ConfigException(String message, Throwable cause) {
	super(message, cause);
}

// New Constructor
public ConfigException(String name, Object value, String message, Throwable cause) {
	super("Invalid value " + value + " for configuration " + name + (message == null ? "" : ": " + message), cause);
}

// Update existing constructor
public ConfigException(String name, Object value, String message) {
	this(name, value, message, null);
}


Proposed Changes


We deprecate the existing constructor which takes an Object  as the second argument and introduce a new one which accepts a Throwable  as the cause instead ConfigException(String name, Object value)   and introduce ConfigException(String message, Throwable cause) . We also update existing usages of the former constructor as it will be marked deprecated.


We also add ConfigException(String name, Object value, String message, Throwable cause) and make ConfigException(String name, Object value, String message)  delegate to it. This will be useful in scenarios where we use the latter when catching an exception so as to not lose the underlying cause.

Compatibility, Deprecation, and Migration Plan

...