_make_request decodes 204 as an error: every `result is not None` delete reports failure on success (systemic, 6 modules) #20

New issue

Open

opened 2026-10-01 16:40:37 +02:00 by kade · 0 comments

kade commented

2026-10-01 16:40:37 +02:00

Owner

Severity: high (systemic false negative)

Every DELETE answers 204 No Content. _make_request called
response.json() unconditionally, which raises on an empty body; the resulting
exception was caught by the blanket except Exception handler, which printed
Error making request: Expecting value: line 1 column 1 (char 0) and returned
None.

Every module that reports success with the idiom return result is not None
therefore returned False on a delete that succeeded. Live proof: deleting a
throwaway label printed that JSON error while the label was in fact gone and the
request returned 204.

Affected call sites (is not None on a possibly-204 result):

modules/issues.py:294 (close_issue), modules/issues.py:534
modules/branches.py:46
modules/projects.py:75 (delete_card; see also the swapped-args note there)
modules/labels.py (delete_label)

Fix (implemented in the working tree)

Treat an empty body as a successful, empty result:

response.raise_for_status()
result = {} if not response.content else response.json()

{} is not None so is not None now means success, and {} stays falsy so
if result else [] list callers are unaffected. Genuine failures (4xx/5xx via
raise_for_status) still return None.

Tests (tests/test_labels.py, new)

test_delete_204_reports_success -- asserts the 204 yields non-None and that
.json() was never called on the empty body.
test_delete_204_is_truthy_neutral_for_list_callers -- {} == {} and stays falsy.
test_failure_still_returns_none -- a 404 still returns None (guard against
over-correcting the fix into "always succeed").

## Severity: high (systemic false negative) Every DELETE answers `204 No Content`. `_make_request` called `response.json()` unconditionally, which raises on an empty body; the resulting exception was caught by the blanket `except Exception` handler, which printed `Error making request: Expecting value: line 1 column 1 (char 0)` and returned `None`. Every module that reports success with the idiom `return result is not None` therefore returned `False` on a delete that **succeeded**. Live proof: deleting a throwaway label printed that JSON error while the label was in fact gone and the request returned 204. Affected call sites (`is not None` on a possibly-204 result): - `modules/issues.py:294` (close_issue), `modules/issues.py:534` - `modules/branches.py:46` - `modules/projects.py:75` (`delete_card`; see also the swapped-args note there) - `modules/labels.py` (delete_label) ## Fix (implemented in the working tree) Treat an empty body as a successful, empty result: ```python response.raise_for_status() result = {} if not response.content else response.json() ``` `{}` is not `None` so `is not None` now means success, and `{}` stays falsy so `if result else []` list callers are unaffected. Genuine failures (4xx/5xx via `raise_for_status`) still return `None`. ## Tests (tests/test_labels.py, new) - `test_delete_204_reports_success` -- asserts the 204 yields non-None *and* that `.json()` was never called on the empty body. - `test_delete_204_is_truthy_neutral_for_list_callers` -- `{} == {}` and stays falsy. - `test_failure_still_returns_none` -- a 404 still returns `None` (guard against over-correcting the fix into "always succeed").

kade added the

pkg:ai

label

2026-10-01 16:40:37 +02:00