From d1fab11bee55fc6248e25272425f39869959e61e Mon Sep 17 00:00:00 2001
From: madhavcodez <madhavcbusiness@gmail.com>
Date: Sat, 27 Jun 2026 02:14:58 -0500
Subject: [PATCH] Fix missing return in _retryer that propagated None tiles

_retryer retries on non-404 failures, but the retry branch ran
    request = _retryer(...)
without returning the result, so the function fell through and implicitly
returned None. That None was collected as a tile array and later crashed
_merge_tiles at "h, w, d = arrays[0].shape" (AttributeError) or at the array
assignment (TypeError), which is the intermittent failure reported in #252.

Return the recursive call. As defense-in-depth, _merge_tiles now raises a clear
ValueError if any tile array is None instead of failing with an obscure
AttributeError/TypeError further down.

Add network-free tests: _merge_tiles raises the clear ValueError for a missing
tile in first and later positions; _retryer returns the array after a non-404
failure then a successful retry; and _retryer raises (not returns None) when
retries are exhausted.

Closes #252
---
 contextily/tile.py | 10 +++++++-
 tests/test_cx.py   | 61 +++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 69 insertions(+), 2 deletions(-)

diff --git a/contextily/tile.py b/contextily/tile.py
index 83e9899d..37301440 100644
--- a/contextily/tile.py
+++ b/contextily/tile.py
@@ -503,7 +503,7 @@ def _retryer(tile_url, wait, max_retries, headers: dict[str, str], timeout=None)
             if max_retries > 0:
                 time.sleep(wait)
                 max_retries -= 1
-                request = _retryer(tile_url, wait, max_retries, headers, timeout=timeout)
+                return _retryer(tile_url, wait, max_retries, headers, timeout=timeout)
             else:
                 raise requests.HTTPError("Connection reset by peer too many times. "
                                          f"Last message was: {request.status_code} "
@@ -713,6 +713,14 @@ def _merge_tiles(tiles, arrays):
     # get indices starting at zero
     indices = tile_xys - tile_xys.min(axis=0)
 
+    # guard against tiles that failed to download (see GH#252)
+    if any(arr is None for arr in arrays):
+        raise ValueError(
+            "One or more tiles could not be downloaded (the tile array is "
+            "None). Try a lower zoom level or check that the tile provider is "
+            "reachable."
+        )
+
     # the shape of individual tile images
     h, w, d = arrays[0].shape
 
diff --git a/tests/test_cx.py b/tests/test_cx.py
index 560ef471..52375205 100644
--- a/tests/test_cx.py
+++ b/tests/test_cx.py
@@ -7,7 +7,8 @@
 import mercantile as mt
 import pytest
 import rasterio as rio
-from contextily.tile import _calculate_zoom
+import requests
+from contextily.tile import _calculate_zoom, _merge_tiles, _retryer
 from numpy.testing import assert_array_almost_equal
 from unittest.mock import patch, MagicMock
 import io
@@ -1044,3 +1045,61 @@ def test_aspect():
     cx.add_basemap(ax, zoom=10)
 
     assert ax.get_aspect() == 2
+
+
+def test_merge_tiles_raises_clear_error_for_missing_tile():
+    """A tile that failed to download (None) raises a clear ValueError from
+    _merge_tiles instead of an AttributeError/TypeError (see GH#252)."""
+    tiles = [mt.Tile(0, 0, 1), mt.Tile(1, 0, 1)]
+    valid = np.zeros((256, 256, 4), dtype=np.uint8)
+    # the first tile is missing (was AttributeError on arrays[0].shape)
+    with pytest.raises(ValueError, match="could not be downloaded"):
+        _merge_tiles(tiles, [None, valid])
+    # a later tile is missing (was TypeError on the assignment)
+    with pytest.raises(ValueError, match="could not be downloaded"):
+        _merge_tiles(tiles, [valid, None])
+
+
+def test_retryer_returns_array_after_successful_retry():
+    """_retryer returns the fetched array (not None) when an initial non-404
+    failure is followed by a successful retry (see GH#252)."""
+    img_array = np.random.randint(0, 255, (256, 256, 4), dtype=np.uint8)
+    buf = io.BytesIO()
+    Image.fromarray(img_array, mode="RGBA").save(buf, format="PNG")
+
+    failed = MagicMock()
+    failed.status_code = 500
+    failed.raise_for_status.side_effect = requests.HTTPError("500 Server Error")
+
+    succeeded = MagicMock()
+    succeeded.status_code = 200
+    succeeded.content = buf.getvalue()
+    succeeded.raise_for_status = MagicMock()
+
+    with patch(
+        "contextily.tile.requests.get", side_effect=[failed, succeeded]
+    ) as mock_get:
+        result = _retryer(
+            "https://example.com/0/0/0.png", wait=0, max_retries=2, headers={}
+        )
+
+    assert mock_get.call_count == 2
+    assert isinstance(result, np.ndarray)
+    assert result.shape == (256, 256, 4)
+
+
+def test_retryer_raises_when_retries_exhausted():
+    """When retries are exhausted on a non-404 failure, _retryer raises rather
+    than returning None (guards the else-branch touched in GH#252)."""
+    failed = MagicMock()
+    failed.status_code = 500
+    failed.raise_for_status.side_effect = requests.HTTPError("500 Server Error")
+
+    with patch("contextily.tile.requests.get", return_value=failed):
+        with pytest.raises(requests.HTTPError):
+            _retryer(
+                "https://example.com/0/0/0.png",
+                wait=0,
+                max_retries=0,
+                headers={},
+            )