Skip to content

fix: measure upload and latency without server timing - #4

Merged
mktbsh merged 2 commits into
mainfrom
agent/fix-upload-speed-measurement
Aug 11, 2026
Merged

fix: measure upload and latency without server timing#4
mktbsh merged 2 commits into
mainfrom
agent/fix-upload-speed-measurement

Conversation

@mktbsh

@mktbsh mktbsh commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Measure upload throughput from the client request write timing.
  • Fall back to end-to-end timing when request write timing is unavailable.
  • Fall back to client TTFB for unloaded latency when cfRequestDuration is absent.
  • Add regression coverage for the current Cloudflare response headers.

Root cause

Cloudflare's current speed-test responses no longer include the cfRequestDuration server-timing entry that the CLI treated as required for latency and upload calculations. The CLI therefore rejected all unloaded latency samples and could report an upload value of 0.00 Mbps.

Validation

  • go test ./... (70 tests passed)
  • go vet ./...
  • go mod verify
  • go build -trimpath -ldflags='-s -w' -o dist/unispeedtest ./cmd/unispeedtest
  • Built binary -json run completed with positive upload throughput (2558.96 Mbps) and valid JSON output.

Summary by CodeRabbit

  • Bug Fixes
    • Improved latency and upload timing measurements using client-side request timing when available.
    • Added reliable fallbacks when timing data is unavailable or invalid.
    • Corrected fallback warning behavior for upload measurements.

@coderabbitai

coderabbitai Bot commented Aug 8, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 8d0c4ae4-f421-47be-9edd-4425dae37bfb

📥 Commits

Reviewing files that changed from the base of the PR and between fef5618 and 9ff7dc6.

📒 Files selected for processing (2)
  • internal/cloudflare/measure.go
  • internal/cloudflare/upload_test.go
🚧 Files skipped from review as they are similar to previous changes (2)
  • internal/cloudflare/upload_test.go
  • internal/cloudflare/measure.go

📝 Walkthrough

Walkthrough

The Cloudflare client now records successful request-write time. Upload measurement uses this timing when available. Latency measurement falls back to validated client timing. Tests cover delayed transmission, delayed first-byte receipt, and fallback warnings.

Changes

Cloudflare timing measurement

Layer / File(s) Summary
Request-write timing capture
internal/cloudflare/types.go, internal/cloudflare/client.go
PerfData stores RequestWritten. The HTTP trace records it only after a successful request write.
Client-side measurement fallbacks
internal/cloudflare/measure.go
Latency measurement validates client timing. Upload measurement prefers request-write duration and falls back to end-to-end duration with a warning.
Timing behavior tests
internal/cloudflare/upload_test.go, internal/cloudflare/client_test.go
Tests cover delayed request transmission, delayed first-byte receipt, positive client timing, and the updated fallback warning count.

Estimated code review effort: 3 (Moderate) | ~20 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: measuring upload and latency without relying on server timing.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch agent/fix-upload-speed-measurement

Comment @coderabbitai help to get the list of available commands.

@mktbsh
mktbsh marked this pull request as ready for review August 8, 2026 13:11

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/cloudflare/measure.go`:
- Around line 65-70: Validate serverTiming immediately after ParseServerTiming
in the measurement flow: if it is NaN, infinite, or negative, return
clientDuration with no error. Preserve the existing parse-error fallback and
only calculate dur after the parsed timing passes these checks.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: cc9dc6be-a42a-48ff-9c52-e1c845647b0a

📥 Commits

Reviewing files that changed from the base of the PR and between 7b016f9 and fef5618.

📒 Files selected for processing (5)
  • internal/cloudflare/client.go
  • internal/cloudflare/client_test.go
  • internal/cloudflare/measure.go
  • internal/cloudflare/types.go
  • internal/cloudflare/upload_test.go

Comment thread internal/cloudflare/measure.go Outdated
@mktbsh
mktbsh merged commit 4aa6f0a into main Aug 11, 2026
3 checks passed
@mktbsh
mktbsh deleted the agent/fix-upload-speed-measurement branch August 11, 2026 15:01
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