DiffViewer: SafeReadSide swallows all errors silently

[geevensingh]

Problem

DiffPaneViewModel.SafeReadSide (DiffViewer/ViewModels/DiffPaneViewModel.cs:470-481) silently swallows every exception and returns an empty string:

private string SafeReadSide(FileChange change, ChangeSide side)
{
    try
    {
        var blob = _repository.ReadSide(change, side);
        return blob.Text ?? string.Empty;
    }
    catch
    {
        return string.Empty;
    }
}

SafeReadSide is called twice from LoadAsync (DiffViewer/ViewModels/DiffPaneViewModel.cs:328-329) on a background thread:

string left = SafeReadSide(change, ChangeSide.Left);
string right = SafeReadSide(change, ChangeSide.Right);
var (hunks, map, inline, ws) = ComputeDiffArtifacts(left, right, options, intraLine);

If reading either side throws (IO error, repository lost mid-load, encoding failure, OOM, etc.), the user sees:

• An empty pane on one side
• A misleading "all lines added/removed" diff if only one side failed
• No error indicator, no toast, no logged warning

The in-flight ContinueWith further down (DiffViewer/ViewModels/DiffPaneViewModel.cs:336-346) does handle faults from the diff computation itself with a proper placeholder:

if (t.IsFaulted)
{
    ApplyResult(string.Empty, string.Empty,
        $"Failed to read blobs: {t.Exception?.GetBaseException().Message}",
        ...);
}

…but SafeReadSide's catch prevents the task from ever faulting, so this branch is unreachable for read failures.

Why it matters

• Repo-loss events during load (RepositoryService.cs:146,172,196 document this scenario) produce a confusing empty diff instead of a clear "couldn't read blob" message.
• IO errors (file locked by AV, permission denied, network drive disconnect) silently corrupt the diff display.
• Critical exceptions (OOM, ThreadAbort) are caught when they shouldn't be — the catch has no exception filter at all.

Suggested fix

Three options, in order of preference:

Option A — let it fault, surface in the existing handler: Remove SafeReadSide entirely and call _repository.ReadSide(change, side).Text ?? "" directly inside the Task.Run. The existing t.IsFaulted branch in the ContinueWith will then surface the error via ApplyResult(..., placeholder: "Failed to read blobs: ..."). Cleanest fix.

Option B — surface from inside SafeReadSide: Capture the exception, return a sentinel (e.g., null), and have LoadAsync check for it and route to ApplyResult with a side-specific placeholder ("Failed to read left side: …").

Option C — at minimum, narrow the catch: Add an exception filter when (ex is IOException or LibGit2SharpException or InvalidOperationException) so OOM / ThreadAbort / StackOverflow propagate. Still loses the user-facing signal but stops masking critical failures.

I'd recommend Option A — the surrounding code already has the right shape for it.

Citations

• DiffViewer/ViewModels/DiffPaneViewModel.cs:470-481 — the offending method
• DiffViewer/ViewModels/DiffPaneViewModel.cs:328-329 — call sites
• DiffViewer/ViewModels/DiffPaneViewModel.cs:336-346 — existing fault-handling branch that's currently unreachable for read errors
• DiffViewer/Services/RepositoryService.cs:146,172,196 — IsRepoLossException filter pattern (good model elsewhere)

Severity

Medium. Not a crash, but silent data-quality bug + masks critical exceptions.