Capture and surface the real failure when placeholder hydration fails - #2124
Open
Tyrie Vella (tyrielv) wants to merge 1 commit into
Open
Tyrie Vella (tyrielv) wants to merge 1 commit into
Tyrie Vella (tyrielv) wants to merge 1 commit into
Conversation
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>
Tyrie Vella (tyrielv)
marked this pull request as ready for review
September 30, 2026 20:51
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem and Context
HydrateFile(the read that forces ProjFS to materialize a placeholder'scontent) catches
IOExceptionandUnauthorizedAccessExceptionand reportsonly "Failed to read
<path>". Nothing about the exception itself reachestelemetry: no exception type, no
HResult, no Win32 error code. Whenhydration 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
GetLastErroron failure either. This change ispure instrumentation — it does not alter control flow or return values on
any path.
Changes
IPlatformFileSystem.HydrateFilegains anout Exception failureparameter.
WindowsFileSystemnow populates it with the caught exceptioninstead of discarding it. The success path is unaffected (the out
parameter is simply left unset).
HydrationFailureDiagnosticshelper turns a failure exception intostructured
EventMetadata(exception type,HResult, and the Win32 errorcode derived from the
HResultwhen it is Win32-facility), used by bothHydrateFilesStage(the prefetch pipeline) and thegvfs prefetch --hydrateverb, so the fields are consistently queryable instead ofburied in a concatenated message string.
HydrateFilesStage's per-thread activity now also reports a count perdistinct 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
RelatedErrorand the summary'sStopevent both explicitly carryKeywords.Telemetryso they reachApplication Insights; the
--hydrateverb's warning does the same.MockPlatformFileSystem.HydrateFileis now scriptable (an injectabledelegate) instead of always throwing, enabling a new
HydrateFilesStageTestscovering both the success path and thefailure-aggregation path end to end.
WindowsFileSystemTestsnow pins the actual Win32-wrappedHResultvalues (sharing violation, access denied, missing file) instead of only
asserting non-zero, and
HydrationFailureDiagnosticsTestscoversUnauthorizedAccessException, theWin32=0boundary, and null-argumentguards added to
HydrationFailureDiagnostics.Known gaps and deliberate decisions
IPlatformFileSystem.HydrateFile's newout Exceptionparameter is source/binary breaking for any out-of-treeimplementer. No such consumer exists in this repo; both in-repo
implementers (
WindowsFileSystem,MockPlatformFileSystem) are updated.failureSignatureCountshas nocap 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.
Message/ToString()— unlike some otherexception-telemetry call sites in this repo,
BuildMetadatareports onlytype/
HResult/Win32 code, not the exception's message or stack. This isdeliberate: 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 —PrefetchVerbhas no existing unit-test harness and the method isprivate; adding one was out of scope for this change.
Branch target
Targeting
master: this is new instrumentation with no behavioral changeon 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.