Guice AOP is not compatible with Kotlin Coroutines

[ktabouguia-sq]

Similar to spring-projects/spring-framework#22462

1. the suspend fun may return COROUTINE_SUSPENDED
2. Guice AOP needs to filter COROUTINE_SUSPENDED because the fun did not actually return

Would appreciate if there could be a similar Guice solution for Coroutines + Guice

[sameb]

Any possible fix will be greatly accelerated by a test case that exhibits the problem (and would theoretically pass if the issue were fixed).

[ktabouguia-sq]

Hello - Here is some sample code with a test that fails when intercepting suspending functions in Kotlin

package interceptors

import com.google.inject.Guice
import com.google.inject.Injector
import com.google.inject.matcher.Matchers
import java.io.InterruptedIOException
import javax.inject.Inject
import javax.inject.Singleton
import kotlinx.coroutines.async
import kotlinx.coroutines.delay
import kotlinx.coroutines.runBlocking
import misk.inject.KAbstractModule
import misk.inject.getInstance
import org.aopalliance.intercept.MethodInterceptor
import org.aopalliance.intercept.MethodInvocation
import org.assertj.core.api.Assertions.assertThat
import org.junit.jupiter.api.BeforeEach
import org.junit.jupiter.api.Test

var successfulInterceptedCallCount: Int = 0
var failedInterceptedCallCount: Int = 0

class ClientMetricsInterceptorTest {
  private lateinit var dinosaur: Dinosaur
  private lateinit var clientMetricsInterceptor: ClientMetricsInterceptor
  private lateinit var injector: Injector

  @BeforeEach
  fun before() {
    injector = Guice.createInjector(
      object : KAbstractModule() {
        override fun configure() {
          bindInterceptor(
            Matchers.any(),
            Matchers.annotatedWith(WithMetrics::class.java),
            ClientMetricsInterceptor(),
          )

          // This makes the Dinosaur class a managed bean
          // so that it can support method interception.
          requestStaticInjection(Dinosaur::class.java)
        }
      }
    )
    dinosaur = injector.getInstance()
    clientMetricsInterceptor = injector.getInstance()
  }

  @Test
  fun `failure metrics published on interrupt`() {
    runBlocking {
      (1..100).map {
        async {
          try {
            dinosaur.interceptedMethod(true)
          } catch (e: InterruptedIOException) {
            // exception caught!
          }
        }
      }
    }

    assertThat(successfulInterceptedCallCount).isEqualTo(0)
    assertThat(failedInterceptedCallCount).isEqualTo(100)
  }
}

interface Animal {
  suspend fun interceptedMethod(interrupt: Boolean)
}

open class Dinosaur @Inject constructor() : Animal {
  @WithMetrics("dinosaur")
  override suspend fun interceptedMethod(
    interrupt: Boolean,
  ) {
    delay(10)
    if (interrupt) {
      throw InterruptedIOException()
    }
  }
}

@Singleton class ClientMetricsInterceptor @Inject constructor() : MethodInterceptor {
  override fun invoke(invocation: MethodInvocation?) = invocation?.let {
    try {
      it.proceed().also { successfulInterceptedCallCount++ }
    } catch (e: InterruptedIOException) {
      failedInterceptedCallCount++
      throw e
    }
  }
}

@Retention(AnnotationRetention.RUNTIME)
@Target(AnnotationTarget.FUNCTION, AnnotationTarget.CLASS)
annotation class WithMetrics(
  val clientName: String,
)

[lobaorn]

Hey @ktabouguia-sq , letting you know since you may be targeting Guice 7.0.0, about the new Micronaut Guice project, that depending on your usecase could be a useful replacement: micronaut-projects/micronaut-guice#9

[lobaorn]

Additionally, seems that there have already been similar problems with AOP as well and fixed (as it seems): micronaut-projects/micronaut-core#5301 , micronaut-projects/micronaut-core#7101 , micronaut-projects/micronaut-core#8891 perhaps it would help your use case @ktabouguia-sq