Skip to content

fix(file-retriever): fall back to the DSN when a CID is not in the DAG indexer - #173

Open
EmilFattakhov wants to merge 4 commits into
mainfrom
fix/dsn-fallback-for-unindexed-dag-nodes
Open

fix(file-retriever): fall back to the DSN when a CID is not in the DAG indexer#173
EmilFattakhov wants to merge 4 commits into
mainfrom
fix/dsn-fallback-for-unindexed-dag-nodes

Conversation

@EmilFattakhov

@EmilFattakhov EmilFattakhov commented Jul 27, 2026

Copy link
Copy Markdown
Member

Problem

GET /files/:cid/metadata returns a hard 404 for any CID the DAG indexer has no row for:

// services/file-retriever/src/services/dsnFetcher.ts
const fetchNodeMetadata = async (cid: string) => {
  const node = await dagIndexerRepository.getDagNode(cid)
  if (!node) {
    throw new HttpError(404, 'Not found: Failed to get node metadata')
  }
  return node
}

Since the DAG indexer became the sole source of node metadata and chunk ordering, an indexer miss makes a file unservable — but a miss is not evidence that the file is absent. The indexer trails the chain, and handleCall swallows per-extrinsic decode/save failures, so a node it misses leaves a permanent gap even after it catches up. getFileChunks has the same problem, failing with a 500 instead.

Observed on mainnet via Auto Drive: objects Auto Drive reports as fully archived fail retrieval in under 0.5 s with

500 {"error":"Failed to retrieve data","details":"Error fetching file header: 404 Not Found"}

which is @autonomys/auto-files reporting a non-200 from GET /files/:cid/metadata. Because a 404 reads as permanent, downstream consumers treat these as dead objects rather than retrying later.

Root cause, now measured

Production investigation confirmed the mechanism and, importantly, its scale:

  • GET /files/<cid>/metadata returns 404 {"error":"Not found: Failed to get node metadata"} — the getDagNode miss above.
  • The DAG indexer is wedged, not merely lagging. Its frontier sat at exactly block 8,843,781 across a 53-minute observation window — ~56,000 blocks, ~4 days behind chain head. It had previously tracked chain at ~97% of block rate for months, so this is a recent stall.
  • The Object Mapping Indexer is healthy: it returns valid mappings for every affected CID, including the failing root and its leaves, in ~130 ms. The fallback's precondition holds.

That reframes this PR. It was written for a rare indexer gap; with days of backlog, every recently uploaded archived file takes the fallback path, so it is the primary read path for recent content rather than an exceptional one. Most of the guardrail work below exists because of that reframing.

This PR is containment. The cure is unwedging the indexer.

What this changes

1. Serve the file instead of reporting a miss

On an indexer miss, reconstruct what the indexer would have provided — the capability the service had before the indexer refactor, using machinery that is still present (object mappings + fetchNode):

  • Metadata is decoded from the head node fetched via its object mapping. The CID commits to those bytes, so type/size/name/links/uploadOptions are authoritative. Chain provenance (block, extrinsic, timestamp) is left empty rather than invented, because only the indexer has it and nothing on the retrieval path reads it. Verified field-by-field against the indexer's own writer (dag-indexer/src/mappings/handleCall.ts), including that size defaults to 0 exactly as the indexer does — passing undefined through would make Number(metadata.size) NaN and silently defeat the 416 guard.
  • Chunk ordering comes from a depth-first walk of the DAG, which yields leaves in file order.

2. Guardrails for the fallback as a hot path

