[cpp-sp] branch main updated: Clean up exception messages and logging.

Codeberg noreply at shibboleth.net
Mon Jan 26 16:01:45 UTC 2026


This is an automated email from the git hooks/post-receive script.

codeberg pushed a commit to branch main
in repository cpp-sp.

View the commit online:
https://codeberg.org/Shibboleth/cpp-sp/commit/8449c3eb8b4e6d058813091bf7e9e449447c16d3

The following commit(s) were added to refs/heads/main by this push:
     new 8449c3eb Clean up exception messages and logging.
8449c3eb is described below

commit 8449c3eb8b4e6d058813091bf7e9e449447c16d3
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Mon Jan 26 11:01:34 2026 -0500

    Clean up exception messages and logging.
---
 shibsp/AbstractSPRequest.cpp                     | 23 +++++++++++++++++++++++
 shibsp/AbstractSPRequest.h                       |  1 +
 shibsp/SPRequest.h                               |  9 +++++++++
 shibsp/exceptions.cpp                            | 14 +-------------
 shibsp/handler/impl/StatusHandler.cpp            | 13 +++----------
 shibsp/remoting/impl/AbstractRemotingService.cpp | 12 ++++--------
 shibsp/remoting/impl/AbstractRemotingService.h   |  2 --
 shibsp/remoting/impl/WinHTTPRemotingService.cpp  |  4 ----
 8 files changed, 41 insertions(+), 37 deletions(-)

diff --git a/shibsp/AbstractSPRequest.cpp b/shibsp/AbstractSPRequest.cpp
index 738cbef8..1f256415 100644
--- a/shibsp/AbstractSPRequest.cpp
+++ b/shibsp/AbstractSPRequest.cpp
@@ -453,6 +453,29 @@ const char* AbstractSPRequest::getLogContext() const{
     return nullptr;
 }
 
