Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GRpcExceptionHandler doesn't work [Kotlin] #268

Closed
TheHett opened this issue Dec 12, 2021 · 21 comments
Closed

GRpcExceptionHandler doesn't work [Kotlin] #268

TheHett opened this issue Dec 12, 2021 · 21 comments
Labels
bug Auto-generates notes
Milestone

Comments

@TheHett
Copy link

TheHett commented Dec 12, 2021

Hi, thanks for the work you've done. It's really great starter!

I have some trouble, maybe it related with Kotlin CoroutineImpl.
The interceptor below does not catch exception:

@GRpcService
class TempUrlService() : TempUrlServiceGrpcKt.TempUrlServiceCoroutineImplBase() {

    private val logger = KotlinLogging.logger {}

    @GRpcExceptionHandler
    fun anotherHandler(e: NullPointerException, scope: GRpcExceptionScope): Status {
        logger.warn { "NPE!" }
        return Status.INTERNAL
    }

    @Throws(Throwable::class)
    override suspend fun createThumbnailTempUrl(request: ThumbRequest): ThumbResponse {
        throw NullPointerException("HELLO")
    }
}

Using 4.5.10 version.

@jvmlet
Copy link
Collaborator

jvmlet commented Dec 12, 2021

Can you please try to debug to see the private handler discovered here for your TempUrlService service bean ?
The actual handler then resolved here

@TheHett
Copy link
Author

TheHett commented Dec 12, 2021

@jvmlet
My handler are presented, but method resolveMethodByThrowable isn't invoked.
I could not find the entry point where interceptor should be invoked :(

@jvmlet
Copy link
Collaborator

jvmlet commented Dec 12, 2021

for unary call it's called from here

@jvmlet
Copy link
Collaborator

jvmlet commented Dec 12, 2021

Ohh, I think the issue is this line , I should also check the privateResolvers

@jvmlet
Copy link
Collaborator

jvmlet commented Dec 12, 2021

Meanwhile, please add dummy global handler as temporary workaround to proceed here ..

@TheHett
Copy link
Author

TheHett commented Dec 13, 2021

Hi, do you mean GRpcServiceAdvice or GRpcGlobalInterceptor ?

First doesn't work too for me

image

While I wrapped in try..catch :)

@jvmlet
Copy link
Collaborator

jvmlet commented Dec 13, 2021

The problem is this line , it should check private resolvers as well. So, meanwhile, you can workaround the bug by adding dummy @GRpcServiceAdvice with @GRpcServiceAdvice method, it will force interceptor to proceed (here) and you private handler should be invoked

@TheHett
Copy link
Author

TheHett commented Dec 13, 2021

Please tell, is code ok?

Exception

class TestException(message: String) : Exception(message)

Service

@GRpcService
class FileService() : FileServiceGrpcKt.FileServiceCoroutineImplBase() {

    override suspend fun getInfo(request: FileInfoRequest): FileInfoResponse {
        throw TestException("Hello")
    }
}

Advice

@GRpcServiceAdvice
class GRpcErrorInterceptor {
    private val logger = KotlinLogging.logger {}

    @GRpcExceptionHandler
    fun throwableExceptionHandler(e: TestException, scope: GRpcExceptionScope): Status {
        logger.error(e) { "gRPC unexpected error" }
        return Status.INTERNAL.withCause(e) // should be code 13
    }
}

But no logs in application, and status differs:

hett@STORM:~ $ grpc_cli call 172.23.0.1:9090 getInfo ""
connecting to 172.23.0.1:9090
Rpc failed with status code 2, error message:

I can provide demo app.

@TheHett
Copy link
Author

TheHett commented Dec 13, 2021

Hmmm, I suspected that the problem is in the coroutines, but not. This doesn't work too:

@GRpcService()
class FileService() : FileServiceGrpc.FileServiceImplBase() {
    private val logger = KotlinLogging.logger {}
    override fun getInfo(request: FileInfoRequest, responseObserver: StreamObserver<FileInfoResponse>) {
        throw TestException("Hello")
    }
}

@jvmlet
Copy link
Collaborator

jvmlet commented Dec 13, 2021

You should have both @GRpcExceptionHandler : global and private. Your private handler will be called

@jvmlet
Copy link
Collaborator

jvmlet commented Dec 13, 2021

You also should wrap your exception with throw new GRpcRuntimeExceptionWrapper(new TestException(),"myHint"), the RuntimeException is caught...

@TheHett
Copy link
Author

TheHett commented Dec 13, 2021

Bingo, with runtime exception it is works!
The problem is that the Kotlin does not have throws signature and not differs checked/non-checked exceptions. In may case I testing java checked exception.

If declared next exception, no any logs will produced on error and it big problem:

class TestException(message: String) : java.lang.Exception(message)

The next code with runtime exception works (global handler registered too):

class TestException(message: String) : java.lang.RuntimeException(message)
@GRpcService()
class FileService() : FileServiceGrpc.FileServiceImplBase() {

    @GRpcExceptionHandler
    fun anotherHandler(e: TestException, scope: GRpcExceptionScope): Status {
        logger.warn { "error!" }
        return Status.INTERNAL
    }
    override fun getInfo(request: FileInfoRequest, responseObserver: StreamObserver<FileInfoResponse>) {
        throw TestException("Hello")
    }
}

But if trying to extend kotlin corutine impl we get silence again in the logs :(

@GRpcService()
class FileService() : FileServiceGrpcKt.FileServiceCoroutineImplBase() {
    @GRpcExceptionHandler
    fun anotherHandler(e: TestException, scope: GRpcExceptionScope): Status {
        logger.warn { "error!" }
        return Status.INTERNAL
    }
    override suspend fun getInfo(request: FileInfoRequest): FileInfoResponse {
        throw TestException("Hello")
    }
}

I suspect that it is different troubles.

@TheHett
Copy link
Author

TheHett commented Dec 13, 2021

I created demo project which reproduces both problems https://github.com/TheHett/grpc_demo/tree/master/src/main/kotlin/com/example/demo/grpc

It should be call manually because I doesn't know how to write test for this

grpc_cli call 127.0.0.1:6565  demo.api.DemoService/test ""

@jvmlet
Copy link
Collaborator

jvmlet commented Dec 13, 2021

You can have a look at demo module in this repo to get the idea how to develop unit tests

@YannPerthuis
Copy link

YannPerthuis commented Feb 28, 2022

Hi, It doesn't works for me as for @TheHett. I have checked methodResolver attribute content inside the GRpcExceptionHandlerInterceptor after its initialisation through the constructor. It contains as expected in the mapperHandlers the @GRpcExceptionHandler method defined inside @GRpcServiceAdvice classes, and each grpc services (privateResolvers) contains to their @GRpcExceptionHandler method inside its respective mapperHandlers attribute.

At runtime, when I look at this condition for check global exception handler (declared via @GRpcServiceAdvice) it's return true but when I throw the exception (runtimeEx or not) it's not cached, and when I look too the same place for local exception handler (from @GrpcService) it's return false, and btw exception are not cached too.

Ps: for test with exception, i did used the wrapper as doc advices it.

@jvmlet have you an idea about how I could work around this issue please ?

@YannPerthuis
Copy link

Any idea @jvmlet please ?

@littleniannian
Copy link

i have the same problem with kotlin, @GrpcExceptionHandler in service or @GRpcServiceAdvice on single class doesn't work

@vicmosin
Copy link

Still relevant with 4.9.1, any updates here?

@jvmlet
Copy link
Collaborator

jvmlet commented Aug 14, 2023

Fixed in latest 5.1.5-SNAPSHOT, please verify and confirm.

@jvmlet jvmlet added the bug Auto-generates notes label Aug 14, 2023
@jvmlet jvmlet added this to the 5.1.5 milestone Aug 14, 2023
@jvmlet jvmlet closed this as completed in d257633 Aug 14, 2023
@jvmlet
Copy link
Collaborator

jvmlet commented Sep 27, 2023

5.1.5 was released

@vicmosin
Copy link

vicmosin commented Oct 6, 2023

I am still getting it in 5.1.5

Maybe it's somehow interfere with the order of interceptors?

grpc.security.auth.interceptor-order=-4
grpc.recovery.interceptor-order=-2
grpc.validation.interceptor-order=-1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Auto-generates notes
Projects
None yet
Development

No branches or pull requests

5 participants