Mercurial > hg > orthanc

changeset 1569:27774f6f84e4

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
improved error handling in http answers
author jodogne
date Sun, 23 Aug 2015 11:01:15 +0200
parents 818ae9bc493a
children 2bd2c280f9b5
files Core/HttpServer/MongooseServer.cpp OrthancServer/OrthancRestApi/OrthancRestAnonymizeModify.cpp
diffstat 2 files changed, 109 insertions(+), 137 deletions(-) [+]
line wrap: on
line diff
--- a/Core/HttpServer/MongooseServer.cpp	Sun Aug 23 09:55:23 2015 +0200
+++ b/Core/HttpServer/MongooseServer.cpp	Sun Aug 23 11:01:15 2015 +0200
@@ -719,71 +719,80 @@
 
     LOG(INFO) << EnumerationToString(method) << " " << Toolbox::FlattenUri(uri);
 
-    bool found = false;
 
     try
     {
-      if (that->HasHandler())
-      {
-        found = that->GetHandler().Handle(output, method, uri, headers, argumentsGET, body.c_str(), body.size());
-      }
+      bool found = false;
+
+      try
+	{
+	  if (that->HasHandler())
+	    {
+	      found = that->GetHandler().Handle(output, method, uri, headers, argumentsGET, body.c_str(), body.size());
+	    }
+	}
+      catch (boost::bad_lexical_cast&)
+	{
+	  throw OrthancException(ErrorCode_BadParameterType);
+	}
+      catch (std::runtime_error&)
+	{
+	  // Presumably an error while parsing the JSON body
+	  throw OrthancException(ErrorCode_BadRequest);
+	}
+
+      if (!found)
+	{
+	  throw OrthancException(ErrorCode_UnknownResource);
+	}
     }
     catch (OrthancException& e)
     {
       // Using this candidate handler results in an exception
       LOG(ERROR) << "Exception in the HTTP handler: " << e.What();
 
+      HttpStatus status;
+
       try
       {
         switch (e.GetErrorCode())
         {
-          case ErrorCode_InexistentFile:
-          case ErrorCode_InexistentItem:
-          case ErrorCode_UnknownResource:
-            output.SendStatus(HttpStatus_404_NotFound);
-            break;
+	case ErrorCode_InexistentFile:
+	case ErrorCode_InexistentItem:
+	case ErrorCode_UnknownResource:
+	  status = HttpStatus_404_NotFound;
+	  break;
 
-          case ErrorCode_BadRequest:
-          case ErrorCode_UriSyntax:
-            output.SendStatus(HttpStatus_400_BadRequest);
+	case ErrorCode_BadRequest:
+	case ErrorCode_UriSyntax:
+	case ErrorCode_BadParameterType:
+            status = HttpStatus_400_BadRequest;
             break;
 
           default:
-	    {
-	      Json::Value message = Json::objectValue;
-	      message["ErrorCode"] = e.GetErrorCode();
-	      message["Description"] = e.GetDescription(e.GetErrorCode());
-	      message["Message"] = e.What();
-	      std::string s = message.toStyledString();
-	      output.SendStatus(HttpStatus_500_InternalServerError, s);
-	    }
+	    status = HttpStatus_500_InternalServerError;
+	    break;
         }
+
+	Json::Value message = Json::objectValue;
+	message["HttpStatus"] = status;
+	message["HttpError"] = EnumerationToString(status);
+	message["OrthancStatus"] = e.GetErrorCode();
+	message["OrthancError"] = e.GetDescription(e.GetErrorCode());
+	message["Message"] = e.What();
+	message["Uri"] = request->uri;
+	message["Method"] = EnumerationToString(method);
+	std::string s = message.toStyledString();
+	output.SendStatus(status, s);
       }
       catch (OrthancException&)
       {
-        // An exception here reflects the fact that an exception was
-        // triggered after the status code was sent by the HTTP handler.
+        // An exception here reflects the fact that the status code
+        // was already set by the HTTP handler.
       }
 
       return;
     }
