ExceptionMappingFilter not working

[ysde]

Custom ExceptionMapper is not working or ExceptionMappingFilter is not working

I'm trying mapping custom exception to a ThriftException. following the link below. https://twitter.github.io/finatra/user-guide/thrift/exceptions.html

And test it in FeatureTest. https://twitter.github.io/finatra/user-guide/testing/feature_tests.html

Finatra version: 18.7.0

Expected behavior

IllegalArgumentException can be mapped to IllegalArgumentError

@Singleton
class IllegalArgumentExceptionMapper extends ExceptionMapper[IllegalArgumentException, IllegalArgumentError] {

  def handleException(exception: IllegalArgumentException): Future[IllegalArgumentError] = {
    Future.exception(IllegalArgumentError(exception.msg))
  }
}

class MockSegmentController extends Controller with SegmentService.ServicePerEndpoint {

  override def testExceptionMethod: Service[TestExceptionMethod.Args, String] = {
    throw IllegalArgumentException("test message")
  }
}

class ExampleServer extends ThriftServer {
  override def configureThrift(router: ThriftRouter): Unit = {
    router
      .filter[ExceptionMappingFilter]
      .filter[LoggingMDCFilter]
      .filter[AccessLoggingFilter]
      .exceptionMapper[IllegalArgumentExceptionMapper]
      .add[MockSegmentController]
  }
}

Actual behavior

It seems that ExceptionMapper is not working.

I got an exception wrapped by TApplicationException:

org.apache.thrift.TApplicationException: Internal error processing testExceptionMethod: 'finatra.IllegalArgumentException: test message'

Steps to reproduce the behavior

1. Create a class extends ExceptionMapper
2. Add ExceptionMappingFilter in your router
3. Test this in FeatureTest.

I have no clues why ExceptionMapper is not working.

Thank you very much

[ryanoneill]

Hi @ysde, I'm going to try to look into this today.

[ryanoneill]

Can you provide us with some more information on what IllegalArgumentError looks like?

[ysde]

Hi @ysde, I'm going to try to look into this today.

Thank you!

Below is the definition of IllegalArgumentError of thrift

exception IllegalArgumentError {
    1: required string message
}

[cacoco]

@ysde can you also include what your service definition looks like in your thrift IDL? Thanks!

[ysde]

@ysde can you also include what your service definition looks like in your thrift IDL? Thanks!

Hi @cacoco

Below is the definition, Thanks!

service SegmentService {
    string testExceptionMethod() throws (1:InternalServerError internalServerError, 2:IllegalArgumentError illegalArgumentError)
}

[cacoco]

@ysde are you perhaps using generated code from one language in another? E.g., using generated Java code from Scala or vice versa? The exceptions in the mapper should be the generated code in the same language of the server.

[ysde]

Hi @cacoco

We use scrooge to generate code in Scala.

So the situation should not happen. Right ?

[yufangong]

Hi @ysde, the MockSegmentController defined seems odd to me, could you please take a look at the this doc. I think handle callback is needed for all the built-in filters to work. Please let us know if that helps.

[cacoco]

Ahh, @yufangong is correct. I didn't look at your Controller code -- which is incorrect in several ways. No Finatra filtering will work if you do not use the handle function in your Controller.

As the documentation says:

The handle(ThriftMethod) function may seem magical but it serves an important purpose. By implementing your service method via this function, it allows the framework to apply the configured global filter chain defined in your server definition to your method implementation (passed as the callback to handle(ThriftMethod)).

That is to say, the handle(ThriftMethod) function captures filters that you apply to that particular method plus your method implementation and then exposes it for the ThriftRouter to combine with the configured global filter chain to build the Finagle Service that represents your server.

Additionally, you are not meant to implement the ServicePerEndpoint if you are following the legacy style; you should be extending the BaseServiceIface as documented in the link.

Thanks!

[ysde]

Hi @cacoco @yufangong

1. Add handle()
2. Change override def testExceptionMethod to override val testExceptionMethod

Then it worked.

These two changes are necessary

Thank you