Skip to content

Capture and surface the real failure when placeholder hydration fails - #2124

Open
Tyrie Vella (tyrielv) wants to merge 1 commit into
microsoft:masterfrom
tyrielv:tyrielv/hydratefile-hresult
Open

Tyrie Vella (tyrielv) wants to merge 1 commit into
microsoft:masterfrom
tyrielv:tyrielv/hydratefile-hresult

Conversation

@tyrielv

Copy link
Copy Markdown
Contributor

Problem and Context

HydrateFile (the read that forces ProjFS to materialize a placeholder's
content) catches IOException and UnauthorizedAccessException and reports
only "Failed to read <path>". Nothing about the exception itself reaches
telemetry: no exception type, no HResult, no Win32 error code. When
hydration fails at scale, there is no way to tell from telemetry alone
whether the cause was a sharing violation, access denied, a missing file, or
something else — every failure looks identical after the fact.

This is a long-standing gap, not a regression: the prior P/Invoke-based
implementation never called GetLastError on failure either. This change is
pure instrumentation — it does not alter control flow or return values on
any path.

Changes

  • IPlatformFileSystem.HydrateFile gains an out Exception failure
    parameter. WindowsFileSystem now populates it with the caught exception
    instead of discarding it. The success path is unaffected (the out
    parameter is simply left unset).
  • New HydrationFailureDiagnostics helper turns a failure exception into
    structured EventMetadata (exception type, HResult, and the Win32 error
    code derived from the HResult when it is Win32-facility), used by both
    HydrateFilesStage (the prefetch pipeline) and the gvfs prefetch --hydrate verb, so the fields are consistently queryable instead of
    buried in a concatenated message string.
  • HydrateFilesStage's per-thread activity now also reports a count per
    distinct failure signature (exception type + Win32 code) on its summary
    event, so a large failed prefetch can be triaged from one event instead
    of one query per file. The per-file RelatedError and the summary's
    Stop event both explicitly carry Keywords.Telemetry so they reach
    Application Insights; the --hydrate verb's warning does the same.
  • MockPlatformFileSystem.HydrateFile is now scriptable (an injectable
    delegate) instead of always throwing, enabling a new
    HydrateFilesStageTests covering both the success path and the
    failure-aggregation path end to end.
  • WindowsFileSystemTests now pins the actual Win32-wrapped HResult
    values (sharing violation, access denied, missing file) instead of only
    asserting non-zero, and HydrationFailureDiagnosticsTests covers
    UnauthorizedAccessException, the Win32=0 boundary, and null-argument
    guards added to HydrationFailureDiagnostics.

Known gaps and deliberate decisions

  • Interface signature change — IPlatformFileSystem.HydrateFile's new
    out Exception parameter is source/binary breaking for any out-of-tree
    implementer. No such consumer exists in this repo; both in-repo
    implementers (WindowsFileSystem, MockPlatformFileSystem) are updated.
  • Unbounded failure-signature growth — failureSignatureCounts has no
    cap on distinct keys. In practice this is bounded by the number of
    distinct (exception type, Win32 code) pairs actually seen, which is small;
    no cap was added since one hasn't been needed in practice.
  • Diagnostic schema omits Message/ToString() — unlike some other
    exception-telemetry call sites in this repo, BuildMetadata reports only
    type/HResult/Win32 code, not the exception's message or stack. This is
    deliberate: the triage need here is a small number of structured,
    Kusto-queryable fields, not free-text search, and it keeps the event
    minimal.
  • gvfs prefetch --hydrate's failure branch has no dedicated test —
    PrefetchVerb has no existing unit-test harness and the method is
    private; adding one was out of scope for this change.

Branch target

Targeting master: this is new instrumentation with no behavioral change
on the success path, and the gap it fixes predates this release rather than
being introduced by it, so it carries negligible blast radius for the
shippable line.

HydrateFile swallowed IOException and UnauthorizedAccessException without
recording anything about them, so a failed hydration only ever produced
"Failed to read <path>" in telemetry with no exception type, HResult, or
Win32 error code. That makes every such failure unattributable after the
fact.

HydrateFile on IPlatformFileSystem now has an `out Exception failure`
parameter. WindowsFileSystem populates it with the caught exception
instead of discarding it; the out parameter costs nothing on the (much
more common) success path, so the ~36-40K files/s hydration pipeline is
unaffected there.

HydrateFilesStage and the `gvfs prefetch --hydrate` verb turn that
exception into structured EventMetadata (exception type, HResult, and the
Win32 error code derived from the HResult when it is Win32-facility)
via a new HydrationFailureDiagnostics helper, so the fields are queryable
in telemetry instead of being buried in a concatenated string. The
existing "Failed to read <path>" message text is unchanged. The
per-thread activity summary also now reports a count per distinct failure
signature, so a large failed prefetch can be triaged from the summary
event alone instead of requiring one query per file.

Checked whether the prior P/Invoke-based implementation (replaced by the
managed FileStream rewrite) ever surfaced a Win32 error: it did not -
`CreateFile` failures produced an invalid handle and the method returned
false with no call to GetLastError. Neither implementation ever captured
diagnostics here, so this is a long-standing gap rather than a
regression. It is also a pure instrumentation change with no effect on
control flow or return values for any input.

Self-review found that the new failure-signature summary and the
`--hydrate` verb's warning both defaulted to Keywords.None, so neither
ever reached the telemetry pipe the aggregation was built for (the
per-file error already did). Both now pass Keywords.Telemetry explicitly.
Also added: a HydrateFilesStage test covering the aggregation path (the
mock file system is now scriptable instead of always throwing), pinned
Win32 HResult assertions in WindowsFileSystemTests instead of only
checking non-zero, a missing-file test case, and null-argument guards in
HydrationFailureDiagnostics.

Assisted-by: Claude Sonnet 5
Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
@tyrielv
Tyrie Vella (tyrielv) marked this pull request as ready for review September 30, 2026 20:51
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