changeset 6414:dce1bef431b7

using ErrorPayload in OrthancException
author Sebastien Jodogne <s.jodogne@gmail.com>
date Sat, 15 Nov 2025 10:09:58 +0100
parents 342980a5192d
children b0402a2b2ddf
files OrthancFramework/Sources/DicomNetworking/DimseErrorPayload.h OrthancFramework/Sources/JobsEngine/JobInfo.cpp OrthancFramework/Sources/JobsEngine/JobStepResult.cpp OrthancFramework/Sources/JobsEngine/JobStepResult.h OrthancFramework/Sources/OrthancException.cpp OrthancFramework/Sources/OrthancException.h OrthancServer/Sources/ServerJobs/DicomRetrieveScuBaseJob.cpp OrthancServer/Sources/main.cpp
diffstat 8 files changed, 42 insertions(+), 64 deletions(-) [+]
line wrap: on
line diff
--- a/OrthancFramework/Sources/DicomNetworking/DimseErrorPayload.h	Fri Nov 14 18:36:05 2025 +0100
+++ b/OrthancFramework/Sources/DicomNetworking/DimseErrorPayload.h	Sat Nov 15 10:09:58 2025 +0100
@@ -35,5 +35,5 @@
 {
   Json::Value MakeDimseErrorStatusPayload(uint16_t dimseErrorStatus);
 
-  uint16_t GetDimseErrorStatusFromPayload(const Json::Value& payload);
+  uint16_t GetDimseErrorStatusFromPayload(/* TODO PAYLOAD */ const Json::Value& payload);
 }
--- a/OrthancFramework/Sources/JobsEngine/JobInfo.cpp	Fri Nov 14 18:36:05 2025 +0100
+++ b/OrthancFramework/Sources/JobsEngine/JobInfo.cpp	Sat Nov 15 10:09:58 2025 +0100
@@ -198,11 +198,7 @@
 
     if (status_.GetErrorPayload().HasContent())
     {
-      Json::Value error;
-      error["Type"] = EnumerationToString(status_.GetErrorPayload().GetType());
-      error["Content"] = status_.GetErrorPayload().GetContent();
-
-      target["ErrorPayload"] = error;
+      status_.GetErrorPayload().Format(target["ErrorPayload"]);
     }
 
     target["Type"] = status_.GetJobType();
--- a/OrthancFramework/Sources/JobsEngine/JobStepResult.cpp	Fri Nov 14 18:36:05 2025 +0100
+++ b/OrthancFramework/Sources/JobsEngine/JobStepResult.cpp	Sat Nov 15 10:09:58 2025 +0100
@@ -81,11 +81,11 @@
 
   JobStepResult JobStepResult::Failure(const OrthancException& exception)
   {
-    if (exception.HasPayload())
+    if (exception.GetPayload().HasContent())
     {
       return Failure(exception.GetErrorCode(),
                      exception.HasDetails() ? exception.GetDetails() : NULL,
-                     exception.GetPayload());
+                     exception.GetPayload().GetContent());
     }
     else
     {
--- a/OrthancFramework/Sources/JobsEngine/JobStepResult.h	Fri Nov 14 18:36:05 2025 +0100
+++ b/OrthancFramework/Sources/JobsEngine/JobStepResult.h	Sat Nov 15 10:09:58 2025 +0100
@@ -64,6 +64,7 @@
 
     static JobStepResult Failure(const ErrorCode& error,
                                  const char* details,
+                                 /* TODO PAYLOAD */
                                  const Json::Value& errorPayload);
 
     static JobStepResult Failure(const OrthancException& exception);
--- a/OrthancFramework/Sources/OrthancException.cpp	Fri Nov 14 18:36:05 2025 +0100
+++ b/OrthancFramework/Sources/OrthancException.cpp	Sat Nov 15 10:09:58 2025 +0100
@@ -104,29 +104,29 @@
   }
 
 
+  void ErrorPayload::Format(Json::Value& target) const
+  {
+    target["Type"] = EnumerationToString(GetType());
+    target["Content"] = GetContent();
+  }
+
+
   OrthancException::OrthancException(const OrthancException& other) : 
     errorCode_(other.errorCode_),
     httpStatus_(other.httpStatus_),
     logged_(false),
-    payloadType_(ErrorPayloadType_None)
+    payload_(other.payload_)
   {
     if (other.details_.get() != NULL)
     {
       details_.reset(new std::string(*other.details_));
     }
-
-    // TODO: shouldn't we avoid copies of OrthancException completely ?
-    if (other.HasPayload())
-    {
-      payload_.reset(new Json::Value(other.GetPayload()));
-    }
   }
 
   OrthancException::OrthancException(ErrorCode errorCode) : 
     errorCode_(errorCode),
     httpStatus_(ConvertErrorCodeToHttpStatus(errorCode)),
-    logged_(false),
-    payloadType_(ErrorPayloadType_None)
+    logged_(false)
   {
   }
 
@@ -136,8 +136,7 @@
     errorCode_(errorCode),
     httpStatus_(ConvertErrorCodeToHttpStatus(errorCode)),
     logged_(log == LogException_Yes),
