twitter / finatra

Fast, testable, Scala services built on TwitterServer and Finagle
https://twitter.github.io/finatra/
Apache License 2.0
2.27k stars 408 forks source link

Can't override default mapper for CaseClassMappingException #363

Closed netrebel closed 8 years ago

netrebel commented 8 years ago

Not able to override the default mapper for CaseClassMappingException in the ExceptionManager.

Expected behavior

According to the documentation: "If a user wants to swap out any of these defaults they simply need add their own mapper to the manager for the exception type to map."

Actual behavior

Adding a mapper to CaseClassMappingException throws the following exception:

java.lang.IllegalStateException: ExceptionMapper for class com.twitter.finatra.json.internal.caseclass.exceptions.CaseClassMappingException already registered
    at com.twitter.finatra.http.internal.exceptions.ExceptionManager.add(ExceptionManager.scala:65)
    at com.twitter.finatra.http.internal.exceptions.ExceptionManager.add(ExceptionManager.scala:53)
    at com.twitter.finatra.http.routing.HttpRouter.exceptionMapper(HttpRouter.scala:42)

Looking at finatra source code, ExceptionManager.scala checks if the mapper already contains the Throwable and if so, it throws an Exception.

Steps to reproduce the behavior

  1. Through the HttpRouter, add a new Exception Mapper. E.g: .exceptionMapper[MyCaseClassMappingExceptionMapper]
  2. Create ExceptionMapper
class MyCaseClassMappingExceptionMapper @Inject()(response: ResponseBuilder) extends ExceptionMapper[CaseClassMappingException] with Logging {

  override def toResponse(request: Request, ex: CaseClassMappingException): Response = {
    logger.info(ex.getMessage)
    response
      .badRequest(Map(
        "code" -> 404,
        "message" -> ex.getMessage
      ))
  }
}

And run the application. Exception thrown in during initial boot sequence.

netrebel commented 8 years ago

I think this has been fixed in the commit part of the develop branch: https://github.com/twitter/finatra/commit/36afd2c4f0dfa29108e336bdd60a534a414a9a28

The comments of the commit match the documentation available.

I'm using v2.1.5, the latest available at the moment.

ryanoneill commented 8 years ago

Hi @netrebel, the latest version of Finatra is v2.5.0. That is why the documentation does not match with what you're using.

Previously, users had to override the exceptionMapperModule in their server to control this behavior, but as you found via the docs, that is no longer the case.

cacoco commented 8 years ago

@netrebel the latest available version of Finatra is version 2.5.0. Please update if you can and follow the instructions as listed in the documentation. See #346 as this is a duplicate.

cacoco commented 8 years ago

Duplicate of #346.