From f9071db9983ada76b9ac230a5264dcf9b7d40674 Mon Sep 17 00:00:00 2001
From: emeric <emeric.poupon@laposte.net>
Date: Fri, 17 Oct 2025 23:30:58 +0200
Subject: [PATCH] Always set http response codes for media retrieval endpoints

---
 src/libs/subsonic/impl/SubsonicResource.cpp | 83 ++++++++++++++++-----
 src/libs/subsonic/impl/SubsonicResource.hpp |  5 ++
 2 files changed, 69 insertions(+), 19 deletions(-)

diff --git a/src/libs/subsonic/impl/SubsonicResource.cpp b/src/libs/subsonic/impl/SubsonicResource.cpp
index 5a32166d..ac1b4316 100644
--- a/src/libs/subsonic/impl/SubsonicResource.cpp
+++ b/src/libs/subsonic/impl/SubsonicResource.cpp
@@ -287,6 +287,24 @@ namespace lms::api::subsonic
         if (core::stringUtils::stringEndsWith(requestPath, ".view"))
             requestPath.resize(requestPath.length() - 5);
 
+        // First check for media retrieval endpoints
+        auto itStreamHandler{ mediaRetrievalHandlers.find(requestPath) };
+        if (itStreamHandler != mediaRetrievalHandlers.end())
+        {
+            try
+            {
+                LMS_SCOPED_TRACE_OVERVIEW("Subsonic", itStreamHandler->first);
+                handleMediaRetrievalRequest(itStreamHandler->second, request, response);
+                LMS_LOG(API_SUBSONIC, DEBUG, "Request " << requestId << " '" << requestPath << "' handled!");
+            }
+            catch (const Error& e)
+            {
+                LMS_LOG(API_SUBSONIC, ERROR, "Error while processing request '" << requestId << "', code = " << static_cast<int>(e.getCode()) << ", msg = '" << e.getMessage() << "'");
+            }
+
+            return;
+        }
+
         // Optional parameters
         const ResponseFormat format{ getParameterAs<std::string>(request.getParameterMap(), "f").value_or("xml") == "json" ? ResponseFormat::json : ResponseFormat::xml };
 
@@ -324,25 +342,6 @@ namespace lms::api::subsonic
                 return;
             }
 
-            auto itStreamHandler{ mediaRetrievalHandlers.find(requestPath) };
-            if (itStreamHandler != mediaRetrievalHandlers.end())
-            {
-                LMS_SCOPED_TRACE_OVERVIEW("Subsonic", itStreamHandler->first);
-
-                // Media retrieval endpoints are always authenticated
-                // Optim: no need to reauth user for each continuation
-                db::User::pointer user;
-                if (!request.continuation())
-                    user = getUserFromUserId(_db.getTLSSession(), authenticateUser(request));
-
-                RequestContext requestContext{ request, _db.getTLSSession(), user, _config };
-                protocolVersion = requestContext.getServerProtocolVersion();
-
-                itStreamHandler->second(requestContext, request, response);
-                LMS_LOG(API_SUBSONIC, DEBUG, "Request " << requestId << " '" << requestPath << "' handled!");
-                return;
-            }
-
             // do not disclose unhandled commands for unauthenticated users
             authenticateUser(request);
 
@@ -360,6 +359,52 @@ namespace lms::api::subsonic
         }
     }
 
+    void SubsonicResource::handleMediaRetrievalRequest(MediaRetrievalHandlerFunc handler, const Wt::Http::Request& request, Wt::Http::Response& response)
+    {
+        try
+        {
+            // Media retrieval endpoints are always authenticated
+            // Optimization: no need to reauth user for each continuation
+            db::User::pointer user;
+            if (!request.continuation())
+                user = getUserFromUserId(_db.getTLSSession(), authenticateUser(request));
+
+            RequestContext requestContext{ request, _db.getTLSSession(), user, _config };
+
+            handler(requestContext, request, response);
+        }
+        catch (const UserNotAuthorizedError&)
+        {
+            response.setStatus(401); // Unauthorized
+            throw;
+        }
+        catch (const RequiredParameterMissingError&)
+        {
+            response.setStatus(400); // Bad Request
+            throw;
+        }
+        catch (const BadParameterGenericError&)
+        {
+            response.setStatus(400); // Bad Request
+            throw;
+        }
+        catch (const RequestedDataNotFoundError&)
+        {
+            response.setStatus(404); // Not Found
+            throw;
+        }
+        catch (const InternalErrorGenericError&)
+        {
+            response.setStatus(500); // Internal Server Error
+            throw;
+        }
+        catch (const Error&)
+        {
+            response.setStatus(400); // Assume bad request
+            throw;
+        }
+    }
+
     db::UserId SubsonicResource::authenticateUser(const Wt::Http::Request& request)
     {
         const auto& parameters{ request.getParameterMap() };
diff --git a/src/libs/subsonic/impl/SubsonicResource.hpp b/src/libs/subsonic/impl/SubsonicResource.hpp
index 70c9a5de..32bf4318 100644
--- a/src/libs/subsonic/impl/SubsonicResource.hpp
+++ b/src/libs/subsonic/impl/SubsonicResource.hpp
@@ -33,6 +33,8 @@ namespace lms::db
 
 namespace lms::api::subsonic
 {
+    class RequestContext;
+
     class SubsonicResource final : public Wt::WResource
     {
     public:
@@ -41,6 +43,9 @@ namespace lms::api::subsonic
     private:
         void handleRequest(const Wt::Http::Request& request, Wt::Http::Response& response) override;
 
+        using MediaRetrievalHandlerFunc = std::function<void(RequestContext&, const Wt::Http::Request&, Wt::Http::Response&)>;
+        void handleMediaRetrievalRequest(MediaRetrievalHandlerFunc handler, const Wt::Http::Request& request, Wt::Http::Response& response);
+
         db::UserId authenticateUser(const Wt::Http::Request& request);
 
         const SubsonicResourceConfig _config;