Skip to content

Use web_time::Instant for Cloudflare timing - #1226

Open
dhruv8sh wants to merge 4 commits into
mainfrom
fix/cloudflare-web-time-instant
Open

dhruv8sh wants to merge 4 commits into
mainfrom
fix/cloudflare-web-time-instant

Conversation

@dhruv8sh

@dhruv8sh dhruv8sh commented Oct 1, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • std::time::Instant::now() and std::time::SystemTime::now() panic on the Cloudflare adapter's wasm32-unknown-unknown target (workerd: "time not implemented on this platform"), so any auction on Cloudflare crashed the worker when it started auction telemetry. This PR moves the request-path clock calls in trusted-server-core (auction telemetry, the DataDome IP CIDR source cache, and the publisher template cache) to web_time. On every other target web_time re-exports the std types, so Fastly, Spin, and Axum behave the same as before.
  • Adds scripts/lint-wasm-clock.sh, run in CI, so the std clock calls (now and elapsed) cannot come back into any crate built into the Cloudflare Worker.
  • Adds a workerd integration test that starts wrangler dev with an auction-enabled config and posts to /auction. The request passes the consent gate and runs to completion through the orchestrator, which creates an AuctionObservationContext (and its Instant) inside workerd. With std::time::Instant restored, the worker panics and the test fails. With this fix, it passes.
  • The test builds its auction-enabled config per test from the shared fixture and removes the providers, so it makes no outbound calls. The shared fixture is unchanged, so the other environments still run with auctions disabled.

Changes

File Change
crates/trusted-server-core/src/auction/telemetry.rs Use web_time::Instant instead of std::time::Instant
crates/trusted-server-core/src/integrations/datadome/protection_scope.rs Use web_time::Instant for the IP CIDR source cache timing
crates/trusted-server-core/src/publisher.rs Use web_time::Instant for template cache expiry. origin_shared_ttl gets its SystemTime from std_system_time_now(); origin_shared_ttl_at still takes a std SystemTime because httpdate parses into that type
crates/trusted-server-core/src/ec/mod.rs New std_system_time_now(): a std SystemTime derived from web_time::SystemTime::now(), shared by the publisher and S3 signing callers, with a unit test
crates/trusted-server-core/src/proxy.rs The s3_sigv4::sign_headers call uses std_system_time_now()
crates/trusted-server-core/Cargo.toml Declare chrono's wasmbind feature for wasm32-unknown-unknown explicitly
scripts/lint-wasm-clock.sh, scripts/clippy-wasm-clock/clippy.toml Clippy pass on wasm32-unknown-unknown over the Cloudflare adapter's build graph (-p trusted-server-adapter-cloudflare --features cloudflare --lib) with a dedicated config that denies only std::time::{Instant,SystemTime}::{now,elapsed}
.github/workflows/format.yml Run the lint after cargo clippy-cloudflare-wasm
AGENTS.md, .claude/commands/check-ci.md List the lint under CI gates and run it from /check-ci
crates/trusted-server-integration-tests/tests/integration.rs Add test_cloudflare_enabled_auction_reaches_telemetry
crates/trusted-server-integration-tests/tests/common/config.rs Helper that builds a per-test, auction-enabled config with the providers removed
crates/trusted-server-integration-tests/tests/environments/cloudflare.rs Add spawn_with_config_json to start wrangler with a custom config
crates/trusted-server-adapter-cloudflare/.gitignore Ignore the generated wrangler.integration.generated.toml

Notes:

  • Why a separate lint config. A disallowed-methods rule in the workspace clippy.toml can't work: on native and wasm32-wasip1, web_time::Instant and web_time::SystemTime are the std types, so the rule would also flag every correct web_time call. Only wasm32-unknown-unknown has distinct types, so the lint runs on that target alone. It covers every crate built into the Cloudflare Worker, including the adapter. The elapsed methods are denied too, since std builds them on the panicking now().
  • No runtime test covers the DataDome IP CIDR path or the template cache. Only the Fastly adapter runs request filters and provides a template cache, so Cloudflare doesn't reach either today. The switch to web_time keeps that code safe on Cloudflare, and the lint guards it.
  • Left alone: chrono::Utc::now() calls. chrono's wasmbind feature, now declared explicitly for wasm32-unknown-unknown, reads the JS clock there.
  • On Workers, performance.now() only advances after I/O, so Cloudflare elapsed_ms values aren't comparable with the other adapters' wall-clock values (noted on AuctionObservationContext::elapsed_ms).

