FXC-4775: Surface batch download failures when gzip extraction fails

[yaugenst-flex]

Summary

• Fix Batch.download() to raise when any background download fails.
• Make gzip extraction atomic and report clearer errors.
• Add regression test.

Testing

• poetry run pytest -q tests/test_web/test_webapi.py -k "batch_download_surfaces_download_errors"
• poetry run pytest -q tests/test_web/test_s3utils.py
• poetry run pre-commit run --all-files

---

Note

^{Cursor Bugbot is generating a summary for commit 8302236. Configure here.}

Greptile Overview

Greptile Overview

Greptile Summary

This PR fixes a critical bug where Batch.download() would silently succeed even when background downloads failed due to gzip extraction errors.

Key Changes

container.py: Modified the batch download mechanism to call fut.result() on each completed future. This ensures exceptions from background worker threads are propagated to the main thread rather than being silently swallowed.

s3utils.py: Made gzip extraction atomic by:

• Creating a temporary output file in the target directory using tempfile.mkstemp()
• Extracting the gzip to the temp file
• Using atomic replace() to move it to the final destination
• Properly cleaning up temp files on error
• Providing clearer error messages when extraction fails

test_webapi.py: Added regression test test_batch_download_surfaces_download_errors that verifies exceptions from background downloads are properly surfaced to the caller.

CHANGELOG.md: Added user-facing entry describing the bug fix.

Impact

This fix ensures that users are immediately notified when batch downloads fail, preventing silent data corruption or missing files that could lead to confusing downstream errors. The atomic extraction also prevents partially-written files from appearing as successful downloads.

Confidence Score: 4/5

• This PR is safe to merge with minor style improvements recommended
• The core logic correctly fixes the critical bug by propagating exceptions from background downloads. The atomic extraction pattern is sound and follows established best practices. Two style-level suggestions were provided: (1) improving the error message clarity when replace() fails, and (2) handling an extremely unlikely edge case with temp file cleanup. These are non-critical improvements that don't affect the correctness of the fix.
• tidy3d/web/core/s3utils.py - review error message wording and temp file cleanup edge case

Important Files Changed

File Analysis

Filename	Score	Overview
tidy3d/web/core/s3utils.py	4/5	Makes gzip extraction atomic using temp files and improves error messages; minor edge case with cleanup order
tidy3d/web/api/container.py	5/5	Correctly calls fut.result() to propagate exceptions from background downloads
tests/test_web/test_webapi.py	5/5	Adds regression test for batch download error propagation
CHANGELOG.md	5/5	Clear and accurate changelog entry following project guidelines

Sequence Diagram

sequenceDiagram
    participant User
    participant Batch
    participant ThreadPool
    participant Job
    participant download_gz_file
    participant extract_gzip
    participant FileSystem

    User->>Batch: download(path_dir)
    Batch->>Batch: Create download tasks for each job
    Batch->>ThreadPool: Submit futures for parallel downloads
    
    loop For each completed future
        ThreadPool-->>Batch: future completes
        Batch->>Batch: fut.result() [NEW: propagates exceptions]
        
        alt Download succeeds
            Batch->>Batch: Update progress
        else Download fails (e.g., gzip error)
            Job->>download_gz_file: download and extract
            download_gz_file->>FileSystem: Download .gz to temp file
            download_gz_file->>FileSystem: Create temp output file [NEW: atomic]
            download_gz_file->>extract_gzip: Extract to temp output
            
            alt Extraction fails
                extract_gzip-->>download_gz_file: Exception
                download_gz_file->>FileSystem: Clean up temp output [NEW]
                download_gz_file-->>Job: Raise WebError [NEW]
                Job-->>Batch: RuntimeError
                Batch-->>User: Exception raised [FIXED]
            end
        end
    end
    
    Batch-->>User: Download complete or error raised

Loading

Important Files Changed

File Analysis

Filename	Score	Overview
tidy3d/web/core/s3utils.py	4/5	Makes gzip extraction atomic using temp files and improves error messages; minor edge case with cleanup order
tidy3d/web/api/container.py	5/5	Correctly calls fut.result() to propagate exceptions from background downloads
tests/test_web/test_webapi.py	5/5	Adds regression test for batch download error propagation
CHANGELOG.md	5/5	Clear and accurate changelog entry following project guidelines

Sequence Diagram

sequenceDiagram
    participant User
    participant Batch
    participant ThreadPool
    participant Job
    participant download_gz_file
    participant extract_gzip
    participant FileSystem

    User->>Batch: download(path_dir)
    Batch->>Batch: Create download tasks for each job
    Batch->>ThreadPool: Submit futures for parallel downloads
    
    loop For each completed future
        ThreadPool-->>Batch: future completes
        Batch->>Batch: fut.result() [NEW: propagates exceptions]
        
        alt Download succeeds
            Batch->>Batch: Update progress
        else Download fails (e.g., gzip error)
            Job->>download_gz_file: download and extract
            download_gz_file->>FileSystem: Download .gz to temp file
            download_gz_file->>FileSystem: Create temp output file [NEW: atomic]
            download_gz_file->>extract_gzip: Extract to temp output
            
            alt Extraction fails
                extract_gzip-->>download_gz_file: Exception
                download_gz_file->>FileSystem: Clean up temp output [NEW]
                download_gz_file-->>Job: Raise WebError [NEW]
                Job-->>Batch: RuntimeError
                Batch-->>User: Exception raised [FIXED]
            end
        end
    end
    
    Batch-->>User: Download complete or error raised

Loading

[greptile-apps]

_{2 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

[github-actions]

Diff Coverage

Diff: origin/develop...HEAD, staged and unstaged changes

• tidy3d/web/api/container.py (50.0%): Missing lines 1293-1294
• tidy3d/web/core/s3utils.py (0.0%): Missing lines 450,453,456-463,467

Summary

• Total: 15 lines
• Missing: 13 lines
• Coverage: 13%

tidy3d/web/api/container.py

Lines 1289-1298

  1289                 with Progress(*progress_columns, console=console, transient=False) as progress:
  1290                     pbar_message = f"Downloading data for {len(fns)} tasks"
  1291                     pbar = progress.add_task(pbar_message, total=len(fns))
  1292                     completed = 0
! 1293                     for fut in concurrent.futures.as_completed(futures):
! 1294                         fut.result()
  1295                         completed += 1
  1296                         progress.update(pbar, completed=completed)
  1297             else:
  1298                 # Still ensure completion if verbose is off

---

tidy3d/web/core/s3utils.py

Lines 446-469

  446             to_file=Path(tmp_file_path_str),
  447             verbose=verbose,
  448             progress_callback=progress_callback,
  449         )
! 450         if not Path(tmp_file_path_str).exists():
  451             raise WebError(f"Failed to download and extract '{remote_filename}'.")
  452 
! 453         tmp_out_fd, tmp_out_path_str = tempfile.mkstemp(
  454             suffix=IN_TRANSIT_SUFFIX, dir=to_path.parent
  455         )
! 456         os.close(tmp_out_fd)
! 457         tmp_out_path = Path(tmp_out_path_str)
! 458         try:
! 459             extract_gzip_file(Path(tmp_file_path_str), tmp_out_path)
! 460             tmp_out_path.replace(to_path)
! 461         except Exception as e:
! 462             tmp_out_path.unlink(missing_ok=True)
! 463             raise WebError(
  464                 f"Failed to extract '{remote_filename}' from '{tmp_file_path_str}' to '{to_path}'."
  465             ) from e
  466     finally:
! 467         Path(tmp_file_path_str).unlink(missing_ok=True)
  468     return to_path

---

[marcorudolphflex]

LGTM!