Invalid targetType in KotlinInvalidNullException

[damianmalczewski]

Search before asking

• I searched in the issues and found nothing similar.
• I have confirmed that the same problem is not reproduced if I exclude the KotlinModule.
• I searched in the issues of databind and other modules used and found nothing similar.
• I have confirmed that the problem does not reproduce in Java and only occurs when using Kotlin and KotlinModule.

Describe the bug

While deserializing objects that have non-nullable fields, the exception thrown behaves differently, depending on what is the underlying type.

If there's an underlying Java primitive, then a TypeMismatchException from jackson-databind allows to retrieve what was the type of the field.

// this is a modified snippet from KotlinInvalidNullExceptionTest.kt

private data class IntDto(
    val foo: Int
)

class KotlinInvalidNullExceptionTest {
    @Test
    fun fooTest() {
        val json = """{"bar":"bar"}"""

        val ex2 = assertThrows<MismatchedInputException> { defaultMapper.readValue<IntDto>(json) }
        assertEquals("foo", ex2.path.map { it.propertyName }.joinToString("."))
        assertEquals(Int::class, ex2.targetType.kotlin)
        // notice that here targetType returns Int
    }
}

However, when I'm deserializing a type that is backed by a non-primitive, then a KotlinInvalidNullException is thrown and getTargetType() returns type of a DTO object, not type of a field.

This is even tested currently in KotlinInvalidNullExceptionTest.kt:

private data class Dto(
    val foo: String,
    @JsonProperty("bar")
    val _bar: String
)

class KotlinInvalidNullExceptionTest {
    @Test
    fun fooTest() {
        val json = """{"bar":"bar"}"""
        val ex = assertThrows<KotlinInvalidNullException> { defaultMapper.readValue<Dto>(json) }

        assertEquals("foo", ex.kotlinPropertyName)
        assertEquals("foo", ex.propertyName.simpleName)
        assertEquals(Dto::class, ex.targetType.kotlin)
        // notice here that targetType returns Dto
    }
}

To Reproduce

This is a modified snippet from currently existing tests.

private data class Dto(
    val foo: String,
    @JsonProperty("bar")
    val _bar: String
)

private data class IntDto(
    val foo: Int
)

class KotlinInvalidNullExceptionTest {
    @Test
    fun fooTest() {
        val json = """{"bar":"bar"}"""
        val ex = assertThrows<KotlinInvalidNullException> { defaultMapper.readValue<Dto>(json) }

        assertEquals("foo", ex.kotlinPropertyName)
        assertEquals("foo", ex.propertyName.simpleName)
        assertEquals("foo", ex.path.map { it.propertyName }.joinToString("."))

        // I think this should be String::class, because the property is of type String
        assertEquals(Dto::class, ex.targetType.kotlin)

        val ex2 = assertThrows<MismatchedInputException> { defaultMapper.readValue<IntDto>(json) }
        assertEquals("foo", ex2.path.map { it.propertyName }.joinToString("."))
        assertEquals(Int::class, ex2.targetType.kotlin)
    }
}

Expected behavior

Because one exception inherits from another, I believe that it should also provide information in the same way - the information about field type, not the deserialized object.

Versions

Kotlin:
Jackson-module-kotlin: noticed on 3.0.4, tested also on current 3.x branch
Jackson-databind: noticed on 3.0.4, tested also on current 3.x branch

Additional context

I think the easy fix would be to change this in KotlinValueInstantiator, by replacing this.valueClass with paramType.javaType as Class<*>:

if (isMissingAndRequired || (!paramType.isMarkedNullable && !paramType.isGenericTypeVar())) {
    throw KotlinInvalidNullException(
        paramDef.name,
        // this.valueClass,  // this is currently
        paramType.javaType as Class<*>,  // this makes test return String::class instead of Dto::class
        ctxt.parser,
        "Instantiation of ${this.valueTypeDesc} value failed for JSON property $pname due to missing (therefore NULL) value for creator parameter ${paramDef.name} which is a non-nullable type",
        jsonProp.fullName,
    ).wrapWithPath(this.valueClass, pname)
}

Sorry for not raising a PR for this, as I'm not exactly a Kotlin expert and I'm not sure about that as Class<*> casting if it wouldn't break something else 😅

[k163377]

First, changing the information passed to KotlinInvalidNullException would constitute a breaking change, so it cannot be done.
I don’t recall the exact reason for setting valueClass to _targetType, but I’m sure there was a valid reason for it.

Additionally, the difference in how primitive types are handled is related to databind’s DeserializationFeature.FAIL_ON_NULL_FOR_PRIMITIVES, and it is difficult for kotlin-module to intervene in this.

At the very least, it is possible to understand the errors that are currently occurring to some extent, so I do not intend to make any active changes regarding this issue.