Each of these is a separate commit with its discovery documented.

  • Chunk-list TTL is refreshed on read (updateAgeOnGet). The SDK downloads a file one /files/:cid/partial?chunk=N request at a time and every one calls getFileChunks, so without this the TTL was a wall clock on the download rather than on idle time: any transfer outliving it re-walked the whole DAG mid-stream, and a re-walk of a large DAG can itself outlast the TTL.
  • Concurrent rebuilds of the same CID share one walk. The cache is only written when a walk finishes, so overlapping requests each started their own full traversal of the same DAG.
  • The chunk-list cache is bounded by chunk count, not entry count. max: 500 said nothing about memory when one entry holds up to maxNodes (5000) records — roughly 1.2 GB at capacity.
  • Reconstruction has a wall-clock deadline (DAG_INDEXER_FALLBACK_DEADLINE_MS, default 45 s). A node fetch may take FETCH_TIMEOUT (180 s) and is retried three times, so a rebuild had no effective bound — while Auto Drive abandons the gateway after 60 s. The check is cooperative rather than a Promise.race, so a timed-out walk stops fetching instead of continuing in the background for a request that has gone away.
  • The sibling-batch retry only fires for a real miss. get_object_mappings rejects the whole batch when any hash is unknown, so the target-alone retry exists for that case — but it previously fired on any failure, doubling request volume and time-to-failure exactly when the indexer is already unhealthy, once per cold node during a walk.

3. Chunk ordering on the indexed path was wrong

Found while checking this PR's own claim that the DSN walk matches the indexer's link_order. It does not, and the indexer was the one that was wrong.

link_order is a node's ordinality within its immediate parent, so every inlink's children restart at 1, and ordering leaves by it interleaves them. Reproduced against a real PostgreSQL 17 — a head with two inlinks over c1..c5 returned:

c1,c4,c2,c5,c3     instead of     c1,c2,c3,c4,c5

Now ordered by the accumulated path of link positions (int[], compared element-wise), which is exactly depth-first order. Single-level and single-node files are unaffected.

This is load-bearing for the rollout order. Multi-level DAGs begin above DEFAULT_MAX_LINK_PER_NODE (1626 chunks, ~106 MB). Every such file uploaded during the current gap is unindexed and therefore served by the (correct) fallback right now — so catching the indexer up is precisely what would activate this corruption. It needs to ship before the re-sync completes, not after.

4. Make the remaining failures honest

Callers can pick a retry strategy from a reason code instead of parsing messages:

condition before after
indexed 200 200 (unchanged)
not indexed, retrievable from the DSN 404 200
not indexed, mapping exists, gateway returned no bytes 404 503 object_not_retrievable_yet + Retry-After
mapping lookup itself failed (timeout / unreachable / faulting) 404 / 500 503 object_mapping_lookup_failed + Retry-After
nothing knows the CID 404 / 500 404 object_not_found
DAG too large to rebuild 503 dag_too_large_for_fallback (no Retry-After; cached)
reconstruction exceeded its budget 503 dag_indexer_fallback_timed_out + Retry-After

dag_too_large_for_fallback is 503 rather than 500: nothing has faulted, the service is declining a request it cannot serve until a dependency catches up. As a 500 a deliberate, deterministic refusal landed in 5xx alerting and would page on-call for a non-incident.

Related fixes in the same path:

  • fetchNode resolved undefined when the batch response didn't contain the requested node, which surfaced as a bare 404 on /files/:cid/metadata and an empty 200 on /nodes/:cid. It now throws, and its declared Promise<PBNode> return type is true.
  • fetchFile no longer flattens typed HttpErrors into 500, which told callers to give up on retryable failures.
  • errorMiddleware is now registered. It was exported but never wired — index.ts had its own inline handler — so it had been dead code, and this PR initially taught both copies about reason/Retry-After and added tests for the copy that never runs. Those tests asserted a body shape ({error: 'HttpError', message}) production has never served. Unified on the shape that actually ships ({error: <message>, reason?}); changing live responses to match unreachable code would have been a gratuitous API break. The handler now also logs via winston rather than console.error and tolerates a thrown non-Error.

Config

All optional; the fallback is on by default. Set DAG_INDEXER_FALLBACK_ENABLED=false to restore the previous fail-on-miss behaviour.

Variable Default Notes
DAG_INDEXER_FALLBACK_ENABLED true
DAG_INDEXER_FALLBACK_MAX_NODES 5000 ~320 MB of file
DAG_INDEXER_FALLBACK_CHUNK_LIST_CACHE_SIZE 500 entry count
DAG_INDEXER_FALLBACK_CHUNK_LIST_CACHE_MAX_CHUNKS 100000 the real memory cap
DAG_INDEXER_FALLBACK_CHUNK_LIST_CACHE_TTL 600000 idle TTL
DAG_INDEXER_FALLBACK_DEADLINE_MS 45000 keep below the caller's timeout
DAG_INDEXER_LAG_ALERT_BLOCKS 1000
UNAVAILABLE_RETRY_AFTER_SECONDS 60

Testing

71 tests, 9 suites (was 54/7). lint, build and test all pass.

Every regression test was checked for vacuousness by reverting its fix and confirming failure. That caught two real problems:

  • A first attempt at the TTL test used jest fake timers and passed with and without the fixlru-cache captures the performance object at import and never sees the fake clock. Rewritten against a real short TTL, where it fails Expected: 3, Received: 6.
  • The existing fails fast on retry instead of re-walking the DAG test was passing for the wrong reason. Both over-large-DAG tests build their fixture from identical content, so they share a CID, and the module-level rejection cache was never cleared — the earlier test poisoned it, so the later test's first attempt short-circuited and never walked. The assertion was 0 === 0. There is now a resetDagIndexerFallbackState() seam plus a permanent guard (expect(afterFirstAttempt).toBeGreaterThan(0)) so it cannot hollow out again.

Two behaviours were verified against real dependencies rather than assumed:

  • PostgreSQL 17 — the chunk-ordering bug and its fix, and that pg returns jsonb as JS numbers/booleans (which the health-check type guards depend on).
  • lru-cache — that an entry larger than the whole budget is silently not stored rather than throwing (so an oversized DAG degrades to re-walking, not a 500), and that a sizeCalculation returning 0 does throw, which is why the empty-list case falls back to 1.

Not verified: the fallback has not been exercised against a live mainnet gateway. GET /nodes/:cid/ipld for affected CIDs returned 504 at ~120 s from outside the VPC — that is shorter than a single origin attempt (FETCH_TIMEOUT 180 s), so it reads as an edge timeout rather than proof the DSN failed, but it is unconfirmed. Worth running from inside the VPC against origin :8090 with a client timeout above 540 s before merge.

Known gaps

  • The deadline bounds the multi-node walk, not a single hung node fetch, which is still bounded only by FETCH_TIMEOUT × 3 retries. That is the case that affects /files/:cid/metadata. Closing it means plumbing a per-request timeout into fetchObjects.
  • Nothing consumes reason or Retry-After yet: getChunkedFile in @autonomys/auto-files discards the response body and retries a hard 404 four times. This PR is groundwork for that.
  • Auto Drive's FILES_GATEWAY_FETCH_TIMEOUT_MS is 60 s against the gateway's 45 s deadline; it needs raising in tandem for large files.

Follow-ups (not in this PR)

  • The DAG indexer runs --workers=1 --batch-size=125 --disable-historical=false. A single worker with historical state tracking leaves no headroom to catch up after a stall.
  • object-mapping-indexer's get_object_mappings throws Object mapping not found for an entire batch when a single hash is unknown; a tolerant variant (or a distinguishable error) would remove the need for the target-alone retry.
  • dag-indexer's handleCall catches every error with one generic log line and drops the node, which is what makes indexer gaps permanent — worth surfacing as a metric.
  • OBJECT_MAPPING_INDEXER_URL must end in exactly /ws (the rpc package serves HTTP JSON-RPC only at POST /ws, and a trailing slash breaks the match), but .env.sample and docs/development.md both omit it. A wrong value yields a fast plain-text 500 that looks like a DSN problem.

🤖 Generated with Claude Code

Comment thread services/file-retriever/src/services/dsnFetcher.ts
Comment thread services/file-retriever/src/services/dsnFetcher.ts Outdated
@EmilFattakhov

Copy link
Copy Markdown
Member Author

bugbot run

@cursor cursor 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.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 8e506e0. Configure here.

@EmilFattakhov

Copy link
Copy Markdown
Member Author

bugbot run

Comment thread services/file-retriever/src/services/dsnFetcher.ts
@EmilFattakhov

Copy link
Copy Markdown
Member Author

bugbot run

Comment thread services/file-retriever/src/services/dsnFetcher.ts
Comment thread services/file-retriever/src/services/dsnFetcher.ts
@EmilFattakhov

