CODEWITHSUNDEEP
javaapiexceptionjava-8jax-rs
NewCoder2015
NewCoder2015

Reputation: 47

Throw BadRequestException(ResponseEntity) vs Catch Error, return ResponseEntity(HTTPStatus.BadRequest) ReST API

I'm working on refactoring some code which is a larger scale of the example below. The 'main' class is very cluttered with these try/catch blocks and obscures what the code is doing.

I'm not working with Spring but using JaxRs to handle exceptions. This would be the ReST entry to the service or controller, but we're also doing our doa processes here (I know, but just how it is). So we need to return a ResponseEntity with the required info.

public restVerifyName(userId) {
    String name;
    try {
        string name = nameProvider.getName(userId)
    } catch (exception A) {
        return new errorResponseBuilder(errorCode, errorMessage, status);
    } catch (exception B) {
        return new errorResponseBuilder(errorCode, errorMessage, status);
    }

    if (name == null) {
      return new errorResponseBuilder(errorCode, errorMessage, status);
     }

    try {
        nameAuthenticator.verifyName(name)
    } catch (Exception B) {
        return new errorResponseBuilder(errorCode, errorMessage, status);
    }
    Return Response.Ok().entity(name);
}

private errorResponseBuilder(errorCode, errorMessage, status) {
    ErrorResponse errorResponse = errorResponseBuilder(errorCode, errorMessage)
    return new Response.status(status).entity(errorResponse);
}

So I want to pull these try/catches into private methods and have it all be a bit more self documenting. My replacement is roughly:

public restVerifyName() {
    String name = getName();
    if (!nameIsVerfied(name)  {
         return new errorResponseBuilder(errorCode, errorMessage);
    }
    Return Response.Ok().entity(name);
}

private String getName() {
    String name;
    try {
        name = nameProvider.getName()
    } catch (exception A) {
        return new errorResponseBuilder(errorCode, errorMessage);
    } catch (exception B) {
        return new errorResponseBuilderConflict(errorCode, errorMessage);
    }

    if (name == null) {
     return new errorResponseBuilderConflict(errorCode, errorMessage);
    }
    return name;
}

private boolean verifyName(name) {
    try {
        return nameAuthenticator.verifyName(name)
    } catch (Exception B) {
        return new errorResponseBuilderBadRequest(errorCode, errorMessage);
    }
    return false;
}


private errorResponseBuilderBadRequest(errorCode, errorMessage) {
    ErrorResponse errorResponse = errorResponseBuilder(errorCode, errorMessage)
    ResponseEntity response = Response.status(status).entity(errorResponse)
    throw new BadRequestException(response)
}

private errorResponseBuilderConflict(errorCode, errorMessage) {
    ErrorResponse errorResponse = errorResponseBuilder(errorCode, errorMessage)
    ResponseEntity response = Response.status(status).entity(errorResponse)
    throw new ConfictException(response)
}

The response returned is the exact same in both stations.

Regardless of any logic errors/formatting etc. Which general approach is best practice? Throwing exceptions with responses, or returning responses (in context of this examples refactor goal)

This is on a larger scale with a few more try/catches, so I feel removing clutter from the 'main' class is more readable. Is throwing exceptions with responseEntities and letting JaxRs handle it frowned upon?

Thanks

Upvotes: 1

Views: 3641

Answers (1)

Mạnh Quyết Nguyễn
Mạnh Quyết Nguyễn

Reputation: 18235

The general approach is to throws the exception with necessary embedded detail.

Then have a global exception handler to transform the exception into response.

For Jax-RS you can use ExceptionMapper to catch the exceptions.

private String getName() {
    String name;
    //try {
        name = nameProvider.getName()
    // Only catch if exception is a checked exception or you need to transform the exception to another type with embedded detail
    // Runtime exception should generally not need to be caught here
    //} catch (exception A) {
    //    return new errorResponseBuilder(errorCode, errorMessage);
    //} catch (exception B) {
    //    return new errorResponseBuilderConflict(errorCode, errorMessage);
    //}

    if (name == null) {
       throw new ACustomErrorExceptionForThisKind(name);
    }
    return name;
}

public class ACustomErrorExceptionForThisKindMapper implements ExceptionMapper<E extends Throwable> {
   public Response toResponse(ACustomErrorExceptionForThisKind e) {
        ....
   }
}

Upvotes: 2

Related Questions