-    catch (boost::bad_lexical_cast&)
-    {
-      LOG(ERROR) << "Exception in the HTTP handler: Bad lexical cast";
-      output.SendStatus(HttpStatus_400_BadRequest, "Cannot cast some argument");
-      return;
-    }
-    catch (std::runtime_error&)
-    {
-      LOG(ERROR) << "Exception in the HTTP handler: Presumably a bad JSON request";
-      output.SendStatus(HttpStatus_400_BadRequest, "Bad JSON request");
-      return;
-    }
-
-    if (!found)
-    {
-      output.SendStatus(HttpStatus_404_NotFound);
-    }
   }
 
 
--- a/OrthancServer/OrthancRestApi/OrthancRestAnonymizeModify.cpp	Sun Aug 23 09:55:23 2015 +0200
+++ b/OrthancServer/OrthancRestApi/OrthancRestAnonymizeModify.cpp	Sun Aug 23 11:01:15 2015 +0200
@@ -314,8 +314,7 @@
       std::string modifiedInstance;
       if (context.Store(modifiedInstance, toStore) != StoreStatus_Success)
       {
-        LOG(ERROR) << "Error while storing a modified instance " << *it;
-        return;
+        throw OrthancException("Error while storing a modified instance " + *it);
       }
 
       // Sanity checks in debug mode
@@ -431,7 +430,7 @@
   }
 
 
-  static bool StoreCreatedInstance(std::string& id /* out */,
+  static void StoreCreatedInstance(std::string& id /* out */,
                                    ServerContext& context,
                                    ParsedDicomFile& dicom)
   {
@@ -442,17 +441,12 @@
 
     if (status == StoreStatus_Failure)
     {
-      LOG(ERROR) << "Error while storing a manually-created instance";
-      return false;
-    }
-    else
-    {
-      return true;
+      throw OrthancException("Error while storing a manually-created instance");
     }
   }
 
 
-  static bool CreateDicomV1(ParsedDicomFile& dicom,
+  static void CreateDicomV1(ParsedDicomFile& dicom,
                             const Json::Value& request)
   {
     // curl http://localhost:8042/tools/create-dicom -X POST -d '{"PatientName":"Hello^World"}'
@@ -467,8 +461,7 @@
       const std::string& name = members[i];
       if (request[name].type() != Json::stringValue)
       {
-        LOG(ERROR) << "Only string values are supported when creating DICOM instances";
-        return false;
+        throw OrthancException("Only string values are supported when creating DICOM instances");
       }
 
       std::string value = request[name].asString();
@@ -483,18 +476,15 @@
         dicom.Replace(tag, value);
       }
     }
-
-    return true;
   }
 
 
-  static bool InjectTags(ParsedDicomFile& dicom,
+  static void InjectTags(ParsedDicomFile& dicom,
                          const Json::Value& tags)
   {
     if (tags.type() != Json::objectValue)
     {
-      LOG(ERROR) << "Bad syntax to specify the tags";
-      return false;
+      throw OrthancException("Bad syntax to specify the tags");
     }
 
     // Inject the user-specified tags
@@ -504,8 +494,7 @@
       const std::string& name = members[i];
       if (tags[name].type() != Json::stringValue)
       {
-        LOG(ERROR) << "Only string values are supported when creating DICOM instances";
-        return false;
+        throw OrthancException("Only string values are supported when creating DICOM instances");
       }
 
       std::string value = tags[name].asString();
@@ -516,14 +505,12 @@
         if (tag != DICOM_TAG_PATIENT_ID &&
             dicom.HasTag(tag))
         {
-          LOG(ERROR) << "Trying to override a value inherited from a parent module";
-          return false;
+          throw OrthancException("Trying to override a value inherited from a parent module");
         }
 
         if (tag == DICOM_TAG_PIXEL_DATA)
         {
-          LOG(ERROR) << "Use \"Content\" to inject an image into a new DICOM instance";
-          return false;
+          throw OrthancException("Use \"Content\" to inject an image into a new DICOM instance");
         }
         else
         {
@@ -531,8 +518,6 @@
         }
       }
     }
