Send Subsonic formatted response on marshalling errors

server/subsonic/api.go

@@ -233,7 +233,7 @@ func h501(r chi.Router, paths ...string) {
 	for _, path := range paths {
 		handle := func(w http.ResponseWriter, r *http.Request) {
 			w.Header().Add("Cache-Control", "no-cache")
-			w.WriteHeader(501)
+			w.WriteHeader(http.StatusNotImplemented)
 			_, _ = w.Write([]byte("This endpoint is not implemented, but may be in future releases"))
 		}
 		addHandler(r, path, handle)
@@ -244,7 +244,7 @@ func h501(r chi.Router, paths ...string) {
 func h410(r chi.Router, paths ...string) {
 	for _, path := range paths {
 		handle := func(w http.ResponseWriter, r *http.Request) {
-			w.WriteHeader(410)
+			w.WriteHeader(http.StatusGone)
 			_, _ = w.Write([]byte("This endpoint will not be implemented"))
 		}
 		addHandler(r, path, handle)
@@ -276,7 +276,7 @@ func mapToSubsonicError(err error) subError {
 func sendError(w http.ResponseWriter, r *http.Request, err error) {
 	subErr := mapToSubsonicError(err)
 	response := newResponse()
-	response.Status = "failed"
+	response.Status = responses.StatusFailed
 	response.Error = &responses.Error{Code: int32(subErr.code), Message: subErr.Error()}
 
 	sendResponse(w, r, response)
@@ -305,11 +305,10 @@ func sendResponse(w http.ResponseWriter, r *http.Request, payload *responses.Sub
 	// This should never happen, but if it does, we need to know
 	if err != nil {
 		log.Error(r.Context(), "Error marshalling response", "format", f, err)
-		w.WriteHeader(http.StatusInternalServerError)
-		_, _ = w.Write([]byte("Internal Server Error: " + err.Error()))
+		sendError(w, r, err)
 		return
 	}
-	if payload.Status == "ok" {
+	if payload.Status == responses.StatusOK {
 		if log.IsGreaterOrEqualTo(log.LevelTrace) {
 			log.Debug(r.Context(), "API: Successful response", "endpoint", r.URL.Path, "status", "OK", "body", string(response))
 		} else {

server/subsonic/api_test.go

@@ -24,12 +24,12 @@ var _ = Describe("sendResponse", func() {
 		w = httptest.NewRecorder()
 		r = httptest.NewRequest("GET", "/somepath", nil)
 		payload = &responses.Subsonic{
-			Status:  "ok",
+			Status:  responses.StatusOK,
 			Version: "1.16.1",
 		}
 	})
 
-	Context("when format is JSON", func() {
+	When("format is JSON", func() {
 		It("should set Content-Type to application/json and return the correct body", func() {
 			q := r.URL.Query()
 			q.Add("f", "json")
@@ -48,7 +48,7 @@ var _ = Describe("sendResponse", func() {
 		})
 	})
 
-	Context("when format is JSONP", func() {
+	When("format is JSONP", func() {
 		It("should set Content-Type to application/javascript and return the correct callback body", func() {
 			q := r.URL.Query()
 			q.Add("f", "jsonp")
@@ -60,8 +60,8 @@ var _ = Describe("sendResponse", func() {
 			Expect(w.Header().Get("Content-Type")).To(Equal("application/javascript"))
 			body := w.Body.String()
 			Expect(body).To(SatisfyAll(
-				ContainSubstring("testCallback("),
-				ContainSubstring(")"),
+				HavePrefix("testCallback("),
+				HaveSuffix(")"),
 			))
 
 			// Extract JSON from the JSONP response
@@ -73,7 +73,7 @@ var _ = Describe("sendResponse", func() {
 		})
 	})
 
-	Context("when format is XML or unspecified", func() {
+	When("format is XML or unspecified", func() {
 		It("should set Content-Type to application/xml and return the correct body", func() {
 			// No format specified, expecting XML by default
 			sendResponse(w, r, payload)
@@ -87,19 +87,25 @@ var _ = Describe("sendResponse", func() {
 		})
 	})
 
-	Context("when an error occurs during marshalling", func() {
-		It("should return HTTP 500", func() {
+	When("an error occurs during marshalling", func() {
+		It("should return a fail response", func() {
 			payload.Song = &responses.Child{
+				// An +Inf value will cause an error when marshalling to JSON
 				ReplayGain: responses.ReplayGain{TrackGain: math.Inf(1)},
-			} // This will cause an error when marshalling to JSON
+			}
 			q := r.URL.Query()
 			q.Add("f", "json")
 			r.URL.RawQuery = q.Encode()
+
 			sendResponse(w, r, payload)
 
-			Expect(w.Code).To(Equal(http.StatusInternalServerError))
-			body := w.Body.String()
-			Expect(body).To(ContainSubstring("Internal Server Error"))
+			Expect(w.Code).To(Equal(http.StatusOK))
+			var wrapper responses.JsonWrapper
+			err := json.Unmarshal(w.Body.Bytes(), &wrapper)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(wrapper.Subsonic.Status).To(Equal(responses.StatusFailed))
+			Expect(wrapper.Subsonic.Version).To(Equal(payload.Version))
+			Expect(wrapper.Subsonic.Error.Message).To(ContainSubstring("json: unsupported value: +Inf"))
 		})
 	})
 

server/subsonic/helpers.go

@@ -19,7 +19,7 @@ import (
 
 func newResponse() *responses.Subsonic {
 	return &responses.Subsonic{
-		Status:        "ok",
+		Status:        responses.StatusOK,
 		Version:       Version,
 		Type:          consts.AppName,
 		ServerVersion: consts.Version,

server/subsonic/responses/responses.go

@@ -61,6 +61,11 @@ type Subsonic struct {
 	LyricsList             *LyricsList             `xml:"lyricsList,omitempty" json:"lyricsList,omitempty"`
 }
 
+const (
+	StatusOK     = "ok"
+	StatusFailed = "failed"
+)
+
 type JsonWrapper struct {
 	Subsonic Subsonic `json:"subsonic-response"`
 }

server/subsonic/responses/responses_test.go

@@ -20,7 +20,7 @@ var _ = Describe("Responses", func() {
 	var response *Subsonic
 	BeforeEach(func() {
 		response = &Subsonic{
-			Status:        "ok",
+			Status:        StatusOK,
 			Version:       "1.8.0",
 			Type:          consts.AppName,
 			ServerVersion: "v0.0.0",