Copy link
Copy Markdown
Member Author

bugbot run

Comment thread services/file-retriever/src/repositories/dag-indexer.ts
…G indexer

`GET /files/:cid/metadata` returned a hard 404 for any CID the DAG Indexer had no
row for. Since the indexer became the sole source of node metadata and chunk
ordering, a miss made a file unservable — but a miss is not evidence the file is
absent. The indexer trails the chain, and `handleCall` swallows per-extrinsic
failures, so a node it misses leaves a permanent gap even after it catches up.
`getFileChunks` had the same problem, failing with a 500.

Observed on mainnet: objects Auto Drive reported as fully archived failed
retrieval in under 0.5s. The DAG Indexer was wedged, not merely lagging — its
frontier sat on block 8,843,781 across a 53-minute window, ~4 days behind head —
while the Object Mapping Indexer answered for every affected CID in ~130ms. So
every recently uploaded archived file took this path, making it the primary read
path for recent content rather than an exceptional one.

This is containment. The cure is unwedging the indexer.

Serve the file instead of reporting a miss:

- Metadata is decoded from the head node fetched via its object mapping. The CID
  commits to those bytes, so type/size/name/links/uploadOptions are
  authoritative. Chain provenance is left empty rather than invented, because
  only the indexer has it and nothing on the retrieval path reads it. `size`
  defaults to 0 exactly as the indexer does; `undefined` would make
  `Number(metadata.size)` NaN and silently defeat the 416 guard.
- Chunk ordering comes from a depth-first walk of the DAG, which yields leaves in
  file order.

Guardrails, because the fallback is a hot path:

- The chunk-list TTL is refreshed on read. The SDK downloads one
  `/files/:cid/partial?chunk=N` at a time and every one calls `getFileChunks`, so
  without this the TTL was a wall clock on the download rather than on idle time.
- Concurrent rebuilds of the same CID share one walk; the cache is only written
  when a walk finishes, so overlapping requests each started their own.
- The cache is bounded by chunk count, not entry count: `max: 500` said nothing
  about memory when one entry holds up to `maxNodes` records (~1.2 GB at
  capacity).
- Reconstruction has a wall-clock deadline, and it bounds each gateway fetch as
  well as the walk: one fetch may take FETCH_TIMEOUT (180s) and is retried three
  times, so a slow call would otherwise overrun a 45s budget by an order of
  magnitude and outlive the caller regardless. The check is cooperative rather
  than a `Promise.race`, so a timed-out walk stops fetching instead of continuing
  for a request that has gone away. It covers the metadata rebuild too — one node
  is enough to hang past any caller's patience.
- The sibling-batch retry only fires for a real miss. `get_object_mappings`
  rejects the whole batch when any hash is unknown, so the target-alone retry
  exists for that case, but firing on any failure doubled request volume and
  time-to-failure exactly when the indexer was already unhealthy.

Chunk ordering on the *indexed* path was also wrong, and this is load-bearing for
rollout order. `link_order` is a node's ordinality within its immediate parent, so
every inlink's children restart at 1 and ordering leaves by it interleaves them.
Verified against PostgreSQL 17: a head with two inlinks over c1..c5 returned
c1,c4,c2,c5,c3. Now ordered by the accumulated path of link positions (`int[]`,
compared element-wise), which is exactly depth-first order. Multi-level DAGs begin
above DEFAULT_MAX_LINK_PER_NODE (~106 MB), so every such file uploaded during the
gap is served by the (correct) fallback today — catching the indexer up is what
would activate this corruption.

A missing *head* is not the only failure mode. Nodes are indexed one extrinsic at
a time, so a file's head can be indexed while nodes below it are not. The
chunk-list query inner-joined `nodes` per link and silently dropped any it had no
row for: a missing leaf shortened the list, a missing inlink pruned its subtree,
and nothing indexed under the head produced an empty list — an empty 200 for a
file that exists. Unresolved links are now reported rather than dropped, and any
of them routes to the DSN rebuild, so a partially indexed file is served correctly
or fails honestly, never truncated.

Make the remaining failures honest. Callers pick a retry strategy from a `reason`
code instead of parsing messages:

