Skip to content

accept JSON add-torrent responses from qBittorrent#54

Open
sgerner wants to merge 2 commits into
JeremiahM37:mainfrom
sgerner:codex/qbittorrent-json-response
Open

accept JSON add-torrent responses from qBittorrent#54
sgerner wants to merge 2 commits into
JeremiahM37:mainfrom
sgerner:codex/qbittorrent-json-response

Conversation

@sgerner
Copy link
Copy Markdown

@sgerner sgerner commented Jun 1, 2026

qBittorrent returns a JSON success payload on add-torrent in this environment, and librarr was treating anything other than literal Ok. as a failure.

What changed:

  • Accept JSON add-torrent success responses that include success_count or added_torrent_ids.
  • Keep rejecting actual failures and surface the qBittorrent error string.
  • Add focused regression tests for success and failure responses.

Validation:

  • go test ./internal/download -run 'TestQBittorrentAddTorrentAcceptsJSONSuccess|TestQBittorrentAddTorrentRejectsJSONFailure'

@sgerner sgerner marked this pull request as ready for review June 1, 2026 00:36
@JeremiahM37 JeremiahM37 self-assigned this Jun 1, 2026
Copy link
Copy Markdown
Owner

@JeremiahM37 JeremiahM37 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The qbittorrent.go change itself looks fine. The test file is the problem — see inline.

func TestLogin_QB4x_OkBody(t *testing.T) {
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
http.SetCookie(w, &http.Cookie{Name: "SID", Value: "session4x"})
func TestQBittorrentAddTorrentAcceptsJSONSuccess(t *testing.T) {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This drops ~210 lines of tests that have nothing to do with the new JSON parsing:

  • TestLogin_QB4x_OkBody
  • TestLogin_QB5x_NoContentWithSessionCookie
  • TestLogin_QB5x_EmptyBodyWithSessionCookie
  • TestLogin_QB4x_FailsBody
  • TestLogin_Banned
  • TestLogin_EmptyBodyNoCookieFails
  • TestMapTorrentStatus / TestMapSABStatus / TestValidTransitions

Can you restore these verbatim? Losing TestLogin_Banned in particular is a regression — qBit IP-bans on failed logins and that test guards against silently breaking the detection.

Also worth adding to the new code: a test for the literal "Ok." success path (stock qBit 4.x), and one for a non-2xx HTTP response.

@JeremiahM37 JeremiahM37 assigned sgerner and unassigned JeremiahM37 Jun 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants