Fix exception handling

hapi-fhir-jaxrsserver-base/src/main/java/ca/uhn/fhir/jaxrs/server/AbstractJaxRsResourceProvider.java

@@ -22,8 +22,6 @@ package ca.uhn.fhir.jaxrs.server;
 
 import java.io.IOException;
 import java.net.URL;
-import java.util.Collections;
-import java.util.List;
 
 import javax.interceptor.Interceptors;
 import javax.ws.rs.Consumes;
@@ -41,7 +39,6 @@ import org.hl7.fhir.instance.model.api.IBaseResource;
 
 import ca.uhn.fhir.context.FhirContext;
 import ca.uhn.fhir.jaxrs.server.interceptor.JaxRsExceptionInterceptor;
-import ca.uhn.fhir.jaxrs.server.interceptor.JaxRsResponseException;
 import ca.uhn.fhir.jaxrs.server.util.JaxRsMethodBindings;
 import ca.uhn.fhir.jaxrs.server.util.JaxRsRequest;
 import ca.uhn.fhir.jaxrs.server.util.JaxRsRequest.Builder;
@@ -60,8 +57,7 @@ import ca.uhn.fhir.rest.server.IRestfulServer;
  * @author Peter Van Houte | peter.vanhoute@agfa.com | Agfa Healthcare
  */
 @Produces({ MediaType.APPLICATION_JSON, MediaType.APPLICATION_XML, MediaType.TEXT_PLAIN })
-@Consumes({ MediaType.APPLICATION_FORM_URLENCODED, MediaType.APPLICATION_JSON, Constants.CT_FHIR_JSON,
+@Consumes({ MediaType.APPLICATION_FORM_URLENCODED, MediaType.APPLICATION_JSON, Constants.CT_FHIR_JSON, Constants.CT_FHIR_XML })
-		Constants.CT_FHIR_XML })
 @Interceptors(JaxRsExceptionInterceptor.class)
 public abstract class AbstractJaxRsResourceProvider<R extends IBaseResource> extends AbstractJaxRsProvider
 
@@ -83,12 +79,11 @@ public abstract class AbstractJaxRsResourceProvider<R extends IBaseResource> ext
      * Provides the ability to specify the {@link FhirContext}.
      * @param ctx the {@link FhirContext} instance.
      */
