smallrye-config icon indicating copy to clipboard operation
smallrye-config copied to clipboard

@ConfigProperties Exception messages could be improved

Open Joseph-Cass opened this issue 4 years ago • 2 comments

This has been attempted in https://github.com/smallrye/smallrye-config/pull/513.

I think it would be useful to include the name of the bean causing the Exception for bad @ConfigProperties beans

In example:

Scenario Example Code Current Exception Desired Additional Information
Defining a @ConfigProperties bean with one bad field
@ConfigProperties(prefix = "validPrefix")
@Dependent
public class BadConfigPropertiesBean {
    // Where validPrefix.myString is defined as "aString"
    public int myString; 
}

io.smallrye.config.ConfigValidationException: Configuration validation failed:

java.lang.IllegalArgumentException: SRCFG00039: The config property validPrefix.myString with the config value "aString" threw an Exception whilst being converted SRCFG00029: Expected an integer value, got "aString"
The name of the bean causing the Exception. i.e. BadConfigPropertiesBean
Defining a @ConfigProperties bean with many bad fields
@ConfigProperties(prefix = "validPrefix")
@Dependent
public class BadConfigPropertiesBean {
    // Where validPrefix.myString is defined as "aString"
    public int myString;
    // Where validPrefix.missingString is not defined
    public String missingString; 
}

io.smallrye.config.ConfigValidationException: Configuration validation failed:

java.util.NoSuchElementException: SRCFG00014: The config property anotherValidPrefix.anotherString is required but it could not be found in any config source

java.lang.IllegalArgumentException: SRCFG00039: The config property validPrefix.myString with the config value "aString" threw an Exception whilst being converted SRCFG00029: Expected an integer value, got "aString"
For each failing config property, the name of the @ConfigProperties bean
Defining multiple bad @ConfigProperties beans
@ConfigProperties(prefix = "validPrefix")
@Dependent
public class BadConfigPropertiesBean {
    // Where validPrefix.myString is defined as "aString"
    public int myString;
    // Where validPrefix.missingString is not defined
    public String missingString; 
}

@ConfigProperties(prefix = "anotherValidPrefix")
@Dependent
public class AnotherBadConfigPropertiesBean {
    // Where anotherValidPrefix.anotherString 
    // is defined as "aString"
    public int anotherString; 
}

io.smallrye.config.ConfigValidationException: Configuration validation failed:

java.lang.IllegalArgumentException: SRCFG00039: The config property validPrefix.myString with the config value "aString" threw an Exception whilst being converted SRCFG00029: Expected an integer value, got "aString"

java.util.NoSuchElementException: SRCFG00014: The config property validPrefix.missingString is required but it could not be found in any config source

java.lang.IllegalArgumentException: SRCFG00039: The config property anotherValidPrefix.anotherString with the config value "aString" threw an Exception whilst being converted SRCFG00029: Expected an integer value, got "aString"
For each failing config property, in each bean, the name of the @ConfigProperties bean
Injecting a valid @ConfigProperties bean with an invalid prefix
@ConfigProperties(prefix = "validPrefix")
@Dependent
public class BadConfigPropertiesBean {
    // Where validPrefix.myString is defined as "aString"
    public String myString;
}

@RequestScoped
public class MyClass {
    @Inject
    @ConfigProperties(prefix = "InvalidPrefix")
    BadConfigPropertiesBean badBean;
}

io.smallrye.config.ConfigValidationException: Configuration validation failed:

java.util.NoSuchElementException: SRCFG00014: The config property InvalidPrefix.myString is required but it could not be found in any config source
Details about the injection point of the @ConfigProperties bean. i.e. .MyClass.badBean

Whilst writing up this issue, I've realised the Exception messages are better than I thought. I still think it would be nice to include the @ConfigProperties bean names though. The Exception message for the second to last row (Defining multiple bad @ConfigProperties beans) for example could be (I think with minimal code change):

