Improves SubsonicUpdate error messages when server is unavailable #5635 #6197

jasling920 · 2025-12-01T11:11:47Z

Description

Fixes #5635: subsonicupdate fails cryptically when server is not available

The plugin previously produced unhelpful JSON-parsing errors when the server was down or returned unexpected data. This PR improves the error handling so that these situations now produce clear, user-friendly messages instead of cryptic failures such as:

Error: Expecting value: line 1 column 1 (char 0)

This update adds explicit handling for:

HTTP connection errors and timeouts
Invalid or non-JSON responses
Missing subsonic-response fields in returned data

Mocked Subsonic responses confirm the improved behavior:

Normal response --> unchanged
Timeout/connection error --> now produce a clear and readable error
Invalid JSON --> now reported as “Invalid JSON from Subsonic” and shows the server’s returned text for context
Missing subsonic-response --> now produces a clear message identifying that the required field is missing

These changes ensure that subsonicupdate failures are now clearly reported and no longer cryptic.

Documentation. (Only changelog was updated. No additional docs required.)
Changelog. (Added an entry to docs/changelog.rst.)
Tests. While no formal unit tests were added, the fix was verified using local mock servers simulating various Subsonic server conditions:
Normal response: Verified scanning proceeds normally with a valid server response (test_subsonic.py).
Timeout: Verified that server timeout produces a clear, user-friendly error message (test_subsonic_timeout.py).
Invalid JSON: Verified that invalid JSON produces a clear error (test_subsonic_invalid_json.py).
Missing subsonic-response field: Verified that a clear error message is shown when the expected field is missing (test_subsonic_missing_response.py).

sourcery-ai

Hey there - I've reviewed your changes - here's some feedback:

When logging a missing 'subsonic-response' field, consider truncating or summarizing the JSON payload (similar to the invalid JSON case) to avoid excessively large log entries for big responses.
Using resp.get("scanStatus", {}).get("count", 0) silently falls back to 0 when scanStatus or count is missing; it may be more helpful to log a warning or treat this as an error so that unexpected server responses are visible rather than hidden.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- When logging a missing 'subsonic-response' field, consider truncating or summarizing the JSON payload (similar to the invalid JSON case) to avoid excessively large log entries for big responses.
- Using `resp.get("scanStatus", {}).get("count", 0)` silently falls back to 0 when `scanStatus` or `count` is missing; it may be more helpful to log a warning or treat this as an error so that unexpected server responses are visible rather than hidden.

## Individual Comments

### Comment 1
<location> `beetsplug/subsonicupdate.py:141-145` </location>
<code_context>
+            try:
+                json = response.json()
+            except ValueError:
+                self._log.error("Invalid JSON from Subsonic: {}", response.text[:200])
+                return
+            resp = json.get("subsonic-response")
</code_context>

<issue_to_address>
**suggestion:** Consider including the HTTP status code when logging invalid JSON responses.

The current log only shows a truncated body, which makes it hard to tell transient HTML error pages from truly malformed JSON. Logging `response.status_code` (and optionally the URL) alongside the body would make these issues easier to diagnose without adding much log noise.

```suggestion
            try:
                json = response.json()
            except ValueError:
                self._log.error(
                    "Invalid JSON from Subsonic (status {} for {}): {}",
                    response.status_code,
                    getattr(response, "url", "<unknown-url>"),
                    response.text[:200],
                )
                return
```
</issue_to_address>

### Comment 2
<location> `docs/changelog.rst:34-35` </location>
<code_context>
 Bug fixes:
-
+- :doc:`plugins/subsonicupdate`: Improve error messages when the Subsonic server 
+  is unavailable or returns invalid/missing JSON. Previously, failures were 
+  cryptic (e.g., "Expecting value: line 1 column 1 (char 0)"). :bug:`5635`
 - :doc:`plugins/inline`: Fix recursion error when an inline field definition
</code_context>

<issue_to_address>
**suggestion (typo):** Clarify the phrasing "invalid/missing JSON".

Consider changing this to `invalid or missing JSON` for clearer, more formal wording while preserving the meaning.

```suggestion
- :doc:`plugins/subsonicupdate`: Improve error messages when the Subsonic server 
-  is unavailable or returns invalid or missing JSON. Previously, failures were 
```
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

sourcery-ai · 2025-12-01T11:12:43Z

beetsplug/subsonicupdate.py

+            try:
+                json = response.json()
+            except ValueError:
+                self._log.error("Invalid JSON from Subsonic: {}", response.text[:200])
+                return


suggestion: Consider including the HTTP status code when logging invalid JSON responses.

The current log only shows a truncated body, which makes it hard to tell transient HTML error pages from truly malformed JSON. Logging response.status_code (and optionally the URL) alongside the body would make these issues easier to diagnose without adding much log noise.

Suggested change

try:

json = response.json()

except ValueError:

self._log.error("Invalid JSON from Subsonic: {}", response.text[:200])

return

try:

json = response.json()

except ValueError:

self._log.error(

"Invalid JSON from Subsonic (status {} for {}): {}",

response.status_code,

getattr(response, "url", "<unknown-url>"),

response.text[:200],

)

return

sourcery-ai · 2025-12-01T11:12:43Z