Closes

Closes #1075
Closes #1240

Test plan

  • cargo test-fastly && cargo test-axum
  • cargo clippy-fastly && cargo clippy-axum
  • cargo fmt --all -- --check
  • JS tests: cd crates/trusted-server-js/lib && npx vitest run (Node 24.12.0)
  • JS format: cd crates/trusted-server-js/lib && npm run format
  • Docs format: cd docs && npm run format
  • WASM build: cargo build --package trusted-server-adapter-fastly --release --target wasm32-wasip1
  • Manual testing via fastly compute serve
  • Other:
    • cargo clippy-cloudflare && cargo clippy-cloudflare-wasm && cargo clippy-spin-native && cargo clippy-spin-wasm && cargo clippy-cli && cargo clippy-codegen
    • cargo test-cloudflare && cargo test-spin, plus cargo test-fastly-reuse and the build-digest test and lint after merging main
    • scripts/lint-wasm-clock.sh passes; with std::time::Instant restored in auction/telemetry.rs it fails with use of a disallowed method std::time::Instant::now
    • Root markdown prettier check from AGENTS.md (covers the AGENTS.md edit)
    • cargo clippy --manifest-path crates/trusted-server-integration-tests/Cargo.toml --all-targets -- -D warnings
    • cargo test --manifest-path crates/trusted-server-integration-tests/Cargo.toml --test parity
    • Built the Cloudflare bundle with crates/trusted-server-adapter-cloudflare/build.sh and ran test_cloudflare_enabled_auction_reaches_telemetry against workerd (wrangler dev): passes. With std::time::Instant restored, the worker panics with "time not implemented on this platform" and the test fails.
    • I did not run the Docker-based WordPress and Next.js integration tests locally because no Docker daemon was available. They don't touch the changed code, and CI runs them.

Checklist

  • Changes follow AGENTS.md conventions
  • No unwrap() in production code — use expect("should ...")
  • Uses log macros (not println!)
  • New code has tests
  • No secrets or credentials committed

@dhruv8sh dhruv8sh self-assigned this Oct 1, 2026
@dhruv8sh
dhruv8sh requested review from ChristianPavilonis, aram356 and prk-Jr and removed request for ChristianPavilonis, aram356 and prk-Jr October 1, 2026 15:24

@ChristianPavilonis ChristianPavilonis left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review summary

Reviewed d7b5e1447f2d4122f3d2cb98394e4e6ba9ffbf16 against cb945254521ed6dab365072f7caec10c524e323b. All six changed files were reviewed, including auction callers, telemetry consumers, DataDome cache timing, configuration serialization, process cleanup, and CI execution.

No actionable issues introduced by this PR found.

Safety proof

  • Cloudflare timer creation: Traced /auction through consent handling into AuctionObservationContext. Confirmed that test_cloudflare_enabled_auction_reaches_telemetry executed successfully in workerd in CI job 110442815930. The CI checkout had the same Git tree as the reviewed head. Runtime evidence, proven.
  • Native/WASI compatibility: Compiled direct assignments from the locked web_time::Instant type to std::time::Instant for native and wasm32-wasip1; both passed. Telemetry and DataDome scope tests passed natively and through Viceroy. Executed evidence, proven.

Validation and review context

Checks ran in an isolated snapshot of the exact head; the original worktree remained clean.

  • cargo fmt --all -- --check and diff whitespace check passed.
  • cargo test-cloudflare --locked --offline: 53 tests passed.
  • Focused core telemetry tests: 14 passed natively and 14 through cargo test-fastly.
  • Focused DataDome protection-scope tests: 11 passed natively and 11 through cargo test-fastly.
  • Integration test binary: 29 passed, 10 runtime tests ignored locally.
  • Cross-adapter parity: 17 passed.
  • cargo check-cloudflare --locked --offline passed.
  • CARGO_NET_OFFLINE=true TSJS_SKIP_BUILD=1 cargo clippy-cloudflare-wasm passed.
  • All reported PR checks passed. No existing reviews, inline comments, review threads, or issue comments.

