Fix #616: Fixed inconsistencies in subsonic error responses

api/funkwhale_api/subsonic/renderers.py

@@ -5,23 +5,27 @@ from rest_framework import renderers
 import funkwhale_api
 
 
+def structure_payload(data):
+    payload = {
+        "status": "ok",
+        "version": "1.16.0",
+        "type": "funkwhale",
+        "funkwhaleVersion": funkwhale_api.__version__,
+    }
+    payload.update(data)
+    if "detail" in payload:
+        payload["error"] = {"code": 0, "message": payload.pop("detail")}
+    if "error" in payload:
+        payload["status"] = "failed"
+    return payload
+
+
 class SubsonicJSONRenderer(renderers.JSONRenderer):
     def render(self, data, accepted_media_type=None, renderer_context=None):
         if not data:
             # when stream view is called, we don't have any data
             return super().render(data, accepted_media_type, renderer_context)
-        final = {
+        final = {"subsonic-response": structure_payload(data)}
-            "subsonic-response": {
-                "status": "ok",
-                "version": "1.16.0",
-                "type": "funkwhale",
-                "funkwhaleVersion": funkwhale_api.__version__,
-            }
-        }
-        final["subsonic-response"].update(data)
-        if "error" in final:
-            # an error was returned
-            final["subsonic-response"]["status"] = "failed"
         return super().render(final, accepted_media_type, renderer_context)
 
 
@@ -32,15 +36,8 @@ class SubsonicXMLRenderer(renderers.JSONRenderer):
         if not data:
             # when stream view is called, we don't have any data
             return super().render(data, accepted_media_type, renderer_context)
-        final = {
+        final = structure_payload(data)
-            "xmlns": "http://subsonic.org/restapi",
+        final["xmlns"] = "http://subsonic.org/restapi"
-            "status": "ok",
-            "version": "1.16.0",
-        }
-        final.update(data)
-        if "error" in final:
-            # an error was returned
-            final["status"] = "failed"
         tree = dict_to_xml_tree("subsonic-response", final)
         return b'<?xml version="1.0" encoding="UTF-8"?>\n' + ET.tostring(
             tree, encoding="utf-8"

api/funkwhale_api/subsonic/views.py

@@ -97,7 +97,10 @@ class SubsonicViewSet(viewsets.GenericViewSet):
     def handle_exception(self, exc):
         # subsonic API sends 200 status code with custom error
         # codes in the payload
-        mapping = {exceptions.AuthenticationFailed: (40, "Wrong username or password.")}
+        mapping = {
+            exceptions.AuthenticationFailed: (40, "Wrong username or password."),
+            exceptions.NotAuthenticated: (10, "Required parameter is missing."),
+        }
         payload = {"status": "failed"}
         if exc.__class__ in mapping:
             code, message = mapping[exc.__class__]

api/tests/subsonic/test_renderers.py

@@ -1,4 +1,5 @@
 import json
+import pytest
 import xml.etree.ElementTree as ET
 
 import funkwhale_api
@@ -6,6 +7,50 @@ import funkwhale_api
 from funkwhale_api.subsonic import renderers
 
 
+@pytest.mark.parametrize(
+    "data,expected",
+    [
+        (
+            {"hello": "world"},
+            {
+                "status": "ok",
+                "version": "1.16.0",
+                "type": "funkwhale",
+                "funkwhaleVersion": funkwhale_api.__version__,
+                "hello": "world",
+            },
+        ),
+        (
+            {
+                "hello": "world",
+                "error": {"code": 10, "message": "something went wrong"},
+            },
+            {
+                "status": "failed",
+                "version": "1.16.0",
+                "type": "funkwhale",
+                "funkwhaleVersion": funkwhale_api.__version__,
+                "hello": "world",
+                "error": {"code": 10, "message": "something went wrong"},
+            },
+        ),
+        (
+            {"hello": "world", "detail": "something went wrong"},
+            {
+                "status": "failed",
+                "version": "1.16.0",
+                "type": "funkwhale",
+                "funkwhaleVersion": funkwhale_api.__version__,
+                "hello": "world",
+                "error": {"code": 0, "message": "something went wrong"},
+            },
+        ),
+    ],
+)
+def test_structure_payload(data, expected):
+    assert renderers.structure_payload(data) == expected
+
+
 def test_json_renderer():
     data = {"hello": "world"}
     expected = {
@@ -32,7 +77,8 @@ def test_xml_renderer_dict_to_xml():
 
 def test_xml_renderer():
     payload = {"hello": "world"}
-    expected = b'<?xml version="1.0" encoding="UTF-8"?>\n<subsonic-response hello="world" status="ok" version="1.16.0" xmlns="http://subsonic.org/restapi" />'  # noqa
+    expected = '<?xml version="1.0" encoding="UTF-8"?>\n<subsonic-response funkwhaleVersion="{}" hello="world" status="ok" type="funkwhale" version="1.16.0" xmlns="http://subsonic.org/restapi" />'  # noqa
+    expected = expected.format(funkwhale_api.__version__).encode()
 
     renderer = renderers.SubsonicXMLRenderer()
     rendered = renderer.render(payload)

api/tests/subsonic/test_views.py

@@ -43,6 +43,20 @@ def test_exception_wrong_credentials(f, db, api_client):
     assert response.data == expected
 
 
+@pytest.mark.parametrize("f", ["json"])
+def test_exception_missing_credentials(f, db, api_client):
+    url = reverse("api:subsonic-get-artists")
+    response = api_client.get(url)
+
+    expected = {
+        "status": "failed",
+        "error": {"code": 10, "message": "Required parameter is missing."},
+    }
+
+    assert response.status_code == 200
+    assert response.data == expected
+
+
 def test_disabled_subsonic(preferences, api_client):
     preferences["subsonic__enabled"] = False
     url = reverse("api:subsonic-ping")

changes/changelog.d/616.bugfix

@@ -0,0 +1 @@
+Fixed inconsistencies in subsonic error responses (#616)