-    details_(new std::string(details)),
-    payloadType_(ErrorPayloadType_None)
+    details_(new std::string(details))
   {
 #if ORTHANC_ENABLE_LOGGING == 1
     if (log == LogException_Yes)
@@ -152,8 +151,7 @@
                                      HttpStatus httpStatus) :
     errorCode_(errorCode),
     httpStatus_(httpStatus),
-    logged_(false),
-    payloadType_(ErrorPayloadType_None)
+    logged_(false)
   {
   }
 
@@ -166,10 +164,9 @@
     errorCode_(errorCode),
     httpStatus_(ConvertErrorCodeToHttpStatus(errorCode)),
     logged_(log == LogException_Yes),
-    details_(new std::string(details)),
-    payloadType_(payloadType)
+    details_(new std::string(details))
   {
-    payload_.reset(new Json::Value(payload));
+    payload_.SetContent(payloadType, payload);
 
 #if ORTHANC_ENABLE_LOGGING == 1
     if (log == LogException_Yes)
@@ -189,10 +186,9 @@
     errorCode_(errorCode),
     httpStatus_(httpStatus),
     logged_(log == LogException_Yes),
-    details_(new std::string(details)),
-    payloadType_(payloadType)
+    details_(new std::string(details))
   {
-    payload_.reset(new Json::Value(payload));
+    payload_.SetContent(payloadType, payload);
 
 #if ORTHANC_ENABLE_LOGGING == 1
     if (log == LogException_Yes)
@@ -209,8 +205,7 @@
     errorCode_(errorCode),
     httpStatus_(httpStatus),
     logged_(log == LogException_Yes),
-    details_(new std::string(details)),
-    payloadType_(ErrorPayloadType_None)
+    details_(new std::string(details))
   {
 #if ORTHANC_ENABLE_LOGGING == 1
     if (log == LogException_Yes)
@@ -251,25 +246,4 @@
       return details_->c_str();
     }
   }
-
-  bool OrthancException::HasPayload() const
-  {
-    return payload_.get() != NULL;
-  }
-
-  const Json::Value& OrthancException::GetPayload() const
-  {
-    if (payload_.get() == NULL)
-    {
-      throw OrthancException(ErrorCode_BadSequenceOfCalls);
-    }
-
-    return *payload_;
-  }
-
-  bool OrthancException::HasBeenLogged() const
-  {
-    return logged_;
-  }
-
 }
--- a/OrthancFramework/Sources/OrthancException.h	Fri Nov 14 18:36:05 2025 +0100
+++ b/OrthancFramework/Sources/OrthancException.h	Sat Nov 15 10:09:58 2025 +0100
@@ -73,9 +73,13 @@
     ErrorPayloadType GetType() const;
 
     const Json::Value& GetContent() const;
+
+    void Format(Json::Value& target) const;
   };
 
 
+  // TODO: Shouldn't copies of OrthancException be avoided completely
+  // (i.e., tag OrthancException as boost::noncopyable)?
   class ORTHANC_PUBLIC OrthancException
   {
   private:
@@ -91,8 +95,7 @@
     std::unique_ptr<std::string>  details_;
     
     // New in Orthanc 1.12.10
-    ErrorPayloadType              payloadType_;
-    std::unique_ptr<Json::Value>  payload_;
+    ErrorPayload  payload_;
     
   public:
     OrthancException(const OrthancException& other);
@@ -135,15 +138,19 @@
 
     const char* GetDetails() const;
 
-    bool HasBeenLogged() const;
-
-    bool HasPayload() const;
-
-    ErrorPayloadType GetPayloadType() const
+    bool HasBeenLogged() const
     {
-      return payloadType_;
+      return logged_;
     }
 
-    const Json::Value& GetPayload() const;
+    ErrorPayload& GetPayload()
+    {
+      return payload_;
+    }
+
+    const ErrorPayload& GetPayload() const
+    {
+      return payload_;
+    }
   };
 }
--- a/OrthancServer/Sources/ServerJobs/DicomRetrieveScuBaseJob.cpp	Fri Nov 14 18:36:05 2025 +0100
+++ b/OrthancServer/Sources/ServerJobs/DicomRetrieveScuBaseJob.cpp	Sat Nov 15 10:09:58 2025 +0100
@@ -101,10 +101,10 @@
     }
     catch (OrthancException& e)
     {
-      if (e.HasPayload() &&
-          e.GetPayloadType() == ErrorPayloadType_Dimse)
+      if (e.GetPayload().HasContent() &&
+          e.GetPayload().GetType() == ErrorPayloadType_Dimse)
       {
-        dimseErrorStatus_ = GetDimseErrorStatusFromPayload(e.GetPayload());
+        dimseErrorStatus_ = GetDimseErrorStatusFromPayload(e.GetPayload().GetContent());
       }
 
       throw;
--- a/OrthancServer/Sources/main.cpp	Fri Nov 14 18:36:05 2025 +0100
+++ b/OrthancServer/Sources/main.cpp	Sat Nov 15 10:09:58 2025 +0100
@@ -714,9 +714,9 @@
         message["Details"] = exception.GetDetails();
       }
 
-      if (exception.HasPayload())
+      if (exception.GetPayload().HasContent())
       {
-        message["ErrorPayload"] = exception.GetPayload();
+        exception.GetPayload().Format(message["ErrorPayload"]);
       }
 
       std::string info = message.toStyledString();