Exception 1 :
io.smallrye.config.inject.ConfigException: SRCFG02008: Failed to create @ConfigProperties bean <classpath>.BadConfigPropertiesBean. io.smallrye.config.ConfigValidationException: Configuration validation failed:
java.lang.IllegalArgumentException: SRCFG00039: The config property validPrefix.myString with the config value "aString" threw an Exception whilst being converted SRCFG00029: Expected an integer value, got "aString"
java.util.NoSuchElementException: SRCFG00014: The config property validPrefix.missingString is required but it could not be found in any config source

Exception 2:
io.smallrye.config.inject.ConfigException: SRCFG02008: Failed to create @ConfigProperties bean <classpath>.AnotherBadConfigPropertiesBean. io.smallrye.config.ConfigValidationException: Configuration validation failed:
java.lang.IllegalArgumentException: SRCFG00039: The config property anotherValidPrefix.anotherString with the config value "aString" threw an Exception whilst being converted SRCFG00029: Expected an integer value, got "aString"

Joseph-Cass avatar May 14 '21 14:05 Joseph-Cass

All exceptions / messages in mappings are reported via io.smallrye.config.ConfigMappingContext#reportProblem. Usually there is enough contextual information around this call to provide an expressive exception message. For instance look here for validation: https://github.com/smallrye/smallrye-config/blob/7e4f7cd5fe006a8624a0aa7cb0d43964e5682c94/validator/src/main/java/io/smallrye/config/validator/BeanValidationConfigValidator.java#L173

For mappings, this is done in the ASM code: https://github.com/smallrye/smallrye-config/blob/f8c31e3c95b36a33cd48e1795eb7a23679090e8b/implementation/src/main/java/io/smallrye/config/ConfigMappingGenerator.java#L715

I little bit more tricky, but the class name and other information are available in that context. Would you like to give it a try and improve the messages?

radcortez avatar May 18 '21 15:05 radcortez

Maybe it's out of scope for this issue but another case with confusing error is incorrect env var like DB_CLIENTS__TEST__CONNECTION-STRING which translates to db.clients."test".connection-string (by StringUtil.toLowerCaseAndDotted) and leads to following error message:

Caused by: java.lang.IllegalStateException: io.smallrye.config.ConfigValidationException: Configuration validation failed:
    java.util.NoSuchElementException: SRCFG00014: The config property db.clients."test".connection-string is required but it could not be found in any config source
    at io.smallrye.config.SmallRyeConfigBuilder.build(SmallRyeConfigBuilder.java:453)
    at io.quarkus.runtime.generated.Config.readConfig(Unknown Source)
    at io.quarkus.deployment.steps.RuntimeConfigSetup.deploy(Unknown Source)
    ... 13 more
Caused by: io.smallrye.config.ConfigValidationException: Configuration validation failed:
    java.util.NoSuchElementException: SRCFG00014: The config property db.clients."test".connection-string is required but it could not be found in any config source
    at io.smallrye.config.ConfigMappingProvider.mapConfiguration(ConfigMappingProvider.java:967)
    at io.smallrye.config.ConfigMappingProvider.lambda$mapConfiguration$3(ConfigMappingProvider.java:923)
    at io.smallrye.config.SecretKeys.lambda$doUnlocked$0(SecretKeys.java:20)
    at io.smallrye.config.SecretKeys.doUnlocked(SecretKeys.java:29)
    at io.smallrye.config.SecretKeys.doUnlocked(SecretKeys.java:19)
    at io.smallrye.config.ConfigMappingProvider.mapConfiguration(ConfigMappingProvider.java:923)
    at io.smallrye.config.SmallRyeConfigBuilder.build(SmallRyeConfigBuilder.java:450)
    ... 15 more

Message says that property is required but from user point of view they should remove incorrect env var.

See https://quarkusio.zulipchat.com/#narrow/stream/187030-users/topic/SR.20Config.20complex.20map.20values.20from.20Env.2FDotEnv.20source/near/313103956.2E01

grossws avatar Nov 30 '22 17:11 grossws