Exception enhancements

This commit is contained in:
James Agnew 2014-08-20 15:22:25 -04:00
parent d67a29a366
commit 47c522ac6f
8 changed files with 286 additions and 43 deletions

View File

@ -257,6 +257,12 @@
<version>${hamcrest_version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>com.google.guava</groupId>
<artifactId>guava</artifactId>
<version>17.0</version>
<scope>test</scope>
</dependency>
<!--
<dependency>
<groupId>org.springframework.security</groupId>

View File

@ -547,21 +547,29 @@ public class RestfulServer extends HttpServlet {
} catch (Throwable e) {
OperationOutcome oo = new OperationOutcome();
Issue issue = oo.addIssue();
issue.getSeverity().setValueAsEnum(IssueSeverityEnum.ERROR);
OperationOutcome oo=null;
int statusCode = 500;
if (e instanceof InternalErrorException) {
ourLog.error("Failure during REST processing", e);
issue.getDetails().setValue(e.toString() + "\n\n" + ExceptionUtils.getStackTrace(e));
} else if (e instanceof BaseServerResponseException) {
ourLog.warn("Failure during REST processing: {}", e.toString());
if (e instanceof BaseServerResponseException) {
oo = ((BaseServerResponseException) e).getOperationOutcome();
statusCode = ((BaseServerResponseException) e).getStatusCode();
issue.getDetails().setValue(e.getMessage());
} else {
ourLog.error("Failure during REST processing", e);
issue.getDetails().setValue(e.toString() + "\n\n" + ExceptionUtils.getStackTrace(e));
}
if (oo == null) {
oo = new OperationOutcome();
Issue issue = oo.addIssue();
issue.getSeverity().setValueAsEnum(IssueSeverityEnum.ERROR);
if (e instanceof InternalErrorException) {
ourLog.error("Failure during REST processing", e);
issue.getDetails().setValue(e.toString() + "\n\n" + ExceptionUtils.getStackTrace(e));
} else if (e instanceof BaseServerResponseException) {
ourLog.warn("Failure during REST processing: {}", e.toString());
statusCode = ((BaseServerResponseException) e).getStatusCode();
issue.getDetails().setValue(e.getMessage());
} else {
ourLog.error("Failure during REST processing", e);
issue.getDetails().setValue(e.toString() + "\n\n" + ExceptionUtils.getStackTrace(e));
}
}
streamResponseAsResource(this, theResponse, oo, determineResponseEncoding(theRequest), true, false, NarrativeModeEnum.NORMAL, statusCode, false, fhirServerBase);
@ -815,7 +823,7 @@ public class RestfulServer extends HttpServlet {
List<ResourceReferenceDt> references = theContext.newTerser().getAllPopulatedChildElementsOfType(next, ResourceReferenceDt.class);
do {
List<IResource> addedResourcesThisPass = new ArrayList<IResource>();
for (ResourceReferenceDt nextRef : references) {
IResource nextRes = nextRef.getResource();
if (nextRes != null) {
@ -836,21 +844,21 @@ public class RestfulServer extends HttpServlet {
}
}
}
// Linked resources may themselves have linked resources
references = new ArrayList<ResourceReferenceDt>();
for (IResource iResource : addedResourcesThisPass) {
List<ResourceReferenceDt> newReferences = theContext.newTerser().getAllPopulatedChildElementsOfType(iResource, ResourceReferenceDt.class);
references.addAll(newReferences);
}
addedResources.addAll(addedResourcesThisPass);
} while (references.isEmpty() == false);
BundleEntry entry = bundle.addResource(next, theContext, theServerBase);
addProfileToBundleEntry(theContext, next, entry);
}
/*

View File

@ -1,5 +1,6 @@
package ca.uhn.fhir.rest.server.exceptions;
import ca.uhn.fhir.model.dstu.resource.OperationOutcome;
import ca.uhn.fhir.rest.server.Constants;
/*
@ -44,6 +45,17 @@ public class InternalErrorException extends BaseServerResponseException {
private static final long serialVersionUID = 1L;
/**
* Constructor
*
* @param theMessage
* The message
* @param theOperationOutcome The OperationOutcome resource to return to the client
*/
public InternalErrorException(String theMessage, OperationOutcome theOperationOutcome) {
super(theMessage, theOperationOutcome);
}
public InternalErrorException(String theMessage) {
super(STATUS_CODE, theMessage);
}

View File

@ -22,33 +22,52 @@ package ca.uhn.fhir.rest.server.exceptions;
import ca.uhn.fhir.model.api.IResource;
import ca.uhn.fhir.model.dstu.composite.IdentifierDt;
import ca.uhn.fhir.model.dstu.resource.OperationOutcome;
import ca.uhn.fhir.model.primitive.IdDt;
import ca.uhn.fhir.rest.server.Constants;
/**
* Represents an <b>HTTP 404 Resource Not Found</b> response, which means that
* the request is pointing to a resource that does not exist.
* Represents an <b>HTTP 404 Resource Not Found</b> response, which means that the request is pointing to a resource that does not exist.
*/
public class ResourceNotFoundException extends BaseServerResponseException {
private static final long serialVersionUID = 1L;
public static final int STATUS_CODE = Constants.STATUS_HTTP_404_NOT_FOUND;
public ResourceNotFoundException(Class<? extends IResource> theClass, IdDt theId) {
super(STATUS_CODE, createErrorMessage(theClass, theId));
}
public ResourceNotFoundException(Class<? extends IResource> theClass, IdDt theId, OperationOutcome theOperationOutcome) {
super(STATUS_CODE, createErrorMessage(theClass, theId), theOperationOutcome);
}
/**
* @deprecated This doesn't make sense, since an identifier is not a resource ID and shouldn't generate a 404 if it isn't found - Should be removed
*/
public ResourceNotFoundException(Class<? extends IResource> theClass, IdentifierDt theId) {
super(STATUS_CODE, "Resource of type " + theClass.getSimpleName() + " with ID " + theId + " is not known");
}
public ResourceNotFoundException(IdDt theId) {
super(STATUS_CODE, "Resource " + (theId != null ? theId.getValue() : "") + " is not known");
super(STATUS_CODE, createErrorMessage(theId));
}
public ResourceNotFoundException(Class<? extends IResource> theClass, IdentifierDt thePatientId) {
super(STATUS_CODE, "Resource of type " + theClass.getSimpleName() + " with ID " + thePatientId + " is not known");
}
public ResourceNotFoundException(Class<? extends IResource> theClass, IdDt thePatientId) {
super(STATUS_CODE, "Resource of type " + theClass.getSimpleName() + " with ID " + thePatientId + " is not known");
public ResourceNotFoundException(IdDt theId, OperationOutcome theOperationOutcome) {
super(STATUS_CODE, createErrorMessage(theId), theOperationOutcome);
}
public ResourceNotFoundException(String theMessage) {
super(STATUS_CODE, theMessage);
}
private static final long serialVersionUID = 1L;
private static String createErrorMessage(Class<? extends IResource> theClass, IdDt theId) {
return "Resource of type " + theClass.getSimpleName() + " with ID " + theId + " is not known";
}
private static String createErrorMessage(IdDt theId) {
return "Resource " + (theId != null ? theId.getValue() : "") + " is not known";
}
}

View File

@ -21,13 +21,19 @@ package ca.uhn.fhir.rest.server.exceptions;
*/
/**
* Exception for use when a response is received or being sent that
* does not correspond to any other exception type. An HTTP status code
* must be provided, and will be provided to the caller in the case of a
* server implementation.
* Exception for use when a response is received or being sent that does not correspond to any other exception type. An HTTP status code must be provided, and will be provided to the caller in the
* case of a server implementation.
*/
public class UnclassifiedServerFailureException extends BaseServerResponseException {
/**
* Constructor
*
* @param theStatusCode
* The HTTP status code to return (e.g. 404 if you wish to return an HTTP 404 status)
* @param theMessage
* The message to add to the status line
*/
public UnclassifiedServerFailureException(int theStatusCode, String theMessage) {
super(theStatusCode, theMessage);
}

View File

@ -341,6 +341,83 @@
</section>
-->
<section name="Exception/Error Handling">
<p>
Within your RESTful operations, you will generally be returning
resources or bundles of resources under normal operation. During
execution you may also need to propagate errors back to the client
for a variety of reasons.
</p>
<subsection name="Automatic Exception Handling">
<p>
By default, HAPI generates appropriate error responses for a several
built-in conditions. For example, if the user makes a request for
a resource type that does not exist, or tries to perform a search
using an invalid parameter, HAPI will automatically generate
an <code>HTTP 400 Invalid Request</code>, and provide an
OperationOutcome resource as response containing details about
the error.
</p>
<p>
Similarly, if your method implementation throws any exceptions
(checked or unchecked) instead
of returning normally, the server will usually* automatically
generate an <code>HTTP 500 Internal Error</code> and generate
an OperationOutcome with details about the exception.
</p>
<p>
<i>* Note that certain exception types will generate other response
codes, as explained below.</i>
</p>
</subsection>
<subsection name="Generating Specific HTTP Error Responses">
<p>
In many cases, you will want to respond to client requests
with a specific HTTP error code (and possibly your own error message
too). Sometimes this is a requirement of the FHIR specification
(e.g. the "validate" operation requires a response of
<code>HTTP 422 Unprocessable Entity</code> if the validation fails).
Sometimes this is simply a requirement of your specific application
(e.g. you want to provide application specific HTTP status codes for
certain types of errors)
</p>
<p>
To customize the error that is returned by HAPI's server methods, you
must throw an exception which extends HAPI's
<a href="http://jamesagnew.github.io/hapi-fhir/apidocs/ca/uhn/fhir/rest/server/exceptions/BaseServerResponseException.html">BaseServerResponseException</a>
class. Various exceptions which extend this class will generate
a different HTTP status code.
</p>
<p>
For example, the
<a href="http://jamesagnew.github.io/hapi-fhir/apidocs/ca/uhn/fhir/rest/server/exceptions/ResourceNotFoundException.html">ResourceNotFoundException</a>
causes HAPI to return an <code>HTTP 404 Resource Not Found</code>. A complete list
of available exceptions is available
<a href="http://jamesagnew.github.io/hapi-fhir/apidocs/ca/uhn/fhir/rest/server/exceptions/package-summary.html">here</a>.
</p>
<p>
If you wish to return an HTTP status code for which there is no
pre-defined exception, you may throw the
<a href="http://jamesagnew.github.io/hapi-fhir/apidocs/ca/uhn/fhir/rest/server/exceptions/UnclassifiedServerFailureException.html">UnclassifiedServerFailureException</a>,
which allows you to return any status code you wish.
</p>
</subsection>
<subsection name="Returning an OperationOutcome for Errors">
<p>
By default, HAPI will automatically generate an OperationOutcome
which contains details about the exception that was thrown. You may
wish to provide your own OperationOutcome instead. In this
case, you may pass one into the constructor of the
exception you are throwing.
</p>
</subsection>
</section>
<section name="Using the Server">

View File

@ -1,10 +1,14 @@
package ca.uhn.fhir.rest.server;
import static org.junit.Assert.*;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.fail;
import java.util.List;
import java.util.concurrent.TimeUnit;
import junit.framework.AssertionFailedError;
import org.apache.commons.io.IOUtils;
import org.apache.http.HttpResponse;
import org.apache.http.client.methods.HttpGet;
@ -16,16 +20,21 @@ import org.eclipse.jetty.servlet.ServletHandler;
import org.eclipse.jetty.servlet.ServletHolder;
import org.hamcrest.core.StringContains;
import org.junit.AfterClass;
import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.Test;
import ca.uhn.fhir.model.api.IResource;
import ca.uhn.fhir.model.dstu.resource.OperationOutcome;
import ca.uhn.fhir.model.dstu.resource.Patient;
import ca.uhn.fhir.model.primitive.IdDt;
import ca.uhn.fhir.rest.annotation.IdParam;
import ca.uhn.fhir.rest.annotation.Read;
import ca.uhn.fhir.rest.annotation.RequiredParam;
import ca.uhn.fhir.rest.annotation.Search;
import ca.uhn.fhir.rest.param.StringParam;
import ca.uhn.fhir.rest.server.exceptions.InternalErrorException;
import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException;
import ca.uhn.fhir.testutil.RandomServerPortProvider;
/**
@ -34,11 +43,19 @@ import ca.uhn.fhir.testutil.RandomServerPortProvider;
public class ExceptionTest {
private static CloseableHttpClient ourClient;
private static boolean ourGenerateOperationOutcome;
private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(ExceptionTest.class);
private static int ourPort;
private static Server ourServer;
private static RestfulServer servlet;
@Before
public void before() {
ourGenerateOperationOutcome = false;
ourExceptionType=null;
}
@Test
public void testInternalError() throws Exception {
{
@ -53,6 +70,36 @@ public class ExceptionTest {
}
}
@Test
public void testResourceReturning() throws Exception {
// No OO
{
ourExceptionType=ResourceNotFoundException.class;
ourGenerateOperationOutcome=false;
HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient/123");
HttpResponse status = ourClient.execute(httpGet);
String responseContent = IOUtils.toString(status.getEntity().getContent());
IOUtils.closeQuietly(status.getEntity().getContent());
ourLog.info(responseContent);
assertEquals(404, status.getStatusLine().getStatusCode());
OperationOutcome oo = (OperationOutcome) servlet.getFhirContext().newXmlParser().parseResource(responseContent);
assertThat(oo.getIssueFirstRep().getDetails().getValue(), StringContains.containsString("Resource Patient/123 is not known"));
}
// Yes OO
{
ourExceptionType=ResourceNotFoundException.class;
ourGenerateOperationOutcome=true;
HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient/123");
HttpResponse status = ourClient.execute(httpGet);
String responseContent = IOUtils.toString(status.getEntity().getContent());
IOUtils.closeQuietly(status.getEntity().getContent());
ourLog.info(responseContent);
assertEquals(404, status.getStatusLine().getStatusCode());
OperationOutcome oo = (OperationOutcome) servlet.getFhirContext().newXmlParser().parseResource(responseContent);
assertThat(oo.getIssueFirstRep().getDetails().getValue(), StringContains.containsString(OPERATION_OUTCOME_DETAILS));
}
}
@Test
public void testInternalErrorFormatted() throws Exception {
{
@ -69,17 +116,16 @@ public class ExceptionTest {
@Test
public void testInternalErrorJson() throws Exception {
HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient?throwInternalError=aaa&_format=json");
HttpResponse status = ourClient.execute(httpGet);
String responseContent = IOUtils.toString(status.getEntity().getContent());
IOUtils.closeQuietly(status.getEntity().getContent());
ourLog.info(responseContent);
assertEquals(500, status.getStatusLine().getStatusCode());
OperationOutcome oo = (OperationOutcome) servlet.getFhirContext().newJsonParser().parseResource(responseContent);
assertThat(oo.getIssueFirstRep().getDetails().getValue(), StringContains.containsString("InternalErrorException: Exception Text"));
HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient?throwInternalError=aaa&_format=json");
HttpResponse status = ourClient.execute(httpGet);
String responseContent = IOUtils.toString(status.getEntity().getContent());
IOUtils.closeQuietly(status.getEntity().getContent());
ourLog.info(responseContent);
assertEquals(500, status.getStatusLine().getStatusCode());
OperationOutcome oo = (OperationOutcome) servlet.getFhirContext().newJsonParser().parseResource(responseContent);
assertThat(oo.getIssueFirstRep().getDetails().getValue(), StringContains.containsString("InternalErrorException: Exception Text"));
}
@AfterClass
public static void afterClass() throws Exception {
ourServer.stop();
@ -106,12 +152,17 @@ public class ExceptionTest {
ourClient = builder.build();
}
private static Class<? extends Exception> ourExceptionType;
private static final String OPERATION_OUTCOME_DETAILS = "OperationOutcomeDetails";
/**
* Created by dsotnikov on 2/25/2014.
*/
public static class DummyPatientResourceProvider implements IResourceProvider {
@Search
public List<Patient> findPatient(@RequiredParam(name = "throwInternalError") StringParam theParam) {
throw new InternalErrorException("Exception Text");
@ -122,6 +173,22 @@ public class ExceptionTest {
return Patient.class;
}
@Read
public Patient read(@IdParam IdDt theId) {
OperationOutcome oo = null;
if (ourGenerateOperationOutcome) {
oo = new OperationOutcome();
oo.addIssue().setDetails(OPERATION_OUTCOME_DETAILS);
}
if (ourExceptionType == ResourceNotFoundException.class) {
throw new ResourceNotFoundException(theId, oo);
}else {
throw new AssertionFailedError("Unknown exception type: " + ourExceptionType);
}
}
}
}

View File

@ -0,0 +1,48 @@
package ca.uhn.fhir.rest.server.exception;
import static org.junit.Assert.*;
import java.io.IOException;
import org.junit.Test;
import org.mockito.internal.matchers.GreaterThan;
import ca.uhn.fhir.model.dstu.resource.OperationOutcome;
import ca.uhn.fhir.rest.server.exceptions.AuthenticationException;
import ca.uhn.fhir.rest.server.exceptions.BaseServerResponseException;
import com.google.common.collect.ImmutableSet;
import com.google.common.reflect.ClassPath;
import com.google.common.reflect.ClassPath.ClassInfo;
public class ExceptionTest {
private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(ExceptionTest.class);
@Test
public void testExceptionsAreGood() throws Exception {
ImmutableSet<ClassInfo> classes = ClassPath.from(Thread.currentThread().getContextClassLoader()).getTopLevelClasses(BaseServerResponseException.class.getPackage().getName());
assertTrue(classes.size() > 5);
for (ClassInfo classInfo : classes) {
ourLog.info("Scanning {}", classInfo.getName());
Class<?> next = Class.forName(classInfo.getName());
assertNotNull(next);
if (next == AuthenticationException.class) {
continue;
}
if (next == BaseServerResponseException.class) {
continue;
}
try {
next.getConstructor(String.class, OperationOutcome.class);
} catch (NoSuchMethodException e) {
fail(classInfo.getName() + " has no constructor with params: (String, OperationOutcome)");
}
}
}
}