Skip to content

Fix cache and server error propagation#2779

Draft
seungpyoson wants to merge 6 commits into
mozilla:mainfrom
seungpyoson:agent/fail-closed-cache-errors
Draft

Fix cache and server error propagation#2779
seungpyoson wants to merge 6 commits into
mozilla:mainfrom
seungpyoson:agent/fail-closed-cache-errors

Conversation

@seungpyoson

@seungpyoson seungpyoson commented Jul 23, 2026

Copy link
Copy Markdown

Summary

  • fail closed on cache backend, transport, timeout, filesystem, and cache-entry decoding failures; only confirmed absence remains a cache miss
  • apply the same failure contract to direct, client-side, preprocessor, disk, and multi-level cache paths
  • validate storage capabilities once at daemon startup and return versioned, cached metadata to client-side compilers
  • make multi-level backfill operate only after full consumer validation and copy the exact validated bytes
  • preserve explicit read-only operation, including non-mutating disk reads and credentialless public S3/OSS caches

Root cause

The OpenDAL integration introduced by #1497 correctly separated NotFound from noisy logging, but later storage and IPC boundaries erased other failure information too. Backend errors, corrupt members, disk I/O failures, timeouts, and client disconnects could therefore become ordinary cache misses followed by successful uncached compilation.

The same broad fallback existed independently in multi-level lookup, preprocessor validation, disk indexing, cache archive extraction, and client-side protocol handling. Fixing only the initial OpenDAL match would leave those alternate fail-open paths intact.

Resulting behavior

  • A confirmed missing key is a normal miss. Other lookup failures stop before compiler dispatch and cannot publish a replacement entry.
  • Remote namespace readability is validated once at startup where the backend permits it; that proof is reused rather than issuing LIST after every cold miss.
  • Existing credentialless public S3/OSS mode remains GET-only and does not require bucket-list permission; it must be configured explicitly as READ_ONLY.
  • Read-write disk startup performs a reversible write/delete probe. Read-only disk startup requires an existing readable root and does not create, delete, evict, or update timestamps.
  • Corrupt stdout, stderr, required outputs, and present optional outputs are errors. A genuinely absent optional output and valid empty diagnostics remain accepted.
  • Preprocessor cache corruption and unexpected dependency I/O errors are fatal; confirmed deletion or content change remains normal invalidation. Optional metadata refresh remains best-effort.
  • Multi-level reads propagate level failures. A later-level hit returns without a second raw read, and backfill begins only after full archive validation.
  • Client-side storage RPCs use an explicit protocol version and preserve daemon errors. An absolute IPC deadline shuts down stalled or trickling connections, with atomic timeout/completion arbitration.
  • A post-CompileStarted disconnect fails by default. SCCACHE_IGNORE_SERVER_IO_ERROR=1 remains the explicit opt-in to local fallback, including client-side lookup transport failure, without recompiling after write-only failure.
  • Direct and client-side timeout failures retain cache_timeouts telemetry on Unix and Windows.

Compatibility

The client-side storage protocol is versioned. A client and daemon with incompatible storage RPC layouts fail during the handshake rather than interpreting an error as a miss.

The capability-check behavior intentionally reverses the rate-limit tolerance introduced by #1557. A rate-limited read or unavailable requested write capability can prevent server startup. Users who require read-only operation must configure it explicitly.

Validation

  • cargo fmt --check
  • cargo clippy --locked --all-targets -- -D warnings -A unknown-lints -A clippy::type_complexity -A clippy::new-without-default
  • cargo check --locked --no-default-features
  • cargo check --locked --no-default-features --features s3
  • cargo test --locked --lib

@seungpyoson

Copy link
Copy Markdown
Author

Exact-head review and verification record for e14c22cd35469239d405752c92a22be60769e4df:

  • internal adversarial review: APPROVE, no remaining findings
  • cargo fmt --check: passed
  • cargo clippy --locked --all-targets -- -D warnings -A unknown-lints -A clippy::type_complexity -A clippy::new-without-default: passed
  • cargo check --locked --no-default-features: passed (existing warnings unchanged)
  • cargo check --locked --no-default-features --features s3: passed (existing warnings unchanged)
  • cargo test --locked --lib: 518 passed, 0 failed

The adversarial pass specifically rechecked fatal compiler disposition, archive-member validation, exact-byte post-validation backfill, disk read-only behavior, remote namespace/public-cache handling, version skew, absolute IPC deadlines and their completion race, fallback phase safety, and cross-platform timeout telemetry.

@seungpyoson

Copy link
Copy Markdown
Author

Closing and reopening to refresh stale head metadata after the fork branch advanced.

@seungpyoson seungpyoson reopened this Jul 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant