Skip to content

Conversation

@finevan
Copy link

@finevan finevan commented Nov 23, 2025

Previously the status code was not checked, so bad base paths or reverse proxies in front of unavailable qbittorrent instances would not trip the health check.

This endpoint is particularly critical because it's used for the client health check in QUI here:

https://github.com/autobrr/qui/blob/de0e00a6eda2f6988a42d4d426a5c3f960fa2063/internal/qbittorrent/client.go#L376-L388

Via this check, which only validates the response body is non-empty, where in many cases it includes a body along the lines of "404 Not Found".

https://github.com/autobrr/qui/blob/de0e00a6eda2f6988a42d4d426a5c3f960fa2063/internal/qbittorrent/client.go#L200-L226

There are several other endpoints which don't check the response status codes, I'd be happy to update them in follow-up commits.

Previously the status code was not checked, so bad base paths
or reverse proxies in front of unavailable qbittorrent instances
would not trip the health check.

There are several other endpoints which don't check the response
status codes, I'd be happy to update them in follow-up commits.
finevan added a commit to finevan/go-qbittorrent that referenced this pull request Nov 24, 2025
While most endpoints unmarshal json, which will error on unexpected
responses, verifying the status code first is less expensive, and
provides more accurate and descriptive failure messages in the logs.

I have verified each endpoints response statuses in the official
qbittorrent documentation, with the exception of `torrents/export`,
which is not documented. For this endpoint I have implemented
specific handling for 404 not found, but left the 409 conflict status
to fall through to the default unknown status branch, based on the
qbittorrent implementation:
https://github.com/qbittorrent/qBittorrent/blob/a77b17e6dafadca1eb0f80d4c0167c791107f787/src/webui/api/torrentscontroller.cpp#L2045-L2059
The qbittorrent 409 conflict status code seems questionable, as the
failure appers not to be related to the request, so it would
make more sense as a 5xx rather than 4xx status class, and I wouldn't
be surprised if this changed in the future.

This is a follow-up to autobrr#97 which handles the rest of the endpoints.
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.

1 participant