Improve behavior on getting error 404 during offline regions loading

CHANGELOG.md

@@ -26,6 +26,10 @@
 
 - [android] Add jni binding for min and max pitch ([#16236](https://github.com/mapbox/mapbox-gl-native/pull/16236))
 
+- [offline] Offline tool does not hang on 404 error ([#16240](https://github.com/mapbox/mapbox-gl-native/pull/16240))
+
+  The missing resource gets skipped and teh offline region download continues.
+
 ##### ⚠️  Breaking changes
 
 - Changes to `mbgl::FileSourceManager::getFileSource()` ([#16238](https://github.com/mapbox/mapbox-gl-native/pull/16238))

platform/default/src/mbgl/storage/offline_download.cpp

@@ -484,6 +484,13 @@ void OfflineDownload::ensureResource(Resource&& resource,
         *fileRequestsIt = onlineFileSource.request(resource, [=](Response onlineResponse) {
             if (onlineResponse.error) {
                 observer->responseError(*onlineResponse.error);
+                if (onlineResponse.error->reason == Response::Error::Reason::NotFound) {
+                    // On error 404, we skip this request and go further.
+                    requests.erase(fileRequestsIt);
+                    assert(status.requiredResourceCount > 0);
+                    status.requiredResourceCount--;
+                    continueDownload();
+                }
                 return;
             }
 

platform/default/src/mbgl/storage/online_file_source.cpp

@@ -459,6 +459,15 @@ void OnlineFileRequest::schedule(optional<Timestamp> expires) {
     });
 }
 
+namespace {
+
+inline std::string sanitizeURL(std::string& url) {
+    std::size_t queryIndex = url.find('?');
+    return (queryIndex != std::string::npos) ? std::string(url, 0, queryIndex) : url;
+}
+
+} // namespace
+
 void OnlineFileRequest::completed(Response response) {
     // If we didn't get various caching headers in the response, continue using the
     // previous values. Otherwise, update the previous values to the new values.
@@ -498,6 +507,9 @@ void OnlineFileRequest::completed(Response response) {
     }
 
     if (response.error) {
+        if (response.error->reason == Response::Error::Reason::NotFound) {
+            Log::Error(Event::General, "The resource `%s` not found", sanitizeURL(resource.url).c_str());
+        }
         failedRequests++;
         failedRequestReason = response.error->reason;
         retryAfter = response.error->retryAfter;

test/storage/offline_download.test.cpp

@@ -1059,6 +1059,75 @@ TEST(OfflineDownload, NoFreezingOnCachedTilesAndNewStyle) {
     download.setObserver(std::move(observer));
     download.setState(OfflineRegionDownloadState::Active);
 
+    test.loop.run();
+    // Passes if does not freeze.
+}
+
+TEST(OfflineDownload, NoFreezingOnNotFoundError) {
+    OfflineTest test;
+    auto region = test.createRegion();
+    ASSERT_TRUE(region);
+    OfflineDownload download(region->getID(),
+                             OfflineTilePyramidRegionDefinition(
+                                 "http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0, true),
+                             test.db,
+                             test.fileSource);
+
+    test.fileSource.styleResponse = [&](const Resource& resource) {
+        EXPECT_EQ("http://127.0.0.1:3000/style.json", resource.url);
+        return test.response("style.json");
+    };
+
+    test.fileSource.spriteImageResponse = [&](const Resource& resource) {
+        EXPECT_TRUE(resource.url == "http://127.0.0.1:3000/sprite.png" ||
+                    resource.url == "http://127.0.0.1:3000/sprite@2x.png");
+        return test.response("sprite.png");
+    };
+
+    test.fileSource.imageResponse = [&](const Resource& resource) {
+        EXPECT_EQ("http://127.0.0.1:3000/radar.gif", resource.url);
+        return test.response("radar.gif");
+    };
+
+    test.fileSource.spriteJSONResponse = [&](const Resource&) {
+        Response response;
+        response.error = std::make_unique<Response::Error>(Response::Error::Reason::NotFound);
+        return response;
+    };
+
+    test.fileSource.glyphsResponse = [&](const Resource&) { return test.response("glyph.pbf"); };
+
+    test.fileSource.sourceResponse = [&](const Resource& resource) {
+        EXPECT_EQ("http://127.0.0.1:3000/streets.json", resource.url);
+        return test.response("streets.json");
+    };
+
+    test.fileSource.tileResponse = [&](const Resource& resource) {
+        const Resource::TileData& tile = *resource.tileData;
+        EXPECT_EQ("http://127.0.0.1:3000/{z}-{x}-{y}.vector.pbf", tile.urlTemplate);
+        EXPECT_EQ(1, tile.pixelRatio);
+        EXPECT_EQ(0, tile.x);
+        EXPECT_EQ(0, tile.y);
+        EXPECT_EQ(0, tile.z);
+        return test.response("0-0-0.vector.pbf");
+    };
+
+    auto observer = std::make_unique<MockObserver>();
+    bool errorReported = false;
+    observer->responseErrorFn = [&](Response::Error error) {
+        EXPECT_EQ(Response::Error::Reason::NotFound, error.reason);
+        errorReported = true;
+    };
+    observer->statusChangedFn = [&](OfflineRegionStatus status) {
+        if (status.complete()) {
+            EXPECT_TRUE(errorReported);
+            test.loop.stop();
+        }
+    };
+
+    download.setObserver(std::move(observer));
+    download.setState(OfflineRegionDownloadState::Active);
+
     test.loop.run();
     // Passes if does not freeze.
 }