| condition                                    | before      | after                                       |
| -------------------------------------------- | ----------- | ------------------------------------------- |
| not indexed, retrievable from the DSN        | 404         | 200                                         |
| not indexed, mapping exists, no bytes yet    | 404         | 503 object_not_retrievable_yet + Retry-After|
| mapping lookup itself failed                 | 404/500     | 503 object_mapping_lookup_failed + Retry-After|
| nothing knows the CID                        | 404/500     | 404 object_not_found                        |
| DAG too large to rebuild                     | —           | 503 dag_too_large_for_fallback (cached)     |
| reconstruction exceeded its budget           | —           | 503 dag_indexer_fallback_timed_out          |

`dag_too_large_for_fallback` is 503 rather than 500: nothing has faulted, the
service is declining a request it cannot serve until a dependency catches up. As a
500 a deterministic refusal would page on-call for a non-incident.

Related fixes on the same path:

- `fetchNode` resolved `undefined` when the batch response omitted the requested
  node, surfacing as a bare 404 on `/files/:cid/metadata` and an empty 200 on
  `/nodes/:cid`. It now throws, and its declared return type is true.
- `fetchFile` no longer flattens typed `HttpError`s into 500.
- `isActuallyCompressed` no longer swallows every failure into `false`. That
  default is only safe for "readable and not zlib"; when the bytes cannot be read
  it strips `Content-Encoding` from a body that really is compressed.
- `errorMiddleware` is now registered *and* declares the four parameters Express
  requires. Express identifies an error handler by arity, so a three-parameter
  handler is silently demoted to ordinary middleware: errors bypassed it for an
  HTML body with no `reason`, and unmatched routes died on `res.status is not a
  function`. The body shape is the one production has always served
  (`{error, reason?}`), not the never-reached shape the dead code described.

Observability, since reconstruction converts a loud outage into extra latency:

- `/health/dag-indexer` exposes the frontier — `lagBlocks`,
  `lastProcessedHeight`, `targetHeight`, `indexerHealthy` — and returns 503 past
  `DAG_INDEXER_LAG_ALERT_BLOCKS`. `/health` stays pure liveness on purpose: a
  lagging indexer is degradation, not death, and this service is the only thing
  still able to serve those files. Both SubQuery timestamps are reported, because
  frontier block time says how stale the data is while indexer activity time says
  whether it is moving, and only the pair distinguishes a backlog from the wedge.
- A `dag_indexer_fallback` counter tagged by outcome, with `nodes_walked`,
  `chunk_count` and `unindexed_links` as separate fields — one field carrying all
  three made cache hits (one per chunk request) report 25M walked nodes for a
  rebuild that visited 5000, while multi-level rebuilds under-reported by omitting
  inlinks.

All config is optional and the fallback is on by default;
`DAG_INDEXER_FALLBACK_ENABLED=false` restores the previous fail-on-miss
behaviour. See docs/file-retriever.md and the service README for the knobs.

97 tests across 10 suites; lint, build and test pass. Every regression test was
checked by reverting its fix and confirming failure, which caught two tests that
passed either way: one used fake timers that `lru-cache` never sees, and the
over-large-DAG test was satisfied by a sibling test's cached rejection, asserting
0 === 0. `getSortedChunksByCid` is deliberately not unit-tested — its behaviour is
its SQL, which a mocked client cannot exercise; both bugs there were verified
against a real PostgreSQL 17 and the README records how to repeat it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@EmilFattakhov
EmilFattakhov force-pushed the fix/dsn-fallback-for-unindexed-dag-nodes branch from 233e01e to 8040054 Compare July 31, 2026 16:29
@EmilFattakhov

Copy link
Copy Markdown
Member Author

bugbot run

Comment thread services/file-retriever/src/services/dsnFetcher.ts
Comment thread services/file-retriever/src/services/dsnFetcher.ts
… (Bugbot)

Two failures on the DAG Indexer fallback path escaped the typed error handling
the fallback is built on.

