Skip to content

fix(relay/media): fall back to full download when S3 range GET fails (#3786) - #4079

Open
iroiro147 wants to merge 3 commits into
block:mainfrom
iroiro147:claude/issue3786-20260801
Open

fix(relay/media): fall back to full download when S3 range GET fails (#3786)#4079
iroiro147 wants to merge 3 commits into
block:mainfrom
iroiro147:claude/issue3786-20260801

Conversation

@iroiro147

Copy link
Copy Markdown

Problem — #3786

The Range handler on GET /v1/media returns an opaque 500 {"error":"internal error"} whenever the S3 range GET fails, even though the object exists and plain (non-range) GETs return 200 fine. This was reported for a ~9.9 MiB file stored on Cloudflare R2:

Request Observed Expected
Range: bytes=0-1023 500 206 with first 1024 bytes
Range: bytes=0-99 416 206 (or 416 only when out of bounds)
No Range header 200 200

The reporter verified the same object+range works directly via aws s3api get-object --range bytes=0-99, so R2 itself handles ranges correctly — the failure path is inside buzz's handling of the range GET.

Root-cause hypothesis

buzz-media::storage::get_range() calls bucket.get_object_range(...). rust-s3 is built with the fail-on-err feature, so any non-2xx from S3/R2 surfaces as S3Error::HttpFailWithBody(status, body). The existing code mapped only S3Error::Http(404, _) to NotFound; everything else fell through to a generic MediaError::StorageError, which the error handler turns into 500.

The relay's range branch (serve_blob_for_tenant) then propagated that 500 with no logging showing the upstream status/body — which is also why the reporters couldn't provide actionable diagnostics when asked.

What this PR does

  1. buzz-media::storage::get_range: map HttpFailWithBody explicitly — log tracing::warn!(s3_status, s3_body, key, start, end) and surface a descriptive StorageError (calling out 416 specifically) instead of the opaque one.

  2. buzz-relay::api::media range branch: on a failed get_range (except NotFound), log a warning and fall back to a full get(&key), then slice [start..(end+1).min(len)] in memory. Returns 416 if start >= len. The existing response headers (including Content-Range) are untouched since it's all keyed on the final slice.

    The fallback keeps the existing MAX_RANGE_CHUNK-bounded behaviour intact: a legitimately huge file where the primary range GET succeeds is untouched. It's only used when S3 rejected the range GET in the first place, at which point we'd otherwise be returning 500.

  3. Unit tests for the new slice math and the clamping logic (including a regression-sized case matching the reporter's 9,896,517-byte object, plus a 20 MiB case where MAX_RANGE_CHUNK binds the end).

Diagnostics

Going forward, if R2 (or any S3-compatible backend) rejects a range GET, the logs now contain the upstream HTTP status and body for the exact key/start/end — precisely the information requested from the reporter in #3786 that they couldn't provide. The fallback means they'd also no longer see a 500 in this scenario.

Testing

cargo check  -p buzz-media
cargo check  -p buzz-relay
cargo clippy -p buzz-media -p buzz-relay -- -D warnings
cargo test   -p buzz-media                # 107 pass
cargo test   -p buzz-relay api::media::tests  # 12/12 range + fallback tests pass
cargo test   -p buzz-relay                # 824 pass / 9 fail (pre-existing: 2 admin asserts + 7 media Sqlx(PoolTimedOut) needing live Postgres — unchanged from main baseline)

Adds a root-level `buzz doctor` subcommand that runs independent
local + remote readiness checks and aggregates them into a single
structured JSON report. Use it to answer 'is my Buzz environment
ready to use?' without publishing events or mutating state.

Checks performed:

- relay URL presence, syntax, canonicalization
- BUZZ_PRIVATE_KEY parseability (never printed; only the derived
  public identity is reported)
- BUZZ_AUTH_TAG syntax + verification against the signing identity
  (never printed)
- CLI version
- relay unauthenticated reachability probe
- NIP-11 parseation against the relay info document
- authenticated read probe (single bounded query)
- community membership probe (kind:39002 #p=self)

`--offline` runs only local checks and marks remote probes as
skipped. Missing or malformed config surfaces as a check result
with status error, not as an argument-time failure, so doctor
remains useful when the environment being debugged is the broken
one.

Exit behavior distinguishes failed required checks from warnings:
  0  all applicable checks ok (warnings allowed)
  3  any auth/identity check failed
  2  any relay/network check failed
All applicable checks run so one invocation can reveal multiple
problems.

Tests cover the aggregator's exit-precedence (auth wins over
network, skipped never affects outcome, warnings stay zero) and
the JSON output shape.

Signed-off-by: iroiro147 <sarthak.singh@juspay.in>
… not auth (exit 3) (block#3926)

The first cut routed every authed-read failure into the same
auth-shaped bucket. On an unreachable relay the relay_reachable
probe was reported error, then the authed read ALSO reported
auth_read error, which the aggregator mapped to exit 3 — even
though a DNS/connect failure has nothing to do with credentials.

Split the failure handling:

- Distinguish true auth rejections (Auth variant, or relay
  401/403) from transport/other relay failures. Only the former
  produces an auth_read error -> exit 3.
- Track whether the unauthenticated reachability probe already
  failed; if so, skip re-emitting the relay-side error and mark
  auth_read/membership as skipped instead.
- If reachability passed but the authed read failed for a
  transport reason, emit a relay_reachable error explaining the
  authed leg failed.

Smoke-verified: with BUZZ_RELAY_URL pointing at an unresolvable
host, exit is now 2 with relay_reachable + nip11 reporting the
transport problem and auth_read/membership cleanly skipped.

Signed-off-by: iroiro147 <sarthak.singh@juspay.in>
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