[cpp-sp] branch main updated: Clean up some error paths into exceptions.

[Scott Cantor]

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

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

View the commit online:
http://git.shibboleth.net/view/?p=cpp-sp.git;a=commit;h=262fc9dac0098e4f8c42314f6f8ae04148424c57

The following commit(s) were added to refs/heads/main by this push:
     new 262fc9da Clean up some error paths into exceptions.
262fc9da is described below

commit 262fc9dac0098e4f8c42314f6f8ae04148424c57
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Tue Sep 16 09:28:26 2025 -0400

    Clean up some error paths into exceptions.
---
 shibsp/session/SessionCacheSPI.h               |  6 ++++--
 shibsp/session/impl/AbstractSessionCache.cpp   | 14 ++++++++++++--
 shibsp/session/impl/FilesystemSessionCache.cpp | 12 +++++++-----
 3 files changed, 23 insertions(+), 9 deletions(-)

diff --git a/shibsp/session/SessionCacheSPI.h b/shibsp/session/SessionCacheSPI.h
index 0b9a7a9f..15f64a3e 100644
--- a/shibsp/session/SessionCacheSPI.h
+++ b/shibsp/session/SessionCacheSPI.h
@@ -37,6 +37,9 @@ namespace shibsp {
      * <p>As a general rule, implementations should log errors internally and raise exceptions
      * such that the caller need not log the resulting object to ensure adequate logging of the
      * outcome.</p>
+     * 
+     * <p>Errors that reflect unexpected or impossible-to-anticipate issues should be thrown as
+     * an IOException, with return values used for more expected/controlled issues.</p>
      */
     class SHIBSP_API SessionCacheSPI
     {
@@ -104,8 +107,7 @@ namespace shibsp {
          * 
          * <p>The caller retains ownership of the session data object.</p>
          * 
-         * <p>The return value signals success or a version mismatch/collision, while any other
-         * more systemic error will result in an IOException.
+         * <p>The return value signals success or a version mismatch/collision.</p>
          * 
          * @param agent request, if available
          * @param key session key/ID
diff --git a/shibsp/session/impl/AbstractSessionCache.cpp b/shibsp/session/impl/AbstractSessionCache.cpp
index ee2c63a4..ab21f06f 100644
--- a/shibsp/session/impl/AbstractSessionCache.cpp
+++ b/shibsp/session/impl/AbstractSessionCache.cpp
@@ -561,7 +561,12 @@ void AbstractSessionCache::remove(SPRequest& request)
 void AbstractSessionCache::remove(const char* key)
 {
     dormant(key);
-    cache_remove(nullptr, key);
+    try {
+        cache_remove(nullptr, key);
+    }
+    catch (const exception&) {
+        // Should be logged by the SPI.
+    }
 }
 
 void AbstractSessionCache::dormant(const string& key)
@@ -734,7 +739,12 @@ bool BasicSession::isValid(SPRequest* request, unsigned int lifetime, unsigned i
                 string expired(date::format("%FT%TZ", chrono::system_clock::from_time_t(getCreation() + lifetime)));
                 m_cache.log().warn("session (%s) has expired, created (%s), expired (%s)", getID(), created.c_str(), expired.c_str());
             }
-            m_cache.cache_remove(request, getID());
+            try {
+                m_cache.cache_remove(request, getID());
+            }
+            catch (const exception&) {
+                // Should be logged by SPI.
+            }
             return false;
         }
     }
diff --git a/shibsp/session/impl/FilesystemSessionCache.cpp b/shibsp/session/impl/FilesystemSessionCache.cpp
index c4dd91f1..c04a61a0 100644
--- a/shibsp/session/impl/FilesystemSessionCache.cpp
+++ b/shibsp/session/impl/FilesystemSessionCache.cpp
@@ -277,12 +277,12 @@ DDF FilesystemSessionCache::cache_read(
     if (!is) {
         int e = errno;
         if (e == ENOENT) {
-            m_spilog.debug("session file (%s) does not exist", effective_path.c_str());
+            m_spilog.info("session file (%s) does not exist, deleted behind us?", effective_path.c_str());
         }
         else {
             m_spilog.error("error opening session file (%s) for reading, errno=%d", effective_path.c_str(), e);
         }
-        return obj;
+        throw IOException("Session file could not be found or read after acquisition of modification time.");
     }
 
     time_t now = time(nullptr);
@@ -303,7 +303,7 @@ DDF FilesystemSessionCache::cache_read(
 
     if (!isSessionDataValid(obj)) {
         m_spilog.error("deserialized session from file (%s) was invalid", effective_path.c_str());
-        return obj.destroy();
+        throw IOException("Session data was invalid.");
     }
 
     const char* appId = obj["app_id"].string();
@@ -363,7 +363,7 @@ DDF FilesystemSessionCache::cache_read(
             catch (const exception& ex) {
                 // This is an outright error attempting the update, so we just fail hard.
                 m_spilog.error("exception attempting to update session (%s): %s", key, ex.what());
-                return obj.destroy();
+                throw;
             }
         }
     }
@@ -500,7 +500,9 @@ void FilesystemSessionCache::cache_remove(SPRequest* request, const char* key)
             if (e != ENOENT) {
                 m_spilog.error("error removing file for session (%s), version (%u), errno=%d", key, version, e);
             }
-            break;
+            else {
+                break;
+            }
         }
         else {
             m_spilog.debug("removed session file for (%s), version (%u)", key, version);

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