problem-spring-web icon indicating copy to clipboard operation
problem-spring-web copied to clipboard

Return Bad Request for Jackson client exceptions

Open darioseidl opened this issue 3 years ago • 2 comments

The problem-spring-web library already handles many Java and Spring exceptions out-of-the-box, which is great. In any project where we're using it, we added some additional advice traits for common exceptions. Maybe it would make sense to handle them directly in the library?

Jackson parsing and input exceptions:

  • MismatchedInputException

General exception type used as the base class for all {@link JsonMappingException}s that are due to input not mapping to target definition; these are typically considered "client errors" since target type definition itself is not the root cause but mismatching input. This is in contrast to {@link InvalidDefinitionException} which signals a problem with target type definition and not input.

so this exception should clearly return a 400 status code.

  • JsonParseException

Exception type for parsing problems, used when non-well-formed content (content that does not conform to JSON syntax as per specification) is encountered.

this happens on invalid JSON, so that should also return a 400 status code.

We're handling this as such (in Kotlin),

/**
 * Return [Status.BAD_REQUEST] for exceptions indicating bad requests.
 */
@ControllerAdvice
@Order(0)
class BadRequestAdvice : AdviceTrait {

    /**
     * Handle [JsonParseException]s, thrown when JSON cannot be parsed because of invalid syntax.
     */
    @ExceptionHandler
    fun handle(exception: JsonParseException, request: NativeWebRequest): ResponseEntity<Problem> =
        create(Status.BAD_REQUEST, exception, request)
    
    /**
     * Handle [MismatchedInputException]s, thrown when JSON input cannot be mapped to the target type.
     */
    @ExceptionHandler
    fun handle(exception: MismatchedInputException, request: NativeWebRequest): ResponseEntity<Problem> =
        create(Status.BAD_REQUEST, exception, request)

}

I'm not quite sure if there are other Jackson exceptions that indicate client errors, but I believe those two cover most cases.

If it makes sense to add that, I could provide a pull request.

darioseidl avatar Dec 15 '21 07:12 darioseidl

Yeah, that sounds useful. Do we already have a dependency on Jackson? If not, then I would add an optional dependency and offer an advice trait à la carte, i.e. one that isn't part of e.g. ProblemHandling.

whiskeysierra avatar Dec 15 '21 08:12 whiskeysierra

There's a dependency on jackson-databind already (not optional, as far as I can see).

darioseidl avatar Dec 16 '21 14:12 darioseidl