Residual risk: No local workerd rerun because Wrangler and the bundle were absent; the CI runtime result was inspected directly. Cloudflare DataDome cache execution remains untested and currently unreachable through that adapter's request handling.

@prk-Jr prk-Jr left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 LGTM

std::time::Instant::now() panics on the Cloudflare adapter's
wasm32-unknown-unknown target. Switch auction telemetry and the DataDome
IP CIDR source cache to web_time::Instant, which re-exports the standard
type on other targets.

Add a Cloudflare integration test that starts wrangler dev with an
auction-enabled config and posts to /auction. The request passes the
consent gate and completes through the orchestrator, so it creates an
AuctionObservationContext and its web_time::Instant in workerd. The
config drops the fixture's providers so the auction makes no outbound
calls. With std::time::Instant restored, the worker panics with "time
not implemented on this platform" and the test fails.

The config is built per test from the shared fixture, so the Fastly,
Axum, and other Cloudflare tests keep auctions disabled. The DataDome
IP CIDR source is not covered: only the Fastly adapter runs request
filters, so Cloudflare never evaluates the DataDome protection scope.

Closes #1075

Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
The template cache and origin_shared_ttl in publisher.rs still called std::time::Instant::now and SystemTime::now, which panic on wasm32-unknown-unknown. Move them to web_time. origin_shared_ttl_at keeps taking a std SystemTime because httpdate parses into that type, so build it from UNIX_EPOCH plus the web_time elapsed duration.

Add scripts/lint-wasm-clock.sh, a clippy pass on wasm32-unknown-unknown with a dedicated config that denies std::time::Instant::now and std::time::SystemTime::now in trusted-server-core. It runs on that target only because elsewhere web_time re-exports the std types, so a workspace-wide rule would flag the correct web_time calls. Run it in CI and list it under the AGENTS.md CI gates.

Signed-off-by: dhruv8sh <dhruv8sh@proton.me>

@aram356 aram356 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

Moves the remaining request-path std clocks in trusted-server-core (auction telemetry, the DataDome IP CIDR cache, the template cache and origin_shared_ttl) to web_time, adds a wasm32-unknown-unknown clippy gate, and adds a workerd regression test. I checked the main claims locally. With std::time::Instant restored in telemetry.rs, the new integration test fails under wrangler dev, and scripts/lint-wasm-clock.sh fails even when it runs right after cargo clippy-cloudflare-wasm; both pass at this head. The UNIX_EPOCH + elapsed bridge is pure arithmetic on std's unsupported-time backend, and chrono's wasmbind feature is enabled in the Cloudflare build, so Utc::now() is safe as stated. The one blocker is the conflict with main. The two lint suggestions close gaps I could reproduce.

2 of the inline comments below carry a one-click GitHub suggestion. Use Commit suggestion (or Add suggestion to batch for both at once) to apply them as commits on the PR branch. The other two inline comments are a prose refactor that touches files outside this diff and an informational note.

Blocking

🔧 wrench

  • Conflicts with main, needs a rebase — see Cross-cutting below

Non-blocking

♻️ refactor / 📝 note

  • The lint misses std::time::SystemTime::elapsed() — see inline at scripts/clippy-wasm-clock/clippy.toml:4
  • Lint the Cloudflare adapter's build graph, not core alone — see inline at scripts/lint-wasm-clock.sh:2
  • Second copy of the std SystemTime bridge — see inline at crates/trusted-server-core/src/publisher.rs:6470
  • Workers timers only advance after I/O — see inline at crates/trusted-server-core/src/auction/telemetry.rs:12

🌱 seedling / 🏕 camp site

  • Declare chrono's wasmbind feature explicitly — see Cross-cutting below
  • /check-ci does not run the new gate — see Cross-cutting below

