fix(antd): resolve shrunk DataMap before sizing streaming downloads - #243
Conversation
Files over 3 x MAX_CHUNK_SIZE (~12.5 MB) upload as a shrunk (child) DataMap, and original_file_size() on such a map describes the serialized parent map (a few hundred bytes), not the plaintext. All three streaming paths sized their response from it, so the REST raw stream truncated every larger download at a bogus Content-Length, and the NDJSON meta.total_size / gRPC x-content-length denominators were wrong. Resolve the map to its root form up front (ant-core keeps its own resolver private, so antd resolves via the public chunk_get + self_encryption::get_root_data_map), size from the root map, and hand the resolved map to file_download_to_sender, which then skips its internal resolution — wrapper chunks are fetched exactly once. Resolution failures now surface as a proper error response before the stream opens instead of a truncated 200. Linear: V2-1104 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Stable Rust moved to 1.98 since the last green run and its result_large_err lint now fires on functions returning Result<_, tonic::Status> (Status is >=176 bytes): 24 hits in antd-rust's tonic-generated client stubs (would fail on main too) and 1 on the new antd stream helper. The Status type is fixed by tonic's service contract and the generated code can't be reshaped, so allow the lint at those two sites. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
CI red was clippy: stable moved to 1.98 since the last green main run (08-20), and its |
…ngth-child-datamap # Conflicts: # antd-rust/src/grpc_client.rs
dirvine
left a comment
There was a problem hiding this comment.
Verdict: APPROVE recommendation — no blocking findings at 96f88a497e204e7af5cdda07aeee7cc4900375a0.
The fix addresses the truncation mechanism rather than papering over the stream: each REST/gRPC handler resolves a child DataMap before deriving Content-Length, NDJSON meta.total_size, or gRPC x-content-length, then passes the resolved map into file_download_to_sender. Because ant-core only resolves when is_child(), that does not fetch wrapper chunks twice. Resolution failure also occurs before the response/stream opens.
Independent review consensus found no correctness blocker. Local validation passed:
antd: fmt, 68 tests, clippy (-D warnings), docs build (one pre-existing brokenPaymentProofrustdoc-link warning)antd-rust: fmt, 82 unit + 2 integration tests, clippy (-D warnings), docs build- targeted DataMap regressions: 2/2 passed
- GitHub checks:
Check (antd),Check (antd-rust), andSecurity auditall green
Non-blocking follow-ups:
- The new unit test proves child-map size restoration, but an offline handler-level regression asserting full emitted byte count/header metadata would tighten coverage.
- The wrapper resolver uses the sequential
get_root_data_mappath rather than ant-core's parallel resolver. That is a small pre-stream latency cost (normally three wrapper chunks), not a correctness issue. - Missing wrapper chunks preserve
ant_core::data::Error::NotFoundinternally, but the existingAntdError::from_corecatch-all still exposes this as 500. This is pre-existing/out of scope and is already disclosed in the PR.
Submitted as a comment-only review under the repository review-authority policy; recommendation is APPROVE.
Summary
Bug report from SDK users: streaming download of larger files returns only a tiny prefix ("only a single chunk"). Root cause: files over 3 ×
MAX_CHUNK_SIZE(~12.5 MB) upload as a shrunk (child) DataMap, andDataMap::original_file_size()on a child map describes the serialized parent map (a few hundred bytes), not the plaintext. All three streaming paths sized their response from the caller-supplied map:rest/data.rsstream_responseContent-Lengthrest/data.rsstream_response_ndjsonmeta.total_sizegrpc/service.rsdata_chunk_stream_responsex-content-lengthThe truncation also breaks the documented short-read failure signal that antd-go/antd-js rely on. The buffered path was unaffected because ant-core's
data_downloadresolves child maps internally — but that resolver is private, so antd now resolves on its own.Fix
antd/src/datamap.rs: resolve a child DataMap to its root form via the publicClient::chunk_get+self_encryption::get_root_data_map(block_in_placebridge, mirroring ant-core's private resolver, including its error-taxonomy preservation — missing wrapper chunk staysNotFound, network failures keep theirTimeout/Networkclassification). Note:from_corehas noNotFoundarm, so a missing chunk still maps to 500/INTERNAL_ERRORtoday, same as the buffered path — a dedicated 404 mapping would be a separate, all-endpoints change.file_download_to_sender(which then skips its internal resolution — wrapper chunks are fetched exactly once).resolving_mapNDJSON/gRPC progress phase no longer occurs (resolution completes before the stream opens); proto comment updated.Verification
datamap.rs: a >12.5 MB in-memory encrypt yields a child map that misreports its size and resolves back to the true size/chunk count; a flat map passes through without any fetch.cargo test52/52, clippy-D warningsclean, fmt clean.POST /v1/data/stream→ HTTP 200,Content-Length: 490— client received 490 bytes of 20,971,520 as a "successful" download. NDJSONmeta.total_size: 490.POST /v1/data/stream→ HTTP 200,Content-Length: 20971520, 20,971,520 bytes received, sha256 identical to the uploaded file. NDJSONmeta.total_size: 20971520, full payload sha256-identical, phasesresolved → fetching(no moreresolving_map, as documented).POST /v1/data/getcontrol returned the full file, sha256-identical, in both runs — confirming only the stream sizing was broken.Linear: V2-1104
🤖 Generated with Claude Code