**Gateway errors escaped the typed fallback.** `fetchObjects` raised typed errors
for a non-2xx status and an unparseable body, but a raw axios rejection — the
ordinary shape of a flaky or unreachable gateway — went straight through
`fetchNode`, `fetchNodeMetadataFromDsn` and `withDeadlineReporting` (whose
`deadline.check()` is a no-op while budget remains) to `errorMiddleware`, which
answers `res.status(500).send('Internal Server Error')`: plain text, no `reason`,
no `Retry-After`. Callers keying on the reason code to pick a retry strategy were
told to give up on the one failure most likely to clear on its own. This is new
exposure — before the fallback, `GET /files/:cid/metadata` answered a miss with
404 and never consulted the gateway at all.

`fetchObjects` now classifies on the way out: an `AxiosError` becomes 503
`dsn_gateway_fetch_failed` with `Retry-After`, and anything else — a response
that parsed but would not decode — becomes a typed 500, deliberately not
advertised as retryable. The new reason is kept separate from
`object_not_retrievable_yet` because that one is a verdict about the object,
while a gateway that has stopped answering is the absence of a verdict. Doing
this inside `fetchObjects` rather than at the fallback boundary means ordinary
downloads of indexed files get the same treatment.

**Retries ran past the deadline.** `budgetedFetchTimeout` floors each attempt at
1s, and `withRetries` still ran three of them with delays in between after the
budget was spent — the deadline is only re-checked once the whole retry loop has
finished, so no amount of checking *between* fetches could undo it. One node
fetch could finish several seconds past `DAG_INDEXER_FALLBACK_DEADLINE_MS`
despite the docs promising each fetch no more than the time left.

`withRetries` takes a `shouldRetry` predicate and no longer sleeps after its
final attempt (nothing follows it, so the delay only postpones a failure the
caller is already blocked on). `fetchObjects` declines to retry once the budget
is spent, and also refuses to *start* an attempt in that state: the mapping
lookup ahead of each fetch is not itself budgeted, so the first attempt can be
reached over budget too, which the reported case did not cover. The residual
overrun is one attempt's 1s floor rather than three floors plus their delays,
and the docs now say that instead of claiming otherwise. The floor stays — a
timeout of a few milliseconds fails every request it is applied to.

Found while re-reviewing the branch:

- Five files had drifted from `.prettierrc`. `prettier/prettier` is not enabled
  in this repo's eslint config, so `yarn lint` passed either way; every base
  version was clean, so the drift was introduced here. Reformatting stripped the
  quotes from the line-protocol field names, which stranded a comment explaining
  the quotes and raised three `camelcase` warnings — comment rewritten and the
  rule scoped off for those three wire-format names.
- `DAG_INDEXER_FALLBACK_ENABLED=false` does not "restore the previous behaviour"
  for a partially indexed file: it now fails rather than serving truncated bytes
  under a 200. Documented as the deliberate departure it is.

108 tests across 11 suites; lint (no warnings), prettier, build and test pass.
Every new test was checked by reverting its fix: three fail without the
classification, two without the budget guards, one without the trailing-sleep
fix, and two do not compile without `shouldRetry`.

Reviewed and left alone: the recursive CTE's `link_path` ordering and
`LEFT JOIN`/`missing` handling, the in-flight rebuild dedup and its settle-time
cleanup, `lru-cache` v11 with `maxSize` plus `sizeCalculation` (it defaults
`maxEntrySize` to `maxSize`, so an oversized chunk list really is silently not
cached, as the README says, rather than throwing), the `errorMiddleware` arity,
and the fallback metric accounting.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@EmilFattakhov

Copy link
Copy Markdown
Member Author

bugbot run

Comment thread services/file-retriever/src/services/dsnFetcher.ts
…r (Bugbot)

A partially indexed file paid for the whole discovery on every request. Its head
*is* indexed, so `getFileChunks` ran `getSortedChunksByCid` — a recursive CTE over
the file's entire DAG — and only that result revealed the unresolved links, after
which the already-cached DSN rebuild was returned anyway. Since the SDK downloads
a file one `/files/:cid/partial?chunk=N` request at a time and every one of those
calls `getFileChunks`, a single download re-ran that query, re-emitted
`chunk_list_incomplete` and re-logged its warning once per chunk of the file. A
fully unindexed file never paid it: a missing head reaches the cache before the
indexer is asked anything.