-
-    return true;
   }
 
 
@@ -549,11 +534,9 @@
 
     std::string someInstance;
 
-    bool ok = true;
-
     try
     {
-      for (Json::ArrayIndex i = 0; ok && i < content.size(); i++)
+      for (Json::ArrayIndex i = 0; i < content.size(); i++)
       {
         std::auto_ptr<ParsedDicomFile> dicom(base.Clone());
         const Json::Value* payload = NULL;
@@ -566,59 +549,48 @@
         {
           if (!content[i].isMember("Content"))
           {
-            LOG(ERROR) << "No payload is present for one instance in the series";
-            ok = false;
-            break;
+            throw OrthancException("No payload is present for one instance in the series");
           }
 
           payload = &content[i]["Content"];
 
-          if (content[i].isMember("Tags") &&
-              !InjectTags(*dicom, content[i]["Tags"]))
-          {
-            ok = false;
-            break;
-          }
+          if (content[i].isMember("Tags"))
+	    {
+              InjectTags(*dicom, content[i]["Tags"]);
+	    }
         }
 
         if (payload == NULL ||
             payload->type() != Json::stringValue)
         {
-          LOG(ERROR) << "The payload of the DICOM instance must be specified according to Data URI scheme";
-          ok = false;
-          break;
+          throw OrthancException("The payload of the DICOM instance must be specified according to Data URI scheme");
         }
 
         dicom->EmbedContent(payload->asString());
         dicom->Replace(DICOM_TAG_INSTANCE_NUMBER, boost::lexical_cast<std::string>(i + 1));
         dicom->Replace(DICOM_TAG_IMAGE_INDEX, boost::lexical_cast<std::string>(i + 1));
 
-        if (!StoreCreatedInstance(someInstance, context, *dicom))
-        {
-          LOG(ERROR) << "Error while creating the series";
-          ok = false;
-          break;
-        }
+        StoreCreatedInstance(someInstance, context, *dicom);
       }
     }
-    catch (OrthancException&)
+    catch (OrthancException& e)
     {
-      ok = false;
+      // Error: Remove the newly-created series
+      
+      std::string series;
+      if (context.GetIndex().LookupParent(series, someInstance))
+	{
+        Json::Value dummy;
+        context.GetIndex().DeleteResource(dummy, series, ResourceType_Series);
+	}
+
+      throw e;
     }
 
     std::string series;
     if (context.GetIndex().LookupParent(series, someInstance))
     {
-      if (ok)
-      {
-        OrthancRestApi::GetApi(call).AnswerStoredResource(call, series, ResourceType_Series, StoreStatus_Success);
-      }
-      else
-      {
-        // Error: Remove the newly-created series
-        Json::Value dummy;
-        context.GetIndex().DeleteResource(dummy, series, ResourceType_Series);
-      }
+      OrthancRestApi::GetApi(call).AnswerStoredResource(call, series, ResourceType_Series, StoreStatus_Success);
     }
   }
 
@@ -632,7 +604,7 @@
     if (!request.isMember("Tags") ||
         request["Tags"].type() != Json::objectValue)
     {
-      return;
+      throw OrthancException(ErrorCode_BadRequest);
     }
 
     ParsedDicomFile dicom;
@@ -645,8 +617,7 @@
         const char* tmp = request["Tags"]["SpecificCharacterSet"].asCString();
         if (!GetDicomEncoding(encoding, tmp))
         {
-          LOG(ERROR) << "Unknown specific character set: " << tmp;
-          return;
+          throw OrthancException("Unknown specific character set: " + std::string(tmp));
         }
       }
       else
