Improve error handling in JsonSupport when unable to parse JSON

[eneveu]

When an exception is thrown while parsing JSON, JsonSupport catches it, logs it, and returns a JNothing: https://github.com/scalatra/scalatra/blob/v2.6.3/json/src/main/scala/org/scalatra/json/JsonSupport.scala#L48-L51

In our APIs, we would like to handle those exceptions using our own error handler (to return a specific error to the client, with a "traceId" that is also present in our logs next to the stacktrace).

Currently, we have to override JsonSupport to do so:

class JsonParseException(cause: Exception) extends RuntimeException(cause)

trait CustomJacksonJsonSupport extends JacksonJsonSupport {

  // override this to stop calling parsedBody 
  override protected def shouldParseBody(fmt: String)(implicit request: HttpServletRequest) =
    (fmt == "json" || fmt == "xml") && !request.requestMethod.isSafe

  protected def parseRequestBody(format: String)(implicit request: HttpServletRequest) = try {
    val ct = request.contentType getOrElse ""
    if (format == "json") {
      val bd = {
        if (ct == "application/x-www-form-urlencoded") multiParams.keys.headOption map readJsonFromBody getOrElse JNothing
        else if (cacheRequestBodyAsString) readJsonFromBody(request.body)
        else readJsonFromStreamWithCharset(request.inputStream, request.characterEncoding getOrElse defaultCharacterEncoding)
      }
      transformRequestBody(bd)
    } else if (format == "xml") {
      val bd = {
        if (ct == "application/x-www-form-urlencoded") multiParams.keys.headOption map readXmlFromBody getOrElse JNothing
        else if (cacheRequestBodyAsString) readXmlFromBody(request.body)
        else readXmlFromStream(request.inputStream)
      }
      transformRequestBody(bd)
    } else JNothing
  } catch {
    case t: Exception ⇒ {
      throw new JsonParseException(cause)
    }
  }

  error {
    case ex: JsonParseException ⇒
      val traceId = UUID.randomUUID()
      log.error(s"An error occurred while parsing the JSON body ($traceId)", ex)
      BadRequest(s"Invalid JSON (traceId: $traceId)")
  }

}

I think it is brittle to override so much code to handle errors, and we will need to update our CustomJacksonJsonSupport when we upgrade Scalatra. I saw that JsonSupport already changed in the 2.7.x development branch.

Is there a better way of doing what we want to achieve?

Would it be a good idea to change Scalatra's JsonSupport to throw exceptions instead of logging and silently returning JNothing?

If we do change it, how do we avoid breaking apps that depend on the current behavior ?

Maybe we could add an overridable method handleJsonParseException(ex: Exception) to let people decide what they want to do ?

scalatra version: 2.6.3
sbt version: 1.2.1