trim21 / transmission-rpc

https://transmission-rpc.readthedocs.io/en/stable/
MIT License
139 stars 34 forks source link

fix(client): improve error handling of empty torrent metadata #449

Closed dechamps closed 2 months ago

dechamps commented 2 months ago

If an empty torrent file (as in, zero bytes) is passed to add_torrent(), the previous code would fall over and attempt to awkwardly pass the path itself as the torrent data, resulting in a very cryptic error being raised. This commit ensures we fail early and cleanly in this case.

codecov[bot] commented 2 months ago

Codecov Report

Attention: Patch coverage is 75.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 77.67%. Comparing base (85f55c4) to head (2436200).

Files Patch % Lines
transmission_rpc/client.py 75.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #449 +/- ## ========================================== - Coverage 77.71% 77.67% -0.04% ========================================== Files 14 14 Lines 1503 1505 +2 ========================================== + Hits 1168 1169 +1 - Misses 335 336 +1 ``` | [Flag](https://app.codecov.io/gh/trim21/transmission-rpc/pull/449/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Trim21) | Coverage Δ | | |---|---|---| | [3.10](https://app.codecov.io/gh/trim21/transmission-rpc/pull/449/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Trim21) | `77.67% <75.00%> (-0.04%)` | :arrow_down: | | [3.11](https://app.codecov.io/gh/trim21/transmission-rpc/pull/449/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Trim21) | `77.67% <75.00%> (-0.04%)` | :arrow_down: | | [3.12](https://app.codecov.io/gh/trim21/transmission-rpc/pull/449/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Trim21) | `77.67% <75.00%> (-0.04%)` | :arrow_down: | | [3.8](https://app.codecov.io/gh/trim21/transmission-rpc/pull/449/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Trim21) | `77.21% <75.00%> (-0.31%)` | :arrow_down: | | [3.9](https://app.codecov.io/gh/trim21/transmission-rpc/pull/449/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Trim21) | `76.80% <75.00%> (-0.71%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Trim21#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

trim21 commented 2 months ago

please do not use force push

dechamps commented 2 months ago

please do not use force push

Okay, went back to the original commit and made a new one on top. (I usually try to keep PRs in a clean ready-to-merge state, but you're the boss…)

trim21 commented 2 months ago

please do not use force push

Okay, went back to the original commit and made a new one on top. (I usually try to keep PRs in a clean ready-to-merge state, but you're the boss…)

I prefer squash merging so any PR with good patch are clean ready-to-merge PR to me. Only PR title matters, commit history doesn't matter.

dechamps commented 2 months ago

I prefer squash merging

Yeah I guess I can see the appeal of that - it makes sense. I usually prefer rebase merging myself, though. To me the downside of squash merging is, if the PR is complex and contains several well-defined cleanly delineated commits, squash merging will drop the distinction between commits, leading to huge commits that make history hard to read. But I do concede there are valid points on both sides of this debate.