AsyncWaitStrategyFactoryConfig - potential NPE when creating programatically

[JWT007]

AsyncWaitStrategyFactoryConfig (Log4j 2.24.1)

This @Plugin doesn't behave like most others where if the configuration is invalid, it will return a null from the Builder.

The @PluginBuilderAttribute factoryClassName (for the XML class attribute) is marked as @Required, but if creating programatically the missing value is not checked and the constructor will throw a NPE.

@Override
public AsyncWaitStrategyFactoryConfig build() {
    return new AsyncWaitStrategyFactoryConfig(factoryClassName);
}

public AsyncWaitStrategyFactoryConfig(final String factoryClassName) {
    this.factoryClassName = Objects.requireNonNull(factoryClassName, "factoryClassName");
}

Typical behaviour here would be to return null from the build() method if misconfigured.

• adding a NotEmpty check to the Builder#withFactoryClassName(...) method might be sensible.
• adding a NotEmpty check to the constructor might be sensible (an empty string here makes no sense)

[JWT007]

@ppkarwasz I am not sure why the bot didn't mark this as "waiting for maintainer" - not sure if it pops up for you?

[ppkarwasz]

I am seriously considering deprecating AsyncWaitStrategyFactoryConfig. I will discuss it with the other maintainers (especially @remkop, who introduced it), but it could probably be replaced with the log4j2.asyncLoggerConfigWaitStrategy Log4j configuration property.

The presence of both a Log4j configuration property and a direct child of <Configuration> IMHO needlessly complexifies the logic to determine which waiting strategy to use. In main we have a simple DI system and the WaitStrategy beans are the only ones that need a Configuration parameter.

[ppkarwasz]

I am not sure why the bot didn't mark this as "waiting for maintainer" - not sure if it pops up for you?

You are a CONTRIBUTOR now! 😉
The labels are pretty much a work in progress and "waiting for maintainer" is mostly there to make a triage and ask the user some basic questions. Your issues usually don't need more information, so you can consider them accepted by default.