Skip to content

[NEEDS CODE REVIEWER] Handle failed /sync/maindata requests in check_connected#342

Open
jamtur01 wants to merge 2 commits intoManiMatter:devfrom
jamtur01:fix/handle-sync-maindata-500
Open

[NEEDS CODE REVIEWER] Handle failed /sync/maindata requests in check_connected#342
jamtur01 wants to merge 2 commits intoManiMatter:devfrom
jamtur01:fix/handle-sync-maindata-500

Conversation

@jamtur01
Copy link
Copy Markdown

@jamtur01 jamtur01 commented Apr 18, 2026

Previously, an HTTP error from the qBit API (e.g. a 500 from rdt-client) would crash decluttarr entirely. Now the error is caught and treated as a disconnected client, so the cycle is skipped and retried next run.

Here's an example of what happens:

decluttarr   | ERROR   | HTTP error occurred: 500 Server Error: Internal Server Error for url: http://somerdtclient:6500/api/v2/sync/maindata
decluttarr   | Traceback (most recent call last):
decluttarr   |   File "/app/src/utils/common.py", line 79, in make_request
decluttarr   |     response.raise_for_status()
decluttarr   |   File "/usr/local/lib/python3.10/site-packages/requests/models.py", line 1026, in raise_for_status
decluttarr   |     raise HTTPError(http_error_msg, response=self)
decluttarr   | requests.exceptions.HTTPError: 500 Server Error: Internal Server Error for url: http://somerdtclient:6500/api/v2/sync/maindata
decluttarr   | Traceback (most recent call last):
decluttarr   |   File "/app/main.py", line 79, in <module>
decluttarr   |     asyncio.run(main())
decluttarr   |   File "/usr/local/lib/python3.10/asyncio/runners.py", line 44, in run
decluttarr   |     return loop.run_until_complete(main)
decluttarr   |   File "/usr/local/lib/python3.10/asyncio/base_events.py", line 649, in run_until_complete
decluttarr   |     return future.result()
decluttarr   |   File "/app/main.py", line 67, in main
decluttarr   |     await job_manager.run_jobs(arr)
decluttarr   |   File "/app/src/job_manager.py", line 27, in run_jobs
decluttarr   |     await self.removal_jobs()
decluttarr   |   File "/app/src/job_manager.py", line 73, in removal_jobs
decluttarr   |     if not await self._download_clients_connected():
decluttarr   |   File "/app/src/job_manager.py", line 130, in _download_clients_connected
decluttarr   |     if not await self._check_client_connection_status(clients):
decluttarr   |   File "/app/src/job_manager.py", line 139, in _check_client_connection_status
decluttarr   |     if not await client.check_connected():
decluttarr   |   File "/app/src/settings/_download_clients_qbit.py", line 233, in check_connected
decluttarr   |     await make_request(
decluttarr   |   File "/app/src/utils/common.py", line 79, in make_request
decluttarr   |     response.raise_for_status()
decluttarr   |   File "/usr/local/lib/python3.10/site-packages/requests/models.py", line 1026, in raise_for_status
decluttarr   |     raise HTTPError(http_error_msg, response=self)
decluttarr   | requests.exceptions.HTTPError: 500 Server Error: Internal Server Error for url: http://somerdtclient:6500/api/v2/sync/maindata
decluttarr exited with code 1

This closes #348

Previously, an HTTP error from the qBit API (e.g. a 500 from
rdt-client) would crash decluttarr entirely. Now the error is
caught and treated as a disconnected client, so the cycle is
skipped and retried next run.
@ManiMatter
Copy link
Copy Markdown
Owner

hi, I am truly sorry I haven't looked into your PR in such a long time. I do appreciate very much that you took the time to contribute.

Unfortunately, I don't have the time to look into it still.
To overcome me being the bottleneck, I am looking to open this repo up to other people who help maintain it, and contributors can review each others code / merge.

Would you be willing to act as a formal contributor? If yes, I will add you, and if I find others (from open PRs), hopefully you can review each others PR and they can be merged.

Thanks for letting me know, and apologies again for my radio silence.

@ManiMatter ManiMatter changed the title Handle failed /sync/maindata requests in check_connected [NEEDS CODE REVIEWER] Handle failed /sync/maindata requests in check_connected Apr 21, 2026
@lolimmlost
Copy link
Copy Markdown
Collaborator

Hey @jamtur01, Thanks for contributing! I am going through the open PRs with @ManiMatter to clear the backlog.

Fix is clean and scoped:
Catching HTTPError in check_connected and treating it as a disconnected client is the right shape — removal_jobs will skip just this client and retry next cycle instead of taking the whole app down. 17/10 in one file, no collateral.

Ask — issue reference:
Is there a specific issue this fixes? Didn't find one in the open list. If not, would you mind opening a bug issue with that traceback so we have something to attach Closes #N to in the PR body? Useful for the changelog and for users searching for the rdt-client/500 error.

Heads-up — relationship to #333:
#333 wraps removal_jobs in a broader try/except RequestException as a top-level safety net. Your handling is more granular — flags just this client as disconnected and lets the rest of the cycle continue — so they're complementary rather than competing. Both should land. Flagging in case of merge-order conflicts.

Otherwise all checks out!

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.

HTTP error from the qBit API crashes decluttarr

3 participants