docs/changelog.rst

+- :doc:`plugins/subsonicupdate`: Improve error messages when the Subsonic server 
+  is unavailable or returns invalid/missing JSON. Previously, failures were 


suggestion (typo): Clarify the phrasing "invalid/missing JSON".

Consider changing this to invalid or missing JSON for clearer, more formal wording while preserving the meaning.

Suggested change

- :doc:`plugins/subsonicupdate`: Improve error messages when the Subsonic server

is unavailable or returns invalid/missing JSON. Previously, failures were

- :doc:`plugins/subsonicupdate`: Improve error messages when the Subsonic server

- is unavailable or returns invalid or missing JSON. Previously, failures were

semohr

Thank you for the PR and initiative!

Since this seems to be your first contribution you might want to checkout our contribution guide. We are happy to have you onboard!

I’ve left a few comments that should help improve this PR.

semohr · 2025-12-02T13:01:43Z

beetsplug/subsonicupdate.py

+            except ValueError:
+                self._log.error("Invalid JSON from Subsonic: {}", response.text[:200])
+                return
+            resp = json.get("subsonic-response")


The can be simplified to:

if not (resp := json.get("subsonic-response")): self._log.error("Missing 'subsonic-response' field: {}", json) return

semohr · 2025-12-02T13:04:11Z

beetsplug/subsonicupdate.py

                params=payload,
                timeout=10,
            )
-            json = response.json()


Typically we should do response.raise_for_status() here and handle errors properly in the try expect block below. Should also allow us to remove the if ( response.status_code == 200) conditional.

semohr · 2025-12-02T13:05:21Z

beetsplug/subsonicupdate.py

+            if not resp:
+                self._log.error("Missing 'subsonic-response' field: {}", json)
+                return
+            status = resp.get("status")


Do we have to be defensive here? The same applies to resp.get("scanStatus", {}).get("count", 0) below.

…rver is unavailable

codecov · 2025-12-04T16:31:11Z

Codecov Report

❌ Patch coverage is 60.00000% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 67.90%. Comparing base (2bd77b9) to head (948cdac).
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
beetsplug/subsonicupdate.py	60.00%	4 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #6197      +/-   ##
==========================================
- Coverage   67.93%   67.90%   -0.03%     
==========================================
  Files         137      137              
  Lines       18677    18687      +10     
  Branches     3155     3156       +1     
==========================================
+ Hits        12688    12690       +2     
- Misses       5324     5330       +6     
- Partials      665      667       +2

Files with missing lines	Coverage Δ
beetsplug/subsonicupdate.py	`68.49% <60.00%> (-7.70%)`	⬇️

🚀 New features to boost your workflow:

📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

jasling920 requested a review from a team as a code owner December 1, 2025 11:11

sourcery-ai bot reviewed Dec 1, 2025

View reviewed changes

jasling920 changed the title ~~Fixed Issue #5635: improve SubsonicUpdate error messages when server is unavailable~~ Improves SubsonicUpdate error messages when server is unavailable #5635 Dec 1, 2025

semohr requested changes Dec 2, 2025

View reviewed changes

jaslingm added 2 commits December 4, 2025 11:23

Fix Issue beetbox#5635: improve SubsonicUpdate error messages when se…

30cd320

…rver is unavailable

Add changelog entry for beetbox#5635 subsonicupdate error messages

6e08aa0

jasling920 force-pushed the fix-subsonicupdate branch from ef8dd5a to 6e08aa0 Compare December 4, 2025 16:25

jaslingm added 4 commits December 4, 2025 11:36

Apply formatting and fix lint issues

a81456a

Simplifies subsonicupdate JSON handling and updated changelog

51be7be

Simplify SubsonicUpdate JSON handling and fix changelog formatting

5f1975c

Fix subsonicupdate entry in changelog

948cdac

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Improves SubsonicUpdate error messages when server is unavailable #5635 #6197

Improves SubsonicUpdate error messages when server is unavailable #5635 #6197

Uh oh!

jasling920 commented Dec 1, 2025 •

edited

Loading

Uh oh!

sourcery-ai bot left a comment

Uh oh!

sourcery-ai bot Dec 1, 2025

Uh oh!

sourcery-ai bot Dec 1, 2025

Uh oh!

semohr left a comment

Uh oh!

semohr Dec 2, 2025

Uh oh!

semohr Dec 2, 2025

Uh oh!

semohr Dec 2, 2025

Uh oh!

codecov bot commented Dec 4, 2025 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		- :doc:`plugins/subsonicupdate`: Improve error messages when the Subsonic server
		is unavailable or returns invalid/missing JSON. Previously, failures were

Improves SubsonicUpdate error messages when server is unavailable #5635 #6197

Are you sure you want to change the base?

Improves SubsonicUpdate error messages when server is unavailable #5635 #6197

Uh oh!

Conversation

jasling920 commented Dec 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

sourcery-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

sourcery-ai bot Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

sourcery-ai bot Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

semohr left a comment

Choose a reason for hiding this comment

Uh oh!

semohr Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

semohr Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

semohr Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Dec 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jasling920 commented Dec 1, 2025 •

edited

Loading

codecov bot commented Dec 4, 2025 •

edited

Loading