+void AbstractSPRequest::log(Priority::Value level, const exception& e) const
+{
+    const AgentException* rich_ex = dynamic_cast<const AgentException*>(&e);
+    if (rich_ex) {
+        ostringstream msg;
+        msg << e.what() << " [";
+
+        // Dump properties and status code.
+        msg << "status=" << rich_ex->getStatusCode();
+
+        for (const auto& prop : rich_ex->getProperties()) {
+            msg << ", " << prop.first << '=' << prop.second;
+        }
+
+        msg << ']';
+
+        log(level, msg.str());
+    }
+    else {
+        log(level, e.what());
+    }
+}
+
 void AbstractSPRequest::log(Priority::Value level, const std::string& msg) const
 {
     if (isPriorityEnabled(level)) {
diff --git a/shibsp/AbstractSPRequest.h b/shibsp/AbstractSPRequest.h
index cc1f9be2..8580ad66 100644
--- a/shibsp/AbstractSPRequest.h
+++ b/shibsp/AbstractSPRequest.h
@@ -78,6 +78,7 @@ namespace shibsp {
 
         std::string getSecureHeader(const char* name) const;
         void setAuthType(const char* authtype);
+        void log(Priority::Value level, const std::exception& ex) const;
         void log(Priority::Value level, const std::string& msg) const;
         void log(Priority::Value level, const char* formatString, va_list args) const;
         bool isPriorityEnabled(Priority::Value level) const;
diff --git a/shibsp/SPRequest.h b/shibsp/SPRequest.h
index d6e65e72..53857e50 100644
--- a/shibsp/SPRequest.h
+++ b/shibsp/SPRequest.h
@@ -27,6 +27,7 @@
 #include <shibsp/logging/Priority.h>
 
 #include <cstdarg>
+#include <exception>
 #include <mutex>
 
 namespace shibsp {
@@ -165,6 +166,14 @@ namespace shibsp {
          */
         virtual void setAuthType(const char* authtype)=0;
 
+        /**
+         * Log exception to native server environment.
+         * 
+         * @param level logging level
+         * @param e exception
+         */
+        virtual void log(Priority::Value level, const std::exception& e) const=0;
+
         /**
          * Log to native server environment.
          *
diff --git a/shibsp/exceptions.cpp b/shibsp/exceptions.cpp
index 67097655..5ad2b83a 100644
--- a/shibsp/exceptions.cpp
+++ b/shibsp/exceptions.cpp
@@ -103,17 +103,5 @@ string AgentException::toQueryString() const
 
 void AgentException::log(const SPRequest& request, Priority::Value priority) const
 {
-    ostringstream msg;
-    msg << what() << " [";
-
-    // Dump properties and status code.
-    msg << "status=" << getStatusCode();
-
-    for (const auto& prop : m_props) {
-        msg << ", " << prop.first << '=' << prop.second;
-    }
-
-    msg << ']';
-
-    request.log(priority, msg.str());
+    request.log(priority, *this);
 }
diff --git a/shibsp/handler/impl/StatusHandler.cpp b/shibsp/handler/impl/StatusHandler.cpp
index 48efb038..5a8af069 100644
--- a/shibsp/handler/impl/StatusHandler.cpp
+++ b/shibsp/handler/impl/StatusHandler.cpp
@@ -275,20 +275,13 @@ pair<bool,long> StatusHandler::run(SPRequest& request, bool isHandler) const
         return make_pair(true, request.sendResponse(s));
     }
 
-    catch (exception& ex) {
-
-        string fullDetails(string(ex.what()));
-        AgentException* agent_ex = dynamic_cast<AgentException*>(&ex);
-        if (agent_ex && agent_ex->getProperty(AgentException::EVENT_PROP_NAME)) {
-            fullDetails +=  string(" (") + agent_ex->getProperty(AgentException::EVENT_PROP_NAME) + ")";
-        }
-
-        request.error(string("error while processing request: ") + fullDetails);
+    catch (const exception& ex) {
+        request.log(Priority::SHIB_ERROR, ex);
         request.setContentType("text/xml");
         stringstream msg;
         msg << "<StatusHandler time='" << timestamp << "'>"
             << "<Version Shibboleth='" << PACKAGE_VERSION << "'/>";
-        systemInfo(msg) << "<Status><Exception typename='" << typeid(ex).name() << "'>" << fullDetails << "</Exception></Status>"
+        systemInfo(msg) << "<Status><Exception typename='" << typeid(ex).name() << "'>" << ex.what() << "</Exception></Status>"
             << "</StatusHandler>";
         return make_pair(true, request.sendResponse(msg, HTTPResponse::SHIBSP_HTTP_STATUS_ERROR));
     }
diff --git a/shibsp/remoting/impl/AbstractRemotingService.cpp b/shibsp/remoting/impl/AbstractRemotingService.cpp
index 16128daf..34db0942 100644
--- a/shibsp/remoting/impl/AbstractRemotingService.cpp
+++ b/shibsp/remoting/impl/AbstractRemotingService.cpp
@@ -47,25 +47,21 @@ DDF AbstractRemotingService::send(const DDF& in, bool checkEvent) const
     if (!checkEvent) {
         return output;
     }
-
     const char* event = output.getmember("event").string();
     if (event && strcmp(event, "success")) {
-        string message("Send Message Failed: ");
+        string message("Remote operation (");
+        message += in.name() ? in.name() : "unknown";
+        message += ") failed with event: ";
         message += event;
-        OperationException ex("Remote operation was unsuccessful.");
+        OperationException ex(message);
         ex.addProperty(AgentException::EVENT_PROP_NAME, event);
         if (in.name()) {
             ex.addProperty("operation", in.name());
-            message += " operation : ";
-            message += in.name();
         }
         const char* target = output.getmember("target").string();
         if (target) {
             ex.addProperty(AgentException::TARGET_PROP_NAME, target);
-            message += " target : ";
-            message += target;
         }
-        logger().debug(message);
         output.destroy();
         throw ex;
     }
diff --git a/shibsp/remoting/impl/AbstractRemotingService.h b/shibsp/remoting/impl/AbstractRemotingService.h
index 040ffe27..9a911cb8 100644
--- a/shibsp/remoting/impl/AbstractRemotingService.h
+++ b/shibsp/remoting/impl/AbstractRemotingService.h
@@ -45,8 +45,6 @@ namespace shibsp {
          */
         DDF send(const DDF& in, bool checkEvent=true) const;
 
-        virtual Category& logger() const = 0;
-
     protected:
         AbstractRemotingService(const boost::property_tree::ptree& pt);
 
diff --git a/shibsp/remoting/impl/WinHTTPRemotingService.cpp b/shibsp/remoting/impl/WinHTTPRemotingService.cpp
index f03d508d..bd6bab61 100644
--- a/shibsp/remoting/impl/WinHTTPRemotingService.cpp
+++ b/shibsp/remoting/impl/WinHTTPRemotingService.cpp
@@ -55,10 +55,6 @@ namespace {
         WinHTTPRemotingService(ptree& pt);
         virtual ~WinHTTPRemotingService();
 
-        Category& logger() const {
-            return m_log;
-        }
-
         bool isChunked() const {
             return m_chunked;
         }

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.


More information about the commits mailing list