spring-projects / spring-framework

Spring Framework
https://spring.io/projects/spring-framework
Apache License 2.0
56.14k stars 37.95k forks source link

java.lang.NoClassDefFoundError: org/reactivestreams/Publisher in suspending `@EventListener` #33203

Closed cmdjulian closed 2 weeks ago

cmdjulian commented 1 month ago

Hey, I have a problem using my kotlin spring boot app. When using mvc stack and defining a suspending EventListener method, my app just crashes with a class not found exception.

@SpringBootApplication
class DemoApplication

fun main(args: Array<String>) {
    runApplication<DemoApplication>(*args)
}

@Component
class KoEventListener {
    private val flow = MutableSharedFlow<ApplicationEvent>()

    @EventListener
    suspend fun onEvent(event: ApplicationStartedEvent) {
        flow.emit(event)
    }
}

Caused by: java.lang.NoClassDefFoundError: org/reactivestreams/Publisher.

I created a reproducer: event-listener-error.zip.

The problem exists within the file org.springframework.core.CoroutinesUtils. Wouldn't it make more sense in favor of using reactive types here to rely up on Future? Both reactive stack as well as kotlin have integrations with Javas Future class. It seems to me that this is the most interoperable.

I can fix it by including project reactor as an explicit dependency, but I think this should rather be taken care of by the framework. As it's quite overkill to include project reactor into the mvc stack just for that, I would vouche for using Futures instead.

What are your thoughts?

sdeleuze commented 1 month ago

Reactor mandatory dependency for the Coroutines support is at least for now on purpose and documented as in current implementation, suspending functions translate to Mono. Notice Coroutines is not just about suspending function, but also Flow which translates to Flux and the global design which is based on org.springframework.core.ReactiveAdapter.

It could be possible theoretically to use a more streamlined arrangement with suspending function with a translation to CompletableFuture provided in Coroutines JVM support or using kotlinx-coroutines-reactive instead of kotlinx-coroutines-reactor, in Spring MVC maybe that could make sense. That said, we would lose the Reactor to Coroutines context interop, to be analyzed if that would be an issue or not.

Any thoughts @simonbasle @rstoyanchev?

simonbasle commented 1 month ago

I'm not convinced removing the reactor bridge would be a good thing, it would most probably remove the context interop unless I'm missing something

sdeleuze commented 2 weeks ago

I guess we may have to use something like what is documented in https://kotlinlang.org/docs/coroutine-context-and-dispatchers.html#thread-local-data and https://kotlinlang.org/api/kotlinx.coroutines/kotlinx-coroutines-core/kotlinx.coroutines/as-context-element.html, but not sure yet if that makes sense. We may have to think about the various use cases.

sdeleuze commented 2 weeks ago

After a team discussion, we decided to not follow-up on this issue as this requirement is clearly documented and such change would bring limited benefits, and would create additional challenges like on context management.