diff --git a/api/funkwhale_api/subsonic/renderers.py b/api/funkwhale_api/subsonic/renderers.py
index 95b437a554b5df42245b3ccfe33b08795e33deec..e4b6470511378bbff21d824896a4210dd84c6c43 100644
--- a/api/funkwhale_api/subsonic/renderers.py
+++ b/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 = {
- "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"
+ final = {"subsonic-response": structure_payload(data)}
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 = {
- "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"
+ final = structure_payload(data)
+ final["xmlns"] = "http://subsonic.org/restapi"
tree = dict_to_xml_tree("subsonic-response", final)
return b'<?xml version="1.0" encoding="UTF-8"?>\n' + ET.tostring(
tree, encoding="utf-8"
diff --git a/api/funkwhale_api/subsonic/views.py b/api/funkwhale_api/subsonic/views.py
index 8aa9c9dbe893d7d1e0974cfc79def18e6e12f29c..3f485bdff6a56377f43942e7c4a8be8783c7ff4c 100644
--- a/api/funkwhale_api/subsonic/views.py
+++ b/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__]
diff --git a/api/tests/subsonic/test_renderers.py b/api/tests/subsonic/test_renderers.py
index 7e977ac45deb10d900d2127a14d5edff4980930c..acd5500e665b6d4f9d32c0ea6cd9689a9bb37421 100644
--- a/api/tests/subsonic/test_renderers.py
+++ b/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)
diff --git a/api/tests/subsonic/test_views.py b/api/tests/subsonic/test_views.py
index c0ae8407362b1fca31573b4b895ed0a58ca5097f..6be5832311eba4368e3835a107b26c038b550b79 100644
--- a/api/tests/subsonic/test_views.py
+++ b/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")
diff --git a/changes/changelog.d/616.bugfix b/changes/changelog.d/616.bugfix
new file mode 100644
index 0000000000000000000000000000000000000000..fa6db30ee0c27e717078e40818c6fc34a04eff8a
--- /dev/null
+++ b/changes/changelog.d/616.bugfix
@@ -0,0 +1 @@
+Fixed inconsistencies in subsonic error responses (#616)