Tencent / TubeMQ

TubeMQ has been donated to the Apache Software Foundation and renamed to InLong, please visit the new Apache repository: https://github.com/apache/incubator-inlong
https://inlong.apache.org/
2.02k stars 388 forks source link

Improvement the exception type of MasterConfig throws #50

Closed ykgarfield closed 5 years ago

ykgarfield commented 5 years ago

Motivation MasterConfig load configs of master.ini, parse and check.If has problems, almost of them throws NullPointerException or IllegalArgumentException.But in some cases, throw NullPointerException probably inappropriate.For example:

try {
    this.hostName = masterConf.get("hostName").trim();
    AddressUtils.validLocalIp(this.hostName);
} catch (Throwable e) {
    throw new NullPointerException(new StringBuilder(256)
            .append("Illegal hostName value in ").append(SECT_TOKEN_MASTER)
            .append(" section!").toString());
}

IllegalArgumentException may more reasonable or use cusutom exception(for example: ConfigException).

In addition, the following exception message not clear enough:

String tmpAuthToken = masterConf.get("confModAuthToken").trim();
if (tmpAuthToken.length() > TServerConstants.CFG_MODAUTHTOKEN_MAX_LENGTH) {
    throw new IllegalArgumentException(
            "Invalid value: confModAuthToken's value > " + TServerConstants.CFG_MODAUTHTOKEN_MAX_LENGTH);
}

Here is the string length illegal, the more detail exception message should be:

"Invalid value: the length of confModAuthToken's value > " + TServerConstants.CFG_MODAUTHTOKEN_MAX_LENGTH);

(optional) Design

(optional) Example snippet

gosonzhang commented 5 years ago

@ykgarfield, this is a question. Can you contribute a PR?

gosonzhang commented 5 years ago

see https://github.com/Tencent/TubeMQ/pull/53

gosonzhang commented 5 years ago

Thanks @ykgarfield