@@ -666,14 +637,12 @@
       std::string parent = request["Parent"].asString();
       if (!context.GetIndex().LookupResourceType(parentType, parent))
       {
-        LOG(ERROR) << "Trying to attach a new DICOM instance to an inexistent resource: " << parent;
-        return;
+        throw OrthancException("Trying to attach a new DICOM instance to an inexistent resource: " + parent);
       }
 
       if (parentType == ResourceType_Instance)
       {
-        LOG(ERROR) << "Trying to attach a new DICOM instance to an instance (must be a series, study or patient): " << parent;
-        return;
+        throw OrthancException("Trying to attach a new DICOM instance to an instance (must be a series, study or patient): " + parent);
       }
 
       // Select one existing child instance of the parent resource, to
@@ -686,8 +655,9 @@
         context.GetIndex().GetChildInstances(siblingInstances, parent);
 
         if (siblingInstances.empty())
-        {
-          return;   // Error: No instance (should never happen)
+	{
+	  // Error: No instance (should never happen)
+          throw OrthancException(ErrorCode_InternalError);
         }
 
         context.ReadJson(siblingTags, siblingInstances.front());
@@ -705,8 +675,7 @@
               siblingTags[SPECIFIC_CHARACTER_SET]["Value"].type() != Json::stringValue ||
               !GetDicomEncoding(encoding, siblingTags[SPECIFIC_CHARACTER_SET]["Value"].asCString()))
           {
-            LOG(ERROR) << "Unable to get the encoding of the parent resource";
-            return;
+            throw OrthancException("Unable to get the encoding of the parent resource");
           }
 
           dicom.SetEncoding(encoding);
@@ -732,7 +701,7 @@
         std::string tmp;
         if (!context.GetIndex().LookupParent(tmp, parent))
         {
-          return;
+          throw OrthancException(ErrorCode_InternalError);
         }
 
         parent = tmp;
@@ -786,10 +755,7 @@
     }
 
 
-    if (!InjectTags(dicom, request["Tags"]))
-    {
-      return;  // Error
-    }
+    InjectTags(dicom, request["Tags"]);
 
 
     // Inject the content (either an image, or a PDF file)
@@ -813,16 +779,14 @@
       }
       else
       {
-        LOG(ERROR) << "The payload of the DICOM instance must be specified according to Data URI scheme";
+        throw OrthancException("The payload of the DICOM instance must be specified according to Data URI scheme");
         return;
       }
     }
 
     std::string id;
-    if (StoreCreatedInstance(id, context, dicom))
-    {
-      OrthancRestApi::GetApi(call).AnswerStoredResource(call, id, ResourceType_Instance, StoreStatus_Success);
-    }
+    StoreCreatedInstance(id, context, dicom);
+    OrthancRestApi::GetApi(call).AnswerStoredResource(call, id, ResourceType_Instance, StoreStatus_Success);
 
     return;
   }
@@ -831,28 +795,27 @@
   static void CreateDicom(RestApiPostCall& call)
   {
     Json::Value request;
-    if (call.ParseJsonRequest(request) && 
-        request.isObject())
-    {
-      if (request.isMember("Tags"))
+    if (!call.ParseJsonRequest(request) ||
+        !request.isObject())
       {
-        CreateDicomV2(call, request);
+	throw OrthancException(ErrorCode_BadRequest);
       }
-      else
+
+    if (request.isMember("Tags"))
       {
-        // Compatibility with Orthanc <= 0.9.3
-        ServerContext& context = OrthancRestApi::GetContext(call);
-        ParsedDicomFile dicom;
-        if (CreateDicomV1(dicom, request))
-        {
-          std::string id;
-          if (StoreCreatedInstance(id, context, dicom))
-          {
-            OrthancRestApi::GetApi(call).AnswerStoredResource(call, id, ResourceType_Instance, StoreStatus_Success);
-          }
-        }
+	CreateDicomV2(call, request);
       }
-    }
+    else
+      {
+	// Compatibility with Orthanc <= 0.9.3
+	ServerContext& context = OrthancRestApi::GetContext(call);
+	ParsedDicomFile dicom;
+	CreateDicomV1(dicom, request);
+
+	std::string id;
+	StoreCreatedInstance(id, context, dicom);
+	OrthancRestApi::GetApi(call).AnswerStoredResource(call, id, ResourceType_Instance, StoreStatus_Success);
+      }
   }