Cross-cutting / body-level findings

  • 🔧 Conflicts with main, needs a rebase — GitHub reports this branch as CONFLICTING. Since the merge base (7f610c0fc), main changed two of the files this PR touches:

    • crates/trusted-server-core/src/publisher.rs: #1210 added use sha2::Digest as _; where this PR adds use web_time::Instant;. Keep both, sha2 first.
    • AGENTS.md CI Gates: #1179 added cargo test-fastly-reuse to item 3, and #1210 added item 9. Keep main's item 3, and consider giving the lint its own numbered item (like item 9) instead of an indented line under item 2.

    I resolved it that way in a scratch merge. The result passes scripts/lint-wasm-clock.sh, cargo clippy-cloudflare-wasm and cargo fmt --all -- --check, so main's new commits add no std clock calls to core. The existing approvals are on d7b5e1447 (the first commit only); the publisher.rs change, lint script, CI step and AGENTS.md edit in 22268364 came after them.

  • 🌱 Declare chrono's wasmbind feature explicitly — The PR leaves chrono::Utc::now() (auction/telemetry.rs:875, integrations/datadome/protection.rs:384, request_signing/rotation.rs:325) alone because chrono's default wasmbind feature reads the JS clock. That holds today (cargo tree -p trusted-server-adapter-cloudflare --target wasm32-unknown-unknown --features cloudflare -e features -i chrono shows wasmbind and js-sys), but only because the workspace's chrono = "0.4.44" keeps default features. A later default-features = false would make those calls panic on Cloudflare, and no clippy rule here can see inside chrono. Core already declares the equivalent requirement for getrandom and uuid under [target.'cfg(all(target_arch = "wasm32", target_os = "unknown"))'.dependencies]; adding chrono = { workspace = true, features = ["wasmbind"] } there would make it explicit. Fine as a follow-up.

  • 🏕 /check-ci does not run the new gate — .claude/commands/check-ci.md ("Run all CI checks locally, in order") doesn't call scripts/lint-wasm-clock.sh. That list was already behind CI (no clippy-cloudflare-wasm, Spin or CLI lints); adding the script after its Cloudflare clippy step would keep a local run in line with the cargo fmt job.

CI Status

All checks ran against the pre-conflict merge base and need to re-run after the rebase.

  • Analyze (actions): PASS
  • Analyze (javascript-typescript): PASS (both runs)
  • Analyze (python): PASS
  • Analyze (rust): PASS
  • browser integration tests: PASS
  • cargo check (cloudflare native + wasm32-unknown-unknown): PASS
  • cargo check/build/test (spin native + wasm32-wasip1): PASS
  • cargo fmt: PASS (required); this job runs the new scripts/lint-wasm-clock.sh step
  • cargo test: PASS (required)
  • cargo test (axum native): PASS
  • cargo test (cross-adapter parity): PASS
  • cargo test (ts CLI, native) (macos-latest): PASS
  • cargo test (ts CLI, native) (ubuntu-latest): PASS
  • CLAUDE.md symlink guard: PASS
  • CodeQL: PASS
  • format-docs: PASS (required)
  • format-typescript: PASS (required)
  • integration tests: PASS; ran test_cloudflare_enabled_auction_reaches_telemetry
  • integration tests (Fastly EC lifecycle): PASS
  • prepare integration artifacts: PASS
  • vitest: PASS

Comment thread scripts/clippy-wasm-clock/clippy.toml
Comment thread scripts/lint-wasm-clock.sh Outdated
Comment thread crates/trusted-server-core/src/publisher.rs Outdated
Comment thread crates/trusted-server-core/src/auction/telemetry.rs
Lint the Cloudflare adapter's build graph instead of core alone and also deny the std Instant and SystemTime elapsed methods, which call the panicking now(). Extract std_system_time_now for the publisher and S3 signing callers, declare chrono's wasmbind feature for wasm32-unknown-unknown, note the Workers timer behaviour on elapsed_ms, and run the lint from /check-ci.

Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
…me-instant

Signed-off-by: dhruv8sh <dhruv8sh@proton.me>

# Conflicts:
#	AGENTS.md
#	crates/trusted-server-core/src/publisher.rs
@dhruv8sh

Copy link
Copy Markdown
Collaborator Author

Merged main in 025df95. In publisher.rs I kept both imports (sha2 first). In AGENTS.md I kept main's item 3 (with test-fastly-reuse) and item 9, and moved the wasm-clock lint into its own item 10. The full gate set passes after the merge, including test-fastly-reuse, the build-digest test and lint, parity, and scripts/lint-wasm-clock.sh.

In 466520b I also declared chrono = { workspace = true, features = ["wasmbind"] } under core's wasm32-unknown-unknown dependencies so the Utc::now() assumption is explicit, and added the lint to /check-ci.

@dhruv8sh
dhruv8sh requested a review from aram356 October 10, 2026 06:31

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

4 participants