Record gdalcubes read failures after open; drop items without a mask asset (#99)

R/dft_stac_composite.R

@@ -313,6 +313,10 @@ dft_stac_composite <- function(aoi,
     # it, keeping the years already built (and cached) before it.
     tryCatch(
       build(),
+      # before drift_no_items, which it subclasses: more months cannot fix it
+      drift_no_mask_items = function(e) {
+        composite_skip(label, "no scene has the cloud-mask asset")
+      },
       drift_no_items = function(e) composite_skip(label, "no scenes"),
       drift_empty_cube = function(e) composite_skip(label, "no clear pixels")
     )
@@ -322,8 +326,9 @@ dft_stac_composite <- function(aoi,
   if (!length(out)) {
     cli::cli_abort(c(
       "No year produced a composite.",
-      "i" = "Every window had no scenes or no clear pixels; try more \\
-             {.arg months} or a higher {.arg cloud_cover_max}."
+      "i" = "Every window was skipped; the warnings say why. For no scenes or \\
+             no clear pixels, try more {.arg months} or a higher \\
+             {.arg cloud_cover_max}."
     ))
   }
   out

R/dft_stac_cube.R

@@ -524,6 +524,13 @@ stac_cube_items <- function(cfg, aoi_wgs84, datetime, cloud_cover_max, months,
     rlang::abort(paste0("No STAC items found for ", cfg$collection),
                  class = "drift_no_items")
   }
+  # Before is_pre, so the offset split is computed on the features that are read.
+  items$features <- stac_features_masked(items$features, cfg$roles$mask)
+  if (length(items$features) == 0) {
+    rlang::abort(paste0(n_items, " STAC items found for ", cfg$collection,
+                        ", but none has the mask asset '", cfg$roles$mask, "'"),
+                 class = c("drift_no_mask_items", "drift_no_items"))
+  }
 
   # Baseline-conditional offset: split items at the boundary so each side is
   # corrected with its own offset. The split is by item date, so it is the same
@@ -546,6 +553,33 @@ stac_cube_items <- function(cfg, aoi_wgs84, datetime, cloud_cover_max, months,
 }
 
 
+#' Drop STAC features that have no mask asset
+#'
+#' gdalcubes uses an image whose mask band is missing from the collection
+#' UNMASKED: it warns `Missing mask band for image ..., mask will be ignored`
+#' from C++ and records the chunk as OK (drift#99), so clouds reach a median with
+#' nothing R can see. Such a feature is dropped here, before the collection is
+#' built, and the drop is announced, so the read has fewer scenes rather than
+#' unmasked ones: the same outcome as a scene the mask removes entirely.
+#'
+#' `mask_asset` `NULL` (a source with no mask role) keeps everything. The asset
+#' is matched by its exact name: `[[` on the assets list, never `$`, which would
+#' partial-match `SCL_20m` for `SCL`.
+#' @noRd
+stac_features_masked <- function(features, mask_asset) {
+  if (is.null(mask_asset) || length(features) == 0) return(features)
+  has_mask <- vapply(features, function(f) !is.null(f$assets[[mask_asset]]), logical(1))
+  if (all(has_mask)) return(features)
+  n <- sum(!has_mask) # nolint: object_usage_linter. used in the cli template
+  ids <- vapply(features[!has_mask], function(f) f$id %||% NA_character_, "") # nolint: object_usage_linter.
+  cli::cli_inform(c(
+    "i" = "Dropped {n} item{?s} with no {.val {mask_asset}} mask asset, which \
+           gdalcubes would read unmasked: {.val {ids}}."
+  ))
+  features[has_mask]
+}
+
+
 #' Re-sign STAC features just before they are read
 #'
 #' Planetary Computer SAS tokens last about 45 minutes (measured 2026-09-28:
@@ -750,9 +784,10 @@ cube_check_nonempty <- function(stk, collection, datetime, cached) {
 #' file's close (Rcpp's own header warns of exactly this). Other gdalcubes
 #' warnings pass through untouched.
 #'
-#' Not every failed merge reaches either signal: a worker chunk file the main
-#' process cannot open is skipped with status OK and no warning, a C++ stderr
-#' line only (round-2 review of #87; drift#99).
+#' Not every failed merge reaches either signal on gdalcubes 0.7.5: a worker
+#' chunk file the main process cannot open is skipped with status OK and no
+#' warning, a C++ stderr line only (round-2 review of #87). The
+#' NewGraphEnvironment/gdalcubes fix for drift#99 records it as ERROR.
 #' @noRd
 cube_write_ncdf <- function(cube, out) {
   unmerged <- character(0)
@@ -790,23 +825,26 @@ cube_write_ncdf <- function(cube, out) {
 #' INCOMPLETE, `128` UNKNOWN (gdalcubes 0.7.5, `src/gdalcubes/src/cube.cpp`). A
 #' worker carries the status to the main process inside its chunk file.
 #'
-#' **What it records is narrower than "the read failed".** Measured offline
-#' (#87): a band image that fails to OPEN marks its chunks INCOMPLETE, which is
-#' the expired or refused signed URL. A band image that opens and then fails
-#' mid-read (a corrupted tile here; a throttled range request on the network)
-#' leaves the status OK, and so does a mask (SCL) image that fails to open, which
-#' silently drops that scene. A mask image that opens and then fails mid-read is
-#' worse: the scene is used unmasked. gdalcubes sets the status only when a band
-#' image will not open; every other I/O step drops its return code, so none of
-#' these is visible to this check or to anything else R can see (drift#99;
-#' inst/notes/gdalcubes-pc-gotchas.md).
+#' **On gdalcubes 0.7.5 it records less than "the read failed".** Measured
+#' offline (#87, #99): a band image that fails to OPEN marks its chunks
+#' INCOMPLETE, which is the expired or refused signed URL. A deleted mask (SCL)
+#' image is caught the same way in drift's reads, because gdalcubes also reads
+#' the mask asset as a band there (no `select_bands()` before `apply_pixel()`).
+#' An image that opens and then fails mid-read (a corrupted tile here; a
+#' throttled range request on the network) leaves the status OK. For a band,
+#' the chunk is filled from the scenes that did read; for the mask, the scene
+#' is used UNMASKED, so cloud reaches the median. Neither is visible to this
+#' check or to anything else R can see on 0.7.5. The NewGraphEnvironment/gdalcubes
+#' fix for drift#99 records both, so this check then catches them unchanged
+#' (inst/notes/gdalcubes-pc-gotchas.md).
 #'
 #' **The netCDF integer fill value means no status was written for that id**,
 #' not that the chunk was empty: a worker skips writing a chunk that is OK and
 #' all-`NA` (no image, or fully masked), and a chunk whose merge threw keeps fill
 #' too. That second case raises an R warning, which cube_write_ncdf() turns into
 #' an abort, so fill is read as OK here. A worker chunk file the main process
-#' cannot open is recorded as OK, with neither signal (drift#99). Anything that is neither OK nor fill fails, so a
+#' cannot open is recorded as OK on 0.7.5, with neither signal, and as ERROR
+#' with the drift#99 fix. Anything that is neither OK nor fill fails, so a
 #' status code a later gdalcubes adds aborts rather than passes.
 #'
 #' For an open failure, this is the only check that sees it. A median over two
@@ -833,10 +871,12 @@ cube_check_chunks <- function(nc_file) {
   if (failed[["failed"]] == 0L) return(invisible(nc_file))
   cli::cli_abort(class = "drift_incomplete_cube", c(
     "gdalcubes could not read {failed[['failed']]} of {failed[['chunks']]} \\
-     chunk{?s}: an image would not open.",
+     chunk{?s}: an image would not open or read.",
     "x" = "Those cells are {.val NA}, or computed from only the scenes that \\
-           did open.",
-    "i" = "Usually an expired or refused signed URL. Nothing was cached.",
+           did read.",
+    "i" = "Usually an expired or refused signed URL; also a refused range \\
+           request mid-read, or a worker chunk file that could not be read \\
+           back (drift#99). Nothing was cached.",
     "i" = "Re-running signs the request again. If the same read fails every \\
            time, an asset may be missing from the catalogue."
   ))

inst/notes/gdalcubes-pc-gotchas.md

@@ -246,11 +246,28 @@ Measured on gdalcubes 0.7.5 (`appelmar/gdalcubes@ed68331`) with a local collecti
 - **The last row is why a pixel check cannot do this.** A median over two scenes where one failed fills every cell from the survivor, so the holed cube has no NA at all. The issue's third option (require data under every item footprint) would have passed it.
 - **Read it with ncdf4, not terra.** GDAL does not list a one-dimensional variable as a subdataset, so `terra::rast(f, subds = "chunk_status")` errors; the `NETCDF:"f":chunk_status` form opens but prints a stray `R_nc4_open` error line. ncdf4 is a gdalcubes import.
 - **The `[WARNING]` line is C++ stdio**, so neither an R handler nor `capture.output()` silences it, even at `parallel = 1`.
-- **Only one failure is recorded: a band image that fails to open.** gdalcubes sets `chunk_status` in six places, all in `image_collection_cube::read_chunk()`, and only band `GDALOpen` returning NULL can fire in practice; every other I/O step between `GDALOpen` and the final `nc_close` drops its return code or warns and continues. A code-check enumeration of the write pipeline (#87, round 3, `planning/archive/2026-10-issue-87-silent-chunk-read-failures/review-round3.md`) found 38 failure paths: 25 detected by drift (13 through `chunk_status` or the merge warning, 12 as an R error before anything is cached), 13 not. The ones that matter:
-  - a band image that opens then fails mid-read: status OK, cells filled from the scenes that read;
-  - **a mask (SCL) image that opens then fails mid-read: the scene is used UNMASKED** — cloud values in the median, not NA (measured: an all-cloud scene gave 1600 of 1600 cells, status OK);
-  - a mask image that fails to open, or a scene with no mask asset: dropped, or used unmasked;
-  - a worker that writes no chunk file, or a partial or unreadable one: status OK or fill, no R warning;
-  - a worker killed by a signal: `write_ncdf()` hangs.
-  These are drift#99. A chunk whose merge throws keeps fill and raises an R warning (`Chunk N could not be added to output`), which drift does catch.
+- **On 0.7.5 only one failure is recorded: a band image that fails to open.** gdalcubes sets `chunk_status` in six places, all in `image_collection_cube::read_chunk()`, and only band `GDALOpen` returning NULL can fire in practice; every other I/O step between `GDALOpen` and the final `nc_close` drops its return code or warns and continues. A code-check enumeration of the write pipeline (#87, round 3, `planning/archive/2026-10-issue-87-silent-chunk-read-failures/review-round3.md`) found 38 failure paths: 25 detected by drift (13 through `chunk_status` or the merge warning, 12 as an R error before anything is cached), 13 not. Which of them the drift#99 fork fix closes is in the next section.
 - `cube_write_ncdf()` is drift's one `write_ncdf()` call, in `stac_cube_assemble()` and `fetch_extent_to()`. It notes the merge warning, lets gdalcubes return (aborting from inside an `Rcpp::warning()` handler would skip its C++ cleanup), then aborts on it or on `chunk_status`, so a failed read aborts before anything is cached. Before it, the offline fixture showed both `dft_stac_composite()` and `dft_stac_fetch()` caching the holed result, tiled and untiled.
+
+## Failed reads after open, mask failures, worker deaths (#99, 2026-10-14)
+
+Fixed on `NewGraphEnvironment/gdalcubes`, branch `99-chunk-status-read-failures` (off `ed68331`), for upstream. drift keeps `Remotes: appelmar/gdalcubes`; its tests for these cases skip with a drift#99 reason until the installed gdalcubes records them (`skip_if_no_read_failure_status()`, `tests/testthat/helper-gdalcubes.R`).
+
+| case | 0.7.5 | fork branch |
+|---|---|---|
+| band image opens, then a tile fails to decode | OK; chunk filled from the scenes that read | INCOMPLETE |
+| mask (SCL) opens, then fails to read | OK; **scene used unmasked** (the cloud value reaches the output) | INCOMPLETE; scene dropped |
+| mask fails to open, `select_bands()` before the write | OK; scene dropped | INCOMPLETE |
+| mask fails to open, **drift's read shape** (`apply_pixel()`, no `select_bands()`) | INCOMPLETE already: SCL is read as a band too, and its band open fails | INCOMPLETE |
+| main process cannot open / read back a worker chunk file | OK, C++ stderr only | ERROR |
+| worker dies before writing its log (signal at startup) | `write_ncdf()` spins forever at 100% CPU | raises in under a second |
+| worker killed mid-computation | raises (`worker process #0 returned 9`) | raises |
+| item with no mask asset in the STAC listing | used unmasked, status OK | unchanged; **drift drops the item** (`stac_features_masked()`) |
+
+- **Every chunk is computed in an Rscript worker, at `parallel = 1` too** (gdalcubes' `.onLoad` and each `gdalcubes_options()` call set the process executor). Workers load gdalcubes from `R_LIBS`, not from the session's `.libPaths()`, so to test a build from a scratch library export `R_LIBS` in the shell; a debugger on the main R process sees nothing of a worker crash.
+- **The cause was one unchecked return.** `gdalwarp_client::warp_simple()` (every image with a geotransform) ignored `ChunkAndWarpImage()` and returned the `INIT_DEST=nan` output as data. Returning nullptr reaches `read_chunk()`'s existing `!gdal_out` branches, for band and mask alike.
+- **The hang was the log printer, not the supervision loop.** That loop breaks on exit 9 as it should. Then `while (!wlogf.eof()) getline(...)` reads each worker's log, and a worker killed at startup never created one: a failed `ifstream` never reaches `eof()`. Sampled while hung: no worker children, main thread hot in `chunk_processor_multiprocess::apply()`. The #87 round-1 hang was this case; a worker killed mid-write, after its log exists, always raised.
+- **A fixture that fails to read but still opens** needs the IFD intact. A GDAL GTiff keeps its IFD at the END, so overwriting the tail tests an open failure; a COG keeps it first, but its tail is the DEFLATE trailer and still decoded. Overwrite the first tile at its `TileOffsets` entry: `tif_corrupt_tile()` in the helper, giving `ZIPDecode ... incorrect header check`.
+- **Still unrecorded with the fix:** a worker that cannot create or move its chunk file (`nc_create` / `CPLMoveFile` unchecked in `exec()`) leaves fill and no warning; an `sqlite3_step` error truncates a chunk's image list.
+- **Separate, pre-existing:** a two-band image with the mask in band 2 segfaults a worker, at full resolution and through overviews alike, on 0.7.5 too (#92 hit it with `one_band_per_file = FALSE`). drift's Sentinel-2 assets are single-band.
+

planning/archive/2026-10-issue-99-read-failures-after-open/README.md

@@ -0,0 +1,55 @@
+## Outcome
+
+The silent gdalcubes failures left over from #87 were fixed at their source, on our fork, for upstream. The cause was small: `gdalwarp_client::warp_simple()` ignored the return of `ChunkAndWarpImage()`, so a read that failed after a successful open came back as an all-NaN "image". A mask that failed to read therefore left its scene unmasked, and cloud reached the median with status OK.
+
+The fork branch (NewGraphEnvironment/gdalcubes#2, off `ed68331`) does five things:
+- makes a failed warp return nullptr, which reaches `read_chunk()`'s existing INCOMPLETE branches;
+- records a failed mask open;
+- sets ERROR when the main process cannot read back a worker's chunk file;
+- opens an overview before closing the source;
+- fixes the "worker killed by a signal hangs `write_ncdf()`" case.
+
+That hang turned out to be the log printer: `while (!eof())` on a log a worker killed at startup never wrote. It was not the supervision loop. The upstream PR is drafted in `upstream-pr-draft.md` and **not posted**.
+
+drift keeps `Remotes: appelmar/gdalcubes`, as decided at the plan gate. Its changes:
+- drops STAC items that have no mask asset (gdalcubes would read them unmasked), with its own error subclass;
+- adds fixtures that corrupt a COG tile with the IFD intact;
+- adds tests gated on a capability probe, which skip with a drift#99 reason on 0.7.5;
+- updates the docs.
+
+One correction to the issue's own table: in drift's read shape (`apply_pixel()` with no `select_bands()`), gdalcubes also reads SCL as a band. So a *deleted* SCL is already recorded on 0.7.5. The case drift really missed is a *corrupted* SCL.
+
+## Measurement
+
+| check | 0.7.5 | fork branch |
+|---|---|---|
+| `test_chunk_status.R` (32 assertions) | 10 fail: band read, mask open, mask read and strict mask open, each at parallel 1 and 2; the cloud value 900 reached the output | 32 pass |
+| `test_worker_failure.R` | both fail at the 120 s timeout | pass in 0.9 s |
+| full gdalcubes tinytest | — | 159 / 159 |
+| corrupted SCL in drift's shape, failed chunks | 0 | 9 |
+| deleted SCL in drift's shape, failed chunks | 9 | 9 |
+| deleted SCL with `select_bands()` | 0 | 9 |
+
+- **drift tests:** gated tests SKIP on 0.7.5. With `DRIFT_TEST_ASSUME_READ_STATUS=true` exactly those tests fail; on the fork build all pass. The full drift suite is green on 0.7.5.
+- **Mutations that turned tests red:**
+  - unwiring the mask-asset drop (5 failures);
+  - a probe forced to answer FALSE;
+  - `select_bands()` inserted in `stac_cube_assemble()` (M9);
+  - the composite handler order swapped.
+- **Wrong turns, kept:**
+  - The first fixture corrupted the end of a GTiff. A GTiff keeps its IFD there, so that tested an open failure, which read as "0.7.5 already records band read failures". Overwriting the tail of a COG still decoded. The tile has to be broken at its `TileOffsets` entry.
+  - A mid-write SIGKILL never hung. Only a kill at startup does.
+  - A multi-band "overview" segfault from review round 1 also happens at full resolution on 0.7.5. It is the #92 two-band defect and out of scope.
+- **Code-check:**
+  - Fork: 3 rounds. Round 1 found a NULL dereference inside the plan-review fix. Round 3 ended the loop with a 39-row ownership and state enumeration.
+  - drift: 3 rounds. Round 3 walked every new assertion and guard with mutations.
+  - Seven reviewer agents in all, plus one plan reviewer.
+
+## Evidence
+
+- Fork branch `NewGraphEnvironment/gdalcubes:99-chunk-status-read-failures`: `inst/tinytest/test_chunk_status.R`, `inst/tinytest/test_worker_failure.R`.
+- Review rounds: `review-plan.md`, `review-fork-round[123].md`, `review-drift-round[123].md` in this directory.
+- Durable notes: `inst/notes/gdalcubes-pc-gotchas.md`, "Failed reads after open, mask failures, worker deaths (#99)".
+- The hang and segfault repros were scratch-only; the hang is now `test_worker_failure.R`.
+
+Closed by: drift PR for branch `99-gdalcubes-read-failures-after-open-mask` (see `gh pr list --search 99`); fork PR NewGraphEnvironment/gdalcubes#2