-    protected AbstractJaxRsResourceProvider(FhirContext ctx) {
+    protected AbstractJaxRsResourceProvider(final FhirContext ctx) {
         super(ctx);
         theBindings = JaxRsMethodBindings.getMethodBindings(this, getClass());
     }
 
-
     /**
      * This constructor takes in an explicit interface class. This subclass
      * should be identical to the class being constructed but is given
@@ -97,7 +92,7 @@ public abstract class AbstractJaxRsResourceProvider<R extends IBaseResource> ext
      * 
      * @param theProviderClass the interface of the class
      */
-	protected AbstractJaxRsResourceProvider(Class<? extends AbstractJaxRsProvider> theProviderClass) {
+    protected AbstractJaxRsResourceProvider(final Class<? extends AbstractJaxRsProvider> theProviderClass) {
         super();
         theBindings = JaxRsMethodBindings.getMethodBindings(this, theProviderClass);
     }
@@ -111,7 +106,7 @@ public abstract class AbstractJaxRsResourceProvider<R extends IBaseResource> ext
      * @param ctx the {@link FhirContext} instance.
      * @param theProviderClass the interface of the class
      */
-    protected AbstractJaxRsResourceProvider(FhirContext ctx, Class<? extends AbstractJaxRsProvider> theProviderClass) {
+    protected AbstractJaxRsResourceProvider(final FhirContext ctx, final Class<? extends AbstractJaxRsProvider> theProviderClass) {
         super(ctx);
         theBindings = JaxRsMethodBindings.getMethodBindings(this, theProviderClass);
     }
@@ -127,7 +122,8 @@ public abstract class AbstractJaxRsResourceProvider<R extends IBaseResource> ext
     public String getBaseForRequest() {
         try {
             return new URL(getUriInfo().getBaseUri().toURL(), getResourceType().getSimpleName()).toExternalForm();
-		} catch (Exception e) {
+        }
+        catch (final Exception e) {
             // cannot happen
             return null;
         }
@@ -141,7 +137,8 @@ public abstract class AbstractJaxRsResourceProvider<R extends IBaseResource> ext
      * @see <a href="https://www.hl7.org/fhir/http.html#create">https://www.hl7. org/fhir/http.html#create</a>
      */
     @POST
-	public Response create(final String resource) throws IOException {
+    public Response create(final String resource)
+            throws IOException {
         return execute(getResourceRequest(RequestTypeEnum.POST, RestOperationTypeEnum.CREATE).resource(resource));
     }
 
@@ -153,7 +150,8 @@ public abstract class AbstractJaxRsResourceProvider<R extends IBaseResource> ext
      */
     @POST
     @Path("/_search")
-	public Response searchWithPost() throws IOException {
+    public Response searchWithPost()
+            throws IOException {
         return execute(getResourceRequest(RequestTypeEnum.POST, RestOperationTypeEnum.SEARCH_TYPE));
     }
 
@@ -164,7 +162,8 @@ public abstract class AbstractJaxRsResourceProvider<R extends IBaseResource> ext
      * @see <a href="https://www.hl7.org/fhir/http.html#search">https://www.hl7.org/fhir/http.html#search</a>
      */
     @GET
-	public Response search() throws IOException {
+    public Response search()
+            throws IOException {
         return execute(getResourceRequest(RequestTypeEnum.GET, RestOperationTypeEnum.SEARCH_TYPE));
     }
 
@@ -175,7 +174,8 @@ public abstract class AbstractJaxRsResourceProvider<R extends IBaseResource> ext
      * @see <a href="https://www.hl7.org/fhir/http.html#update">https://www.hl7.org/fhir/http.html#update</a>
      */
     @PUT
-    public Response conditionalUpdate(final String resource) throws IOException {
+    public Response conditionalUpdate(final String resource)
+            throws IOException {
         return execute(getResourceRequest(RequestTypeEnum.PUT, RestOperationTypeEnum.UPDATE).resource(resource));
     }
 
@@ -189,7 +189,8 @@ public abstract class AbstractJaxRsResourceProvider<R extends IBaseResource> ext
      */
     @PUT
     @Path("/{id}")
-	public Response update(@PathParam("id") final String id, final String resource) throws IOException {
+    public Response update(@PathParam("id") final String id, final String resource)
+            throws IOException {
         return execute(getResourceRequest(RequestTypeEnum.PUT, RestOperationTypeEnum.UPDATE).id(id).resource(resource));
     }
 
@@ -200,7 +201,8 @@ public abstract class AbstractJaxRsResourceProvider<R extends IBaseResource> ext
      * @see <a href="https://www.hl7.org/fhir/http.html#delete">https://www.hl7.org/fhir/http.html#delete</a>
      */
     @DELETE
-    public Response delete() throws IOException {
+    public Response delete()
+            throws IOException {
         return execute(getResourceRequest(RequestTypeEnum.DELETE, RestOperationTypeEnum.DELETE));
     }
 
@@ -213,7 +215,8 @@ public abstract class AbstractJaxRsResourceProvider<R extends IBaseResource> ext
      */
     @DELETE
     @Path("/{id}")
-	public Response delete(@PathParam("id") final String id) throws IOException {
+    public Response delete(@PathParam("id") final String id)
+            throws IOException {
         return execute(getResourceRequest(RequestTypeEnum.DELETE, RestOperationTypeEnum.DELETE).id(id));
     }
 
@@ -226,7 +229,8 @@ public abstract class AbstractJaxRsResourceProvider<R extends IBaseResource> ext
      */
     @GET
     @Path("/{id}")
-	public Response find(@PathParam("id") final String id) throws IOException {
+    public Response find(@PathParam("id") final String id)
+            throws IOException {
         return execute(getResourceRequest(RequestTypeEnum.GET, RestOperationTypeEnum.READ).id(id));
     }
 
@@ -241,9 +245,10 @@ public abstract class AbstractJaxRsResourceProvider<R extends IBaseResource> ext
      * @return the response
      * @see <a href="https://www.hl7.org/fhir/operations.html">https://www.hl7.org/fhir/operations.html</a>
      */
-	protected Response customOperation(final String resource, RequestTypeEnum requestType, String id,
+    protected Response customOperation(final String resource, final RequestTypeEnum requestType, final String id,
-			String operationName, RestOperationTypeEnum operationType) throws IOException {
+            final String operationName, final RestOperationTypeEnum operationType)
-		Builder request = getResourceRequest(requestType, operationType).resource(resource).id(id);
+            throws IOException {
+        final Builder request = getResourceRequest(requestType, operationType).resource(resource).id(id);
         return execute(request, operationName);
     }
 
@@ -259,8 +264,7 @@ public abstract class AbstractJaxRsResourceProvider<R extends IBaseResource> ext
     @Path("/{id}/_history/{version}")
     public Response findHistory(@PathParam("id") final String id, @PathParam("version") final String version)
             throws IOException {
-		Builder theRequest = getResourceRequest(RequestTypeEnum.GET, RestOperationTypeEnum.VREAD).id(id)
+        final Builder theRequest = getResourceRequest(RequestTypeEnum.GET, RestOperationTypeEnum.VREAD).id(id).version(version);
-				.version(version);
         return execute(theRequest);
     }
 
@@ -275,10 +279,10 @@ public abstract class AbstractJaxRsResourceProvider<R extends IBaseResource> ext
      */
     @GET
     @Path("/{id}/{compartment}")
-	public Response findCompartment(@PathParam("id") final String id,
+    public Response findCompartment(@PathParam("id") final String id, @PathParam("compartment") final String compartment)
-			@PathParam("compartment") final String compartment) throws IOException {
+            throws IOException {
-		Builder theRequest = getResourceRequest(RequestTypeEnum.GET, RestOperationTypeEnum.SEARCH_TYPE).id(id)
+        final Builder theRequest = getResourceRequest(RequestTypeEnum.GET, RestOperationTypeEnum.SEARCH_TYPE).id(id).compartment(
-				.compartment(compartment);
+                compartment);
         return execute(theRequest, compartment);
     }
 
@@ -289,13 +293,15 @@ public abstract class AbstractJaxRsResourceProvider<R extends IBaseResource> ext
      * @param methodKey the key determining the method to be executed
      * @return the response
      */
-	private Response execute(Builder theRequestBuilder, String methodKey) throws IOException {
+    private Response execute(final Builder theRequestBuilder, final String methodKey)
-		JaxRsRequest theRequest = theRequestBuilder.build();
+            throws IOException {
-		BaseMethodBinding<?> method = getBinding(theRequest.getRestOperationType(), methodKey);		
+        final JaxRsRequest theRequest = theRequestBuilder.build();
+        final BaseMethodBinding<?> method = getBinding(theRequest.getRestOperationType(), methodKey);
         try {
             return (Response) method.invokeServer(this, theRequest);
-		} catch (JaxRsResponseException theException) {
+        }
-			return new JaxRsExceptionInterceptor().convertExceptionIntoResponse(theRequest, theException);
+        catch (final Throwable theException) {
+            return handleException(theRequest, theException);
         }
     }
 
@@ -305,7 +311,8 @@ public abstract class AbstractJaxRsResourceProvider<R extends IBaseResource> ext
      * @param theRequestBuilder the requestBuilder that contains the information about the request
      * @return the response
      */
-	private Response execute(Builder theRequestBuilder) throws IOException {
+    private Response execute(final Builder theRequestBuilder)
+            throws IOException {
         return execute(theRequestBuilder, JaxRsMethodBindings.DEFAULT_METHOD_KEY);
     }
 
@@ -316,7 +323,7 @@ public abstract class AbstractJaxRsResourceProvider<R extends IBaseResource> ext
      * @param theBindingKey the key determining the method to be executed (needed for e.g. custom operation)
      * @return
      */
-	protected BaseMethodBinding<?> getBinding(RestOperationTypeEnum restOperation, String theBindingKey) {
+    protected BaseMethodBinding<?> getBinding(final RestOperationTypeEnum restOperation, final String theBindingKey) {
         return getBindings().getBinding(restOperation, theBindingKey);
     }
 

hapi-fhir-jaxrsserver-base/src/main/java/ca/uhn/fhir/jaxrs/server/interceptor/JaxRsExceptionInterceptor.java

@@ -41,7 +41,7 @@ import ca.uhn.fhir.rest.server.interceptor.ExceptionHandlingInterceptor;
 public class JaxRsExceptionInterceptor {
 
     /** the existing exception handler which is able to convert exception into responses*/
-    private ExceptionHandlingInterceptor exceptionHandler;
+    private final ExceptionHandlingInterceptor exceptionHandler;
 
     /**
      * The default constructor
@@ -54,7 +54,7 @@ public class JaxRsExceptionInterceptor {
      * A utility constructor for unit testing
      * @param exceptionHandler the handler for the exception conversion
      */
-    JaxRsExceptionInterceptor(ExceptionHandlingInterceptor exceptionHandler) {
+    JaxRsExceptionInterceptor(final ExceptionHandlingInterceptor exceptionHandler) {
         this.exceptionHandler = exceptionHandler;
     }
 
@@ -65,11 +65,13 @@ public class JaxRsExceptionInterceptor {
      * @throws JaxRsResponseException an exception that can be handled by a jee container
      */
     @AroundInvoke
-    public Object intercept(final InvocationContext ctx) throws JaxRsResponseException {
+    public Object intercept(final InvocationContext ctx)
+            throws JaxRsResponseException {
         try {
             return ctx.proceed();
-        } catch(final Exception theException) {
+        }
-        	AbstractJaxRsProvider theServer = (AbstractJaxRsProvider) ctx.getTarget();
+        catch (final Exception theException) {
+            final AbstractJaxRsProvider theServer = (AbstractJaxRsProvider) ctx.getTarget();
             throw convertException(theServer, theException);
         }
     }
@@ -84,8 +86,8 @@ public class JaxRsExceptionInterceptor {
         if (theServer.withStackTrace()) {
             exceptionHandler.setReturnStackTracesForExceptionTypes(Throwable.class);
         }
-		JaxRsRequest requestDetails = theServer.getRequest(null, null).build();
+        final JaxRsRequest requestDetails = theServer.getRequest(null, null).build();
-		BaseServerResponseException convertedException = preprocessException(theException, requestDetails);
+        final BaseServerResponseException convertedException = preprocessException(theException, requestDetails);
         return new JaxRsResponseException(convertedException);
     }
 
@@ -96,24 +98,31 @@ public class JaxRsExceptionInterceptor {
      * @return the response describing the error
      * @throws IOException
      */
-	public Response convertExceptionIntoResponse(JaxRsRequest theRequest, JaxRsResponseException theException)
+    public Response convertExceptionIntoResponse(final JaxRsRequest theRequest, final JaxRsResponseException theException)
             throws IOException {
         return handleExceptionWithoutServletError(theRequest, theException);
     }
 
-	private BaseServerResponseException preprocessException(final Throwable theException, JaxRsRequest requestDetails) {
+    private BaseServerResponseException preprocessException(final Throwable theException, final JaxRsRequest requestDetails) {
         try {
-			return exceptionHandler.preProcessOutgoingException(requestDetails, theException, null);
+            Throwable theExceptionToConvert = theException;
-		} catch(ServletException e) {
+            if (!(theException instanceof BaseServerResponseException) && (theException.getCause() instanceof BaseServerResponseException)) {
+                theExceptionToConvert = theException.getCause();
+            }
+            return exceptionHandler.preProcessOutgoingException(requestDetails, theExceptionToConvert, null);
+        }
+        catch (final ServletException e) {
             return new InternalErrorException(e);
         }
     }
 
-	private Response handleExceptionWithoutServletError(JaxRsRequest theRequest, BaseServerResponseException theException) throws IOException {
+    private Response handleExceptionWithoutServletError(final JaxRsRequest theRequest, final BaseServerResponseException theException)
+            throws IOException {
         try {
             return (Response) exceptionHandler.handleException(theRequest, theException);
-		} catch (ServletException e) {
+        }
-			BaseServerResponseException newException = preprocessException(new InternalErrorException(e), theRequest);
+        catch (final ServletException e) {
+            final BaseServerResponseException newException = preprocessException(new InternalErrorException(e), theRequest);
             return handleExceptionWithoutServletError(theRequest, newException);
         }
     }