This defeated two of the guardrails added with the fallback. The chunk-list cache
exists precisely so a chunk-by-chunk download does not re-derive the list per
request, and `chunk_list_incomplete` is documented as worth its own alert — which
it cannot be while one download of one file emits thousands of points, the same
inflation that made `nodes_walked` unreadable before it was split out.

`getFileChunks` now consults the rebuilt-list cache first, via a helper shared
with `getFileChunksFromDsn` so the cache hit is still counted as one. Returning a
cached rebuild without asking the indexer costs nothing in correctness even for a
file indexed since: a rebuild is a full depth-first walk of content-addressed
nodes, so it carries the same leaves in the same order as the indexed query, and
the fields read downstream (`cid`, `size`) come from the node bytes the CID
commits to. Chain provenance is the only thing it lacks and nothing on the
retrieval path reads it; the TTL bounds how long the indexer stays unasked.

109 tests across 11 suites; lint, prettier, build and test pass. The regression
test was checked by reverting the fix: the indexer is asked twice for two calls
instead of once.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@EmilFattakhov

Copy link
Copy Markdown
Member Author

bugbot run

Comment thread services/file-retriever/src/services/dsnFetcher.ts
… have them (Bugbot)

The compression probe runs on every download of a ZLIB-flagged file, including
one served entirely from `fileCache` — and it read the file's head from the DSN.
Since the probe now rethrows a typed failure rather than guessing "uncompressed",
that made a download needing no DSN at all fail when the DSN was unavailable.

The two caches have very different lifetimes: a rebuilt chunk list lives 10
minutes, a cached file 24 hours. So for an unindexed file, ten minutes after the
first download the probe re-walks the whole DAG, and a slow or unreachable gateway
turns a local cache hit into a 503 `dag_indexer_fallback_timed_out` — for bytes
sitting on local disk. Exactly during an indexer gap, when the fallback makes
unindexed files the hot read path and the cache is what keeps them served.

Read the head from the cheapest source that has it: the cached copy when there is
one, the DSN only otherwise. The cached body is the stored bytes — `fileComposer`
forks the response before any content transform — so its first two bytes are the
first chunk's first two bytes, which is all `isZlibCompressed` reads. That also
holds under `originControl=no-cache`: that asks for fresh bytes, while this asks
whether the content is zlib, and content does not change under a CID.

A cache read that *fails* falls through to the DSN rather than becoming an answer,
because it is not evidence about the bytes. And the throw stays for the case it
was added for: when nothing local holds the bytes, both wrong answers corrupt the
body — stripping `Content-Encoding` from a compressed one, or advertising it over
plaintext (#169) — so there is no safe default and the request must fail honestly.

Reading the head no longer means draining the body either: `readLeadingBytes`
stops and destroys the stream once it has the two bytes, which matters when the
source is a cache file of arbitrary size.

112 tests across 11 suites; lint, prettier, build and test pass. The existing
probe tests now pin `fileCache.get` to a miss — with a stray cache entry present,
a test asserting the DSN path is consulted would have passed for the wrong reason.
Both new cached-path tests were checked by reverting the fix.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@EmilFattakhov

Copy link
Copy Markdown
Member Author

bugbot run

@cursor cursor 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.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 8518ebf. Configure here.

Comment thread services/file-retriever/src/services/dsnFetcher.ts
@EmilFattakhov

Copy link
Copy Markdown
Member Author

The PR size grew from large to enormous while I was addressing some bugbot findings. The core concept stays the same, however, now more edge cases are covered + test coverage is expanded significantly.

@EmilFattakhov

Copy link
Copy Markdown
Member Author

I actually wonder if the solution is too complex, if all presumptions are right it might work amazing, but if some assumptions are wrong we might be shooting ourselves in the foot and over-complicate DSN reconstructions and function signatures. Curious to hear your honest opinion - again given the PR size, first review should probably by done by AI

@jim-counter

Copy link
Copy Markdown
Member

Suggest we revisit this after we've seen the results of reconfiguring the DAG indexer in #174.

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.

2 participants