BrainLesion / BrainLesion/preprocessing

[BUG] HTTP error responses from Zenodo bypass the "use local copy" fallback

Open
#190 0 comments 0 reactions 0 assignees Claimed by @ErikGro View on GitHub
bug
Dominant language
C
Stars
39
Forks
10
PR merge metrics
No merged PRs in 30d

Description

**Describe the bug**

`ZenodoRecord._fetch_uncached()` contains a fallback that uses the already-downloaded local
copy when Zenodo cannot be reached:

```python
if not zenodo_response:
logger.warning(f"Zenodo unreachable. Using latest downloaded {self.label}.")
return self.target_dir / latest_local
```

This branch is unreachable for HTTP error responses. In `_get_metadata_and_archive_url()`,
the non-200 case raises `ZenodoException` from *inside* the `try` block, while the only
`except` clause catches `requests.exceptions.RequestException`. `ZenodoException` does not
inherit from it, so the exception propagates out of `fetch()` instead of being converted
into the `None` that triggers the fallback.

Consequently `None` is only ever returned for connection-level failures (DNS, connection
reset, read timeout). Any HTTP error status — 502, 504, 407, 429 — aborts the call even
though a valid, complete atlas folder is present on disk.

In practice this made a long batch job fail on ~30 of 201 subjects during transient Zenodo
gateway errors (502/504), each time with the atlases sitting unused in
`registration/atlases/15236131_v2.0.0`.

**To Reproduce**

Steps to reproduce the behavior:

1. Make sure the atlases have been downloaded once, so a valid local copy exists:
```sh
ls "$(python -c 'import brainles_preprocessing, os; print(os.path.dirname(brainles_preprocessing.__file__))')/registration/atlases"
# 15236131_v2.0.0
```
2. Install `brainles-preprocessing`
3. Simulate any non-200 response from the Zenodo API (equivalent to the 502/504 we hit):
```python
import brainles_preprocessing.utils.zenodo as zen

zen.ZenodoRecord.BASE_URL = "https://zenodo.org/api/records-this-path-does-not-exist"
print(zen.fetch_atlases())
```

Result:

```
ERROR | Cannot find record '15236131' on Zenodo (response.status_code=404).
brainles_preprocessing.utils.zenodo.ZenodoException: Cannot find record '15236131' on Zenodo (response.status_code=404).
```

**Expected behavior**

The local copy is used and a path is returned, i.e. the same behaviour as for a
connection-level failure:

```
WARNING | Zenodo returned 404 for record 15236131
INFO | Found local atlases: 15236131_v2.0.0
WARNING | Zenodo unreachable. Using latest downloaded atlases.
.../registration/atlases/15236131_v2.0.0
```

A `ZenodoException` should only be raised when Zenodo is unusable *and* there is no valid
local copy to fall back to.

**Screenshots**

n/a

**Environment**

### operating system and version?

Fedora release 44

### NVIDIA drivers and GPUs

Not relevant to this bug (it fails before any GPU work)

### Python environment and version?

Conda environment with Python 3.12.

### version of brainles_preprocessing ?

0.6.10

I will try to reproduce the bug on the latest 0.6.13 version, and see if parts or all bugs are fixed already.

**Additional context**

Real log lines from the affected run (the job continues to the next subject, so the same
failure repeats whenever Zenodo has a bad minute):

```
2026-09-02 17:13:47.761 | ERROR | brainles_preprocessing.utils.zenodo:_get_metadata_and_archive_url:144 - Cannot find record '15236131' on Zenodo (response.status_code=502).
2026-09-02 17:14:18.286 | ERROR | brainles_preprocessing.utils.zenodo:_get_metadata_and_archive_url:144 - Cannot find record '15236131' on Zenodo (response.status_code=504).
```

Three smaller issues in the same file, happy to fold them into the same PR or split them out:

1. Neither `requests.get()` call passes a `timeout`, so a stalled connection (e.g. behind an
authenticating HTTP proxy) can hang the process indefinitely.
2. `_get_latest_version_folder_name()` inspects only the newest matching folder and returns
`None` if it is empty. Since `_download()` does `folder.mkdir(...)` *before* fetching the
archive, a failed download leaves an empty `_v` directory that then
shadows an intact older copy — turning a recoverable state into
`"... not found locally and Zenodo could not be reached."` Staging the download in a temp
directory and moving it into place only on success would avoid creating the empty folder
at all.
3. The "new version available" path calls `shutil.rmtree()` on the local copy *before*
downloading the replacement, so a failure mid-upgrade leaves no usable copy.

The error text `Cannot find record '...' on Zenodo (response.status_code=502)` is also
misleading — 502/504 are gateway errors and say nothing about whether the record exists.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.