Skip to content

Silent empty results on clone recovery and changelog fetch mask failures as success #400

Description

@Gravirei

Summary

Several code paths return an empty list / empty response when a downstream call fails, without surfacing the error. The user sees a "Done" or "Recovered 0 files" message — or a 200 { events: [], count: 0 } from /changelog — and has no way to tell whether the upstream is healthy. In one case (clone.rs:832-841) the file was just rewritten specifically to add a warning on the same anti-pattern; the line right below it still has the unfixed version.

Locations and specifics

1. crates/gl/src/clone.rs:320-326 — first encrypted-blobs fetch

let resp = match client
    .get_signed(&format!("/api/v1/repos/{owner}/{name}/encrypted-blobs"))
    .await
{
    Ok(r) if r.status().is_success() => r,
    _ => return Ok(vec![]),
};

Any non-2xx (or transport error) is dropped silently. The per-blob stage in the same function (clone.rs:368-375) calls warn_skip(...), so this fetch is the inconsistent outlier — the function otherwise surfaces per-blob failures but treats "couldn't fetch the list at all" as "no blobs to recover".

Suggested fix: match the per-blob warning style — eprintln!("warning: encrypted-blobs fetch failed: {e}") or equivalent — and return Ok(vec![]) only when the response was a definite empty list.

2. crates/gl/src/clone.rs:842-851 — Arweave/IPFS recovery

let from_arweave = recover_from_arweave(...)
    .await
    .unwrap_or_default();

This is the exact anti-pattern the lines above (clone.rs:832-841) were just rewritten to fix. The comment on the previous block explicitly calls out that unwrap_or_default() "silently swallows it into 'no paths'". The Arweave path is missing the equivalent fix:

let from_arweave = recover_from_arweave(...)
    .await
    .unwrap_or_else(|e| {
        eprintln!("warning: arweave/ipfs gateway recovery failed: {e}");
        Vec::new()
    });

3. crates/gitlawb-node/src/api/changelog.rs:46,63

let commits = store::log(&disk_path, &head_ref, limit).unwrap_or_default();
let prs = state.db.list_prs(&record.id).await.unwrap_or_default();

A corrupt git repo or a transient DB outage returns 200 { events: [], count: 0 }. The follow-up sort + truncate(limit) then masks the gap further. changelog is exposed to any caller that can resolve the repo, so an empty result here reads as "this repo has no history" — which is wrong.

Suggested fix: at minimum log a warning (tracing::warn!) on the unwrap branches; for the git path consider returning AppError::Git(...) so callers can distinguish "empty repo" from "we couldn't read it". Today the only way the handler returns a non-200 is for repo lookup / quarantined / store-acquire failures.

Why it matters

These three sites share one failure mode: an external system is unreachable / degraded, the user sees a clean success, and the operator has no signal. For changelog specifically, returning empty during a DB outage looks identical to a brand-new repo with no commits, which is exactly the kind of silent inconsistency that erodes trust in the timeline view.

Suggested fix (summary)

  1. clone.rs:320-326 — add a warning when the encrypted-blobs fetch fails for a reason other than a definite 200-empty.
  2. clone.rs:842-851 — bring recover_from_arweave in line with the recover_encrypted_blobs warning that now sits four lines above it.
  3. changelog.rs:46,63 — replace unwrap_or_default() with unwrap_or_else(|e| { tracing::warn!(...); default }), or surface the error to the caller when the source of truth is unreachable.

Add a regression test for the changelog path that injects a failing list_prs / failing store::log and asserts the response is either 5xx or carries a warning field, not a clean 200 with empty events.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    crate:glgl — the contributor CLIcrate:nodegitlawb-node — the serving node and REST APIkind:bugDefect fix — wrong or unsafe behaviorsev:mediumDegraded but workaround existssubsystem:apiNode REST API request/response surfacesubsystem:replicationMirror, replica, and cross-node syncsubsystem:storageBlob/object store, Arweave, IPFS, archives

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions