diff --git a/CHANGELOG.md b/CHANGELOG.md index c1bcc4738..c59875378 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -261,6 +261,25 @@ Conventions for contributors: ### Fixed +- **`RenderError` names the component that threw, and now reports every render + error** (issue #1321, spec 044 §6.2.1). For class components the event reported + the element record (``ComponentElement`1`` / `ComponentElement`) instead of the + component unless the `Render` keyword was also on, so an Errors-only listener + such as `winapp devtools logs` could not tell which component failed. It now + reports the component's type name (`Counter`; a generic one as `Foo`), + and the other per-component events agree on it. **Listeners now also receive + errors that were previously silent:** a component (class, function or memo) + throwing on its first render, a `ReactorHost` / `ReactorHostControl` root + component or render function throwing (in `Render()` or in its effects), and any + render error an `ErrorBoundary` + catches — reported once, at the throw site, even though the app shows the + fallback. Same event, same payload; the message stays redacted. Generic + component names change from ``Foo`1`` to `Foo` in every component event. + For these component failures the event is emitted before any `RenderErrorHandler` + runs, so it fires whether the handler replaces the fallback or calls `Propagate()` — + once per throw either way. `Reconcile` and `Cleanup` failures are not render errors + and are not reported on it. + - **DataGrid inline editors no longer shift the cell's text when editing starts** (issue #1340). The built-in TextBox and NumberBox editors used `.Padding(2)` inside WinUI's 32px minimum height, which draws text at the top, so the text moved about 4px diff --git a/docs/_pipeline/diagrams/source-mapping/attribution.svg b/docs/_pipeline/diagrams/source-mapping/attribution.svg index 3f105c3a8..1d89bf691 100644 --- a/docs/_pipeline/diagrams/source-mapping/attribution.svg +++ b/docs/_pipeline/diagrams/source-mapping/attribution.svg @@ -40,7 +40,7 @@ class MyView : Component { … } - Component.GetType().Name + ComponentNames.For(component) "MyView" diff --git a/docs/_pipeline/templates/advanced.md.dt b/docs/_pipeline/templates/advanced.md.dt index c9699d27b..31c48373d 100644 --- a/docs/_pipeline/templates/advanced.md.dt +++ b/docs/_pipeline/templates/advanced.md.dt @@ -124,6 +124,16 @@ failing child, once per host render for a host-level failure. De-duplicate side effects such as telemetry, for example by remembering the exceptions you have already recorded. +Independently of the handler, every component failure — a `Render()` or an +effect flush that throws, at any depth including the host's root — is also +reported on the `Microsoft-UI-Reactor` ETW `RenderError` event (`Errors` +keyword), naming the component — see [diagnostics](diagnostics.md). The event is +emitted before the handler runs, so it fires once per throw whatever the handler +does, including when it calls `Propagate()`. Errors an `ErrorBoundary` catches +are reported there too, even though they never reach the handler. `Reconcile` +and `Cleanup` failures have no failing render and are not reported on that +event. + `Cleanup` reports an effect cleanup that threw while a host was being disposed — a window closing, or a `ReactorHostControl` being disposed. There is nothing left to display, so the return value is ignored and the handler only diff --git a/docs/_pipeline/templates/source-mapping.md.dt b/docs/_pipeline/templates/source-mapping.md.dt index 5aa06b26a..ed7ad8ddb 100644 --- a/docs/_pipeline/templates/source-mapping.md.dt +++ b/docs/_pipeline/templates/source-mapping.md.dt @@ -49,15 +49,20 @@ consumers can subscribe to just the render channel: ```csharp snippet="source:src/Reactor/Core/Diagnostics/ReactorEventSource.cs#etw-keywords" ``` -`ComponentRenderStart` / `ComponentRenderStop` fire with -`componentName = node.Component?.GetType().Name`. That string is the +`ComponentRenderStart` / `ComponentRenderStop` fire with the component's +type name — `Counter` for a `Counter : Component`, whichever +factory created it. That string is the attribution token that flows into PerfView / `dotnet-trace` / `xperf`, and it is the same string [`devtools-internals`](devtools-internals.md) uses to label overlay frames. Per-component, not per-element — but sufficient for the common question "which component is re-rendering -on every tick". +on every tick". `RenderError`, `EffectsFlushStart` / `Stop` and +`ComponentUnmount` use the same name. A generic component reports its type +arguments in C# form (`ItemList`, not ``ItemList`1``); function and memo +components, which have no type of their own, report `FuncElement` and +`MemoElement`. -![Source attribution: component-name attribution flows Component.GetType().Name into ETW, while per-element attribution stamps each DSL call site with file and line via the interceptor generator, readable from any realized control through ReactorSourceMap.GetSource.](images/source-mapping/attribution.svg) +![Source attribution: component-name attribution flows the component's type name, formatted by ComponentNames (generic components in C# form), into ETW, while per-element attribution stamps each DSL call site with file and line via the interceptor generator, readable from any realized control through ReactorSourceMap.GetSource.](images/source-mapping/attribution.svg) ## Reconcile-pass attribution @@ -67,7 +72,7 @@ on every tick". | `ReconcileStart` / `Stop` | Root element type + diff counters | ETW `Reconcile` keyword | | `EffectsFlushStart` / `Stop` | Component CLR type name | ETW `Render` keyword | | `StateChange` | Hook kind + value type | ETW `State` keyword | -| `RenderError` | Component name + exception type only (message redacted) | ETW `Errors` keyword | +| `RenderError` | Component name + exception type only (message redacted); emitted when a component's `Render()` or effect flush throws — on mount, update and at the host root — including errors an `ErrorBoundary` catches (named at the throw site, once) | ETW `Errors` keyword | | Per-element file:line | Element call site | `Element.CallSite` / `ReactorSourceMap.GetSource` (when source mapping is enabled at build time) | The reconcile pass also emits a counter summary on stop: diff --git a/docs/guide/advanced.md b/docs/guide/advanced.md index c9b6376bb..0ba8bbb5c 100644 --- a/docs/guide/advanced.md +++ b/docs/guide/advanced.md @@ -204,6 +204,16 @@ failing child, once per host render for a host-level failure. De-duplicate side effects such as telemetry, for example by remembering the exceptions you have already recorded. +Independently of the handler, every component failure — a `Render()` or an +effect flush that throws, at any depth including the host's root — is also +reported on the `Microsoft-UI-Reactor` ETW `RenderError` event (`Errors` +keyword), naming the component — see [diagnostics](diagnostics.md). The event is +emitted before the handler runs, so it fires once per throw whatever the handler +does, including when it calls `Propagate()`. Errors an `ErrorBoundary` catches +are reported there too, even though they never reach the handler. `Reconcile` +and `Cleanup` failures have no failing render and are not reported on that +event. + `Cleanup` reports an effect cleanup that threw while a host was being disposed — a window closing, or a `ReactorHostControl` being disposed. There is nothing left to display, so the return value is ignored and the handler only diff --git a/docs/guide/images/source-mapping/attribution.svg b/docs/guide/images/source-mapping/attribution.svg index 3f105c3a8..1d89bf691 100644 --- a/docs/guide/images/source-mapping/attribution.svg +++ b/docs/guide/images/source-mapping/attribution.svg @@ -40,7 +40,7 @@ class MyView : Component { … } - Component.GetType().Name + ComponentNames.For(component) "MyView" diff --git a/docs/guide/source-mapping.md b/docs/guide/source-mapping.md index 45f858b22..6d710cc87 100644 --- a/docs/guide/source-mapping.md +++ b/docs/guide/source-mapping.md @@ -56,15 +56,20 @@ public static class Keywords } ``` -`ComponentRenderStart` / `ComponentRenderStop` fire with -`componentName = node.Component?.GetType().Name`. That string is the +`ComponentRenderStart` / `ComponentRenderStop` fire with the component's +type name — `Counter` for a `Counter : Component`, whichever +factory created it. That string is the attribution token that flows into PerfView / `dotnet-trace` / `xperf`, and it is the same string [`devtools-internals`](devtools-internals.md) uses to label overlay frames. Per-component, not per-element — but sufficient for the common question "which component is re-rendering -on every tick". +on every tick". `RenderError`, `EffectsFlushStart` / `Stop` and +`ComponentUnmount` use the same name. A generic component reports its type +arguments in C# form (`ItemList`, not ``ItemList`1``); function and memo +components, which have no type of their own, report `FuncElement` and +`MemoElement`. -![Source attribution: component-name attribution flows Component.GetType().Name into ETW, while per-element attribution stamps each DSL call site with file and line via the interceptor generator, readable from any realized control through ReactorSourceMap.GetSource.](images/source-mapping/attribution.svg) +![Source attribution: component-name attribution flows the component's type name, formatted by ComponentNames (generic components in C# form), into ETW, while per-element attribution stamps each DSL call site with file and line via the interceptor generator, readable from any realized control through ReactorSourceMap.GetSource.](images/source-mapping/attribution.svg) ## Reconcile-pass attribution @@ -74,7 +79,7 @@ on every tick". | `ReconcileStart` / `Stop` | Root element type + diff counters | ETW `Reconcile` keyword | | `EffectsFlushStart` / `Stop` | Component CLR type name | ETW `Render` keyword | | `StateChange` | Hook kind + value type | ETW `State` keyword | -| `RenderError` | Component name + exception type only (message redacted) | ETW `Errors` keyword | +| `RenderError` | Component name + exception type only (message redacted); emitted when a component's `Render()` or effect flush throws — on mount, update and at the host root — including errors an `ErrorBoundary` catches (named at the throw site, once) | ETW `Errors` keyword | | Per-element file:line | Element call site | `Element.CallSite` / `ReactorSourceMap.GetSource` (when source mapping is enabled at build time) | The reconcile pass also emits a counter summary on stop: diff --git a/docs/specs/044/swallowed-error-audit.md b/docs/specs/044/swallowed-error-audit.md index fa6e33b9d..76610db8b 100644 --- a/docs/specs/044/swallowed-error-audit.md +++ b/docs/specs/044/swallowed-error-audit.md @@ -103,12 +103,12 @@ row gains a column and the gate rejects it. | Verdict | Sites | Shipped | Deferred | |---|---|---|---| -| `Keep` | 24 | 24 | 0 | +| `Keep` | 34 | 34 | 0 | | `Narrow` | 38 | 38 | 0 | | `Propagate` | 7 | 7 | 0 | | `TryFinally` | 7 | 7 | 0 | | `TryXxx` | 10 | 0 | 10 | -| `PromoteEvent` | 26 | 18 | 8 | +| `PromoteEvent` | 30 | 22 | 8 | | `Deleted` | 7 | 7 | 0 | | `Trace` | 5 | 5 | 0 | @@ -253,7 +253,7 @@ file that exists, unless the heading carries a `(retired …)` marker. | `EndOfStreamException` reject | 1 | `PromoteEvent` | shipped | Same. | | Outer catches — `Capture`, `Restore` | 2 | `Narrow` | shipped | Narrowed to `IOException`. One clause each on the capture and restore paths; the original entry said "Outer catches" without a count. | -### `src/Reactor/Core/Reconciler.cs` — Phase C.7b (commit `054c53ef`) + Phase C.8 (commit `21cd6ef9`) +### `src/Reactor/Core/Reconciler.cs` — Phase C.7b (commit `054c53ef`) + Phase C.8 (commit `21cd6ef9`) + render-error reporting (issue #1321) | Site(s) | Sites | Verdict | Status | Notes | |---|---|---|---|---| @@ -263,12 +263,16 @@ file that exists, unless the heading carries a `(retired …)` marker. | ConnectedAnimation `GetAnimation` | 1 | `Keep` | shipped | **fail-safe-to-default.** Same. | | ConnectedAnimation `TryStart` | 1 | `Keep` | shipped | **fail-safe-to-default.** Same. | | `ApplyThemeBindings` | 1 | `Keep` | shipped | **fail-safe-to-default.** LogCategory.Theme — the catch wraps a XAML `Style.Load` compile. Could narrow to `XamlParseException` in a follow-up. | +| `ReconcileComponent` render catch outside any `ErrorBoundary` (`_errorBoundaryDepth == 0`) | 1 | `Keep` | shipped | **user-callback isolation** per §6.7.3 — the `try` wraps the app's `Render()` and the effect flush, so the failure class is arbitrary app code and cannot be narrowed. The render is replaced by `ErrorFallback`, and the failure is reported through `ILogger` and the typed `RenderError` event, naming the component (issue #1321, PR #1323), then handed to the app's `RenderErrorHandler` when one is set (issue #1291). `OutOfMemoryException` / `StackOverflowException` are filtered out and propagate (after the report-and-rethrow arm has reported them). Pre-dates the ledger; adjudicated when #1321 touched it. | +| `ReconcileComponent` report-and-rethrow arm — inside an `ErrorBoundary` (`_errorBoundaryDepth > 0`), or `OutOfMemoryException` / `StackOverflowException` anywhere | 1 | `PromoteEvent` | shipped | Report-and-rethrow, **not a swallow**: emits `RenderError` naming the component that threw — the only frame that knows it — then `throw;` rethrows unchanged to the enclosing boundary, which renders its fallback and does not report again (one event per throw). No exception-type filter — not even the `OutOfMemoryException` / `StackOverflowException` carve-out — because the boundary's own `catch` takes every exception, so every exception it recovers from is reported. Outside a boundary the arm also takes OOM/SO, which skip the fallback arm and reach the host's outer `catch` — itself a recovery — so they are reported once here too. Like every Reactor catch site since issue #1291, it lets an exception the app declined via `RenderError.Propagate()` pass (`!RenderErrorDispatch.IsPropagating(ex)`): its throw site already reported it. Added by issue #1321 / PR #1323. | -### `src/Reactor/Core/Reconciler.Mount.cs` — Phase C.8 (commit `21cd6ef9`) +### `src/Reactor/Core/Reconciler.Mount.cs` — Phase C.8 (commit `21cd6ef9`) + render-error reporting (issue #1321) | Site(s) | Sites | Verdict | Status | Notes | |---|---|---|---|---| | `ContentDialog.ShowAsync + OnClosed` | 1 | `Keep` | shipped | **user-callback isolation** per §6.7.3 — the try wraps both `ShowAsync` AND the user-supplied `OnClosed` delegate. Cannot narrow without splitting the try-catch into two; deferred. | +| First-render catches outside any `ErrorBoundary` (`_errorBoundaryDepth == 0`) — `MountComponent`, `MountFuncComponent`, `MountMemoComponent` | 3 | `Keep` | shipped | **user-callback isolation** per §6.7.3 — each `try` wraps the app's first `Render()` (class, function or memo component) and its effect flush; arbitrary app code, so no narrowing. The render is replaced by `ErrorFallback` and reported through `ILogger` and, since issue #1321 (PR #1323), the typed `RenderError` event — these arms previously only logged — then handed to the app's `RenderErrorHandler` when one is set (issue #1291). `OutOfMemoryException` / `StackOverflowException` propagate (after the report-and-rethrow arm has reported them). Pre-date the ledger; adjudicated when #1321 touched them. | +| First-render report-and-rethrow arms — inside an `ErrorBoundary` (`_errorBoundaryDepth > 0`), or `OutOfMemoryException` / `StackOverflowException` anywhere — `MountComponent`, `MountFuncComponent`, `MountMemoComponent` | 3 | `PromoteEvent` | shipped | Report-and-rethrow, **not a swallow**: emits `RenderError` naming the component that threw, then `throw;` rethrows unchanged to the enclosing boundary, which does not report again (one event per throw). Same filter as the update-path arm: every exception inside a boundary, plus OOM/SO outside one (the host recovers from those too), minus a declined propagation. Added by issue #1321 / PR #1323. | ### `src/Reactor/Core/RenderContext.cs` — Phase C.6 (commit `90d516b0`) + Phase C.9 narrowing @@ -307,6 +311,20 @@ types without making it a narrowing. Same shape as the existing convention at | `CanResolvePageType` resolver probe | 1 | `Keep` | shipped | **fail-safe-to-default.** The method's contract is "true **only if** definitively resolvable". Any failure to answer means we cannot confirm, and returning `false` refuses the navigation — the safe direction, and the one that cannot produce the access violation. Expected types are `COMException` at the WinRT boundary and `InvalidOperationException` / `ArgumentException` from a generated or hand-written `IXamlMetadataProvider`; propagating anything else would convert a third-party provider's bug into a render-loop error **while the navigation is refused either way**, i.e. strictly worse for identical safety. `OutOfMemoryException` / `StackOverflowException` still propagate. | | `TryNavigate` around `Frame.Navigate` | 1 | `Keep` | shipped | **user-callback isolation** (§6.7.3). What surfaces here is the **page constructor's** exception — arbitrary application code — and routing it into the element's declared `OnNavigationFailed` channel is the arm's entire purpose. Directly analogous to the `ContentDialog.ShowAsync + OnClosed` entry above. **Narrowing would reintroduce the defect this fix exists to remove:** an unanticipated page-constructor failure would escape the mount pass. **Coverage gap, stated rather than hidden:** reaching this arm needs a target that *resolves* but whose constructor then throws, i.e. a real `.xaml`-backed page. The selftest host ships no XAML, so every target it can offer is either refused before `Navigate` (code-only) or a framework type that constructs fine. The arm is therefore reasoned-about, not exercised — unlike the refusal path, which `FrameNav_CodeOnlyPageRefusedNotFatal` pins directly. | +### `src/Reactor/Hosting/ReactorHost.cs` — render-error reporting (issue #1321) + +| Site(s) | Sites | Verdict | Status | Notes | +|---|---|---|---|---| +| Root render catches in `Render` — root component `Render()`, root render function | 2 | `Keep` | shipped | **user-callback isolation** per §6.7.3 — each `try` wraps the app's root render, arbitrary app code. The host shows its error fallback — or, since issue #1291, whatever the app's `RenderErrorHandler` returns — and keeps running; the failure is reported through `ILogger` and, since issue #1321 (PR #1323), the typed `RenderError` event — previously these arms only logged. The catch lets an exception the app declined via `RenderError.Propagate()` pass (`!RenderErrorDispatch.IsPropagating(ex)`, issue #1291). A `HookOrderException` during a hot-reload render is handled by the preceding, narrower clause. Pre-date the ledger; adjudicated when #1321 touched them. | +| Outer render-loop catch in `Render` ("Render FAILED") — reconcile, content install, root effect flush, post-render callbacks | 1 | `Keep` | shipped | **user-callback isolation** per §6.7.3 — the `try` spans the reconcile pass (child renders and effects), the root's effect flush and app callbacks such as `OnRenderComplete`, all of which run app code. The host shows its fallback (or the app's `RenderErrorHandler` result, issue #1291) and keeps running; an exception the app declined via `RenderError.Propagate()` passes (`!RenderErrorDispatch.IsPropagating(ex)`). When the failing phase is the root's effect flush, the root component owns the failure, so since issue #1321 (PR #1323) it is also reported on the typed `RenderError` event, like a child's effect-flush failure; other phases have no failing component and are not. Pre-dates the ledger; adjudicated when #1321 touched it. | + +### `src/Reactor/Hosting/ReactorHostControl.cs` — render-error reporting (issue #1321) + +| Site(s) | Sites | Verdict | Status | Notes | +|---|---|---|---|---| +| Root render catches in `Render` — root component `Render()`, root render function | 2 | `Keep` | shipped | **user-callback isolation** per §6.7.3. Same shape and reporting as `ReactorHost.cs` above. | +| Outer render-loop catch in `Render` ("Render FAILED") | 1 | `Keep` | shipped | **user-callback isolation** per §6.7.3. Same shape and reporting as `ReactorHost.cs` above. | + ### `src/Reactor/Hosting/ReactorWindow.cs` — Phase C.8 (commit `21cd6ef9`) + Phase C.9 narrowing Phase C.8 migrated the file's `Debug.WriteLine` calls to `DiagnosticLog` with diff --git a/src/Reactor/Core/Diagnostics/ComponentNames.cs b/src/Reactor/Core/Diagnostics/ComponentNames.cs new file mode 100644 index 000000000..51ad82b30 --- /dev/null +++ b/src/Reactor/Core/Diagnostics/ComponentNames.cs @@ -0,0 +1,91 @@ +namespace Microsoft.UI.Reactor.Core.Diagnostics; + +/// +/// The component name Reactor's diagnostics report (ReactorEventSource +/// RenderError, ComponentRenderStart/Stop, +/// EffectsFlushStart/Stop, ComponentUnmount, and the +/// reconciler's ILogger messages). +/// +/// Why not element.GetType().Name. A class component is carried by a +/// — and by ComponentElement<TProps> for +/// Component<T, TProps>(…) — so naming the element reports +/// ComponentElement / ComponentElement`1 for every class component in +/// the app. The component is (or the live +/// instance's type, which wins after a hot-reload type swap). +/// +/// Shape. The type's own simple name, as before, for a non-generic +/// component (Counter), so existing traces and greps are unchanged. A generic +/// component renders its arity in C# form (Counter<Int32>) instead of the +/// metadata name (Counter`1). Function and memo components have no type of their +/// own and keep reporting FuncElement / MemoElement. +/// +/// PII. Output is built only from developer-authored type names — never +/// props, state, keys or paths — so it is safe on the ETW payload +/// (spec 044 §6.2.1). +/// +/// Reflection-light (Type.Name and Type.GetGenericArguments()), +/// so it is trim- and AOT-safe. +/// +internal static class ComponentNames +{ + /// + /// Name for a component node: the live 's type when there + /// is one, else the component type an element declares, else the element's own type + /// (FuncElement, MemoElement). + /// + internal static string For(Component? instance, Element? element) + { + if (instance is not null) return For(instance.GetType()); + if (element is ComponentElement componentElement) return For(componentElement.ComponentType); + return element is null ? "unknown" : For(element.GetType()); + } + + /// Display name of a component type; see the class remarks for the shape. + internal static string For(Type type) + { + if (!type.IsGenericType) return type.Name; + + // Generic names are built, so cache them: the name is asked for on every traced + // render and unmount. A weak table, so it never roots a collectible or hot-reloaded type. + return s_genericNames.GetValue(type, static t => + { + var sb = new global::System.Text.StringBuilder(); + Append(sb, t); + return sb.ToString(); + }); + } + + private static readonly global::System.Runtime.CompilerServices.ConditionalWeakTable s_genericNames = new(); + + private static void Append(global::System.Text.StringBuilder sb, Type type) + { + // An array of a constructed generic is not itself generic, but its metadata name + // still carries the arity (List`1[]): format the element type, then the rank. + if (type.IsArray) + { + Append(sb, type.GetElementType()!); + sb.Append('[').Append(',', type.GetArrayRank() - 1).Append(']'); + return; + } + + var name = type.Name; + var tick = name.IndexOf('`'); + sb.Append(tick >= 0 ? name.Substring(0, tick) : name); + + if (!type.IsGenericType) return; + + // For a nested generic the flat argument list also carries the declaring type's + // arguments; this name only reports the ones it introduces itself. + var args = type.GetGenericArguments(); + int own = tick >= 0 && int.TryParse(name.AsSpan(tick + 1), out var arity) ? arity : 0; + if (own == 0) return; + + sb.Append('<'); + for (int i = args.Length - own; i < args.Length; i++) + { + if (i > args.Length - own) sb.Append(", "); + Append(sb, args[i]); + } + sb.Append('>'); + } +} diff --git a/src/Reactor/Core/Reconciler.Mount.cs b/src/Reactor/Core/Reconciler.Mount.cs index e74fbe1d8..72c2953dc 100644 --- a/src/Reactor/Core/Reconciler.Mount.cs +++ b/src/Reactor/Core/Reconciler.Mount.cs @@ -894,9 +894,22 @@ private UIElement MountComponent(ComponentElement compElement, Action requestRer inEffects = true; component.Context.FlushEffects(); } + // Inside an ErrorBoundary — or a fatal exception the fallback arm below won't take: + // name the failing component, then rethrow unchanged (see ReconcileComponent). An + // exception the app declined via RenderError.Propagate() was already reported where + // it was thrown, so it passes through unreported (issue #1291). + catch (Exception ex) when ((_errorBoundaryDepth > 0 || ex is OutOfMemoryException or StackOverflowException) + && !RenderErrorDispatch.IsPropagating(ex)) + { + EmitRenderError(Diagnostics.ComponentNames.For(component, compElement), ex); + throw; + } catch (Exception ex) when (_errorBoundaryDepth == 0 && ex is not OutOfMemoryException and not StackOverflowException && !RenderErrorDispatch.IsPropagating(ex)) { - _logger?.LogError(ex, "Component Render() threw during mount: {ComponentName}", compElement.GetType().Name); + var failedName = Diagnostics.ComponentNames.For(component, compElement); + _logger?.LogError(ex, "Component Render() threw during mount: {ComponentName}", failedName); + // Before the fallback: the app's handler may call Propagate(), which throws. + EmitRenderError(failedName, ex); childElement = BuildInTreeFallback(ex, inEffects, component.GetType().Name); } UIElement? childControl = Mount(childElement, componentRerender); @@ -936,9 +949,21 @@ private UIElement MountFuncComponent(FuncElement funcElement, Action requestRere inEffects = true; ctx.FlushEffects(); } + // Inside an ErrorBoundary — or a fatal exception the fallback arm below won't take: + // name the failing component, then rethrow unchanged (see ReconcileComponent). An + // exception the app declined via RenderError.Propagate() was already reported where + // it was thrown, so it passes through unreported (issue #1291). + catch (Exception ex) when ((_errorBoundaryDepth > 0 || ex is OutOfMemoryException or StackOverflowException) + && !RenderErrorDispatch.IsPropagating(ex)) + { + EmitRenderError(Diagnostics.ComponentNames.For(null, funcElement), ex); + throw; + } catch (Exception ex) when (_errorBoundaryDepth == 0 && ex is not OutOfMemoryException and not StackOverflowException && !RenderErrorDispatch.IsPropagating(ex)) { _logger?.LogError(ex, "FuncComponent Render() threw during mount"); + // Before the fallback: the app's handler may call Propagate(), which throws. + EmitRenderError(Diagnostics.ComponentNames.For(null, funcElement), ex); childElement = BuildInTreeFallback(ex, inEffects, componentName: null); } UIElement? childControl = Mount(childElement, componentRerender); @@ -979,9 +1004,21 @@ private UIElement MountMemoComponent(MemoElement memoElement, Action requestRere inEffects = true; ctx.FlushEffects(); } + // Inside an ErrorBoundary — or a fatal exception the fallback arm below won't take: + // name the failing component, then rethrow unchanged (see ReconcileComponent). An + // exception the app declined via RenderError.Propagate() was already reported where + // it was thrown, so it passes through unreported (issue #1291). + catch (Exception ex) when ((_errorBoundaryDepth > 0 || ex is OutOfMemoryException or StackOverflowException) + && !RenderErrorDispatch.IsPropagating(ex)) + { + EmitRenderError(Diagnostics.ComponentNames.For(null, memoElement), ex); + throw; + } catch (Exception ex) when (_errorBoundaryDepth == 0 && ex is not OutOfMemoryException and not StackOverflowException && !RenderErrorDispatch.IsPropagating(ex)) { _logger?.LogError(ex, "MemoComponent Render() threw during mount"); + // Before the fallback: the app's handler may call Propagate(), which throws. + EmitRenderError(Diagnostics.ComponentNames.For(null, memoElement), ex); childElement = BuildInTreeFallback(ex, inEffects, componentName: null); } UIElement? childControl = Mount(childElement, componentRerender); diff --git a/src/Reactor/Core/Reconciler.cs b/src/Reactor/Core/Reconciler.cs index f64b9bfad..18ce51581 100644 --- a/src/Reactor/Core/Reconciler.cs +++ b/src/Reactor/Core/Reconciler.cs @@ -192,8 +192,13 @@ void OnCleanupError(Exception ex) _errorBoundaryNodes.Remove(wrapper); if (_componentNodes.Remove(wrapper, out var node)) { - Diagnostics.ReactorEventSource.Log.ComponentUnmount( - node.Component?.GetType().Name ?? node.Element?.GetType().Name ?? "unknown"); + if (Diagnostics.ReactorEventSource.Log.IsEnabled( + global::System.Diagnostics.Tracing.EventLevel.Informational, + Diagnostics.ReactorEventSource.Keywords.Lifecycle)) + { + Diagnostics.ReactorEventSource.Log.ComponentUnmount( + Diagnostics.ComponentNames.For(node.Component, node.Element)); + } RunUnmountCleanups(node); } if (wrapper is FrameworkElement fe && _onUnmountActions.TryGetValue(fe, out var onUnmount)) @@ -2162,6 +2167,25 @@ private void PopulateDirtyAncestorPath() // without a matching decrement, which silently suppresses all later spans. internal int ReconcileTraceDepthForTests => _reconcileTraceDepth; + /// + /// ReactorEventSource.RenderError for a component whose Render() threw and was + /// replaced by the error fallback. Shared by the mount and update paths so both name + /// the component the same way (). Only the + /// component name and exception type are written: the sink redacts the message + /// (TASK-064), so 's virtual Message is never read here — + /// an app exception whose Message override throws must not replace the original + /// exception on this error path (the ErrorBoundary arms rethrow it unchanged). + /// + internal static void EmitRenderError(string componentName, Exception ex) + { + if (Diagnostics.ReactorEventSource.Log.IsEnabled( + global::System.Diagnostics.Tracing.EventLevel.Error, + Diagnostics.ReactorEventSource.Keywords.Errors)) + { + Diagnostics.ReactorEventSource.Log.RenderError(componentName, ex.GetType().Name, string.Empty); + } + } + private static void FlushEffectsTraced(RenderContext ctx, string? componentName) { // Fast path when the Render keyword is off: no Stopwatch, no event emit. @@ -2376,7 +2400,7 @@ private void ReconcileComponent(Element oldEl, Element newEl, UIElement control, long renderStart = 0; if (traceRender) { - componentName = node.Component?.GetType().Name ?? newEl.GetType().Name; + componentName = Diagnostics.ComponentNames.For(node.Component, newEl); Diagnostics.ReactorEventSource.Log.ComponentRenderStart( componentName, selfTriggered ? "self" : "parent"); renderStart = global::System.Diagnostics.Stopwatch.GetTimestamp(); @@ -2451,21 +2475,32 @@ private void ReconcileComponent(Element oldEl, Element newEl, UIElement control, _logger?.LogWarning(ex, "Hot reload: hook order/type changed in child component — " + "resetting state and re-rendering: {ComponentName}", - componentName ?? newEl.GetType().Name); + componentName ?? Diagnostics.ComponentNames.For(node.Component, newEl)); hotReloadRetried = true; renderCtx.ResetForHotReload(); continue; } + // Inside an ErrorBoundary the exception propagates to the boundary, which + // renders its fallback. Name the failing component here, at the throw site — + // the boundary itself cannot know which descendant threw — so a listener sees + // the error even though the app recovered. Rethrown unchanged. No OOM/SO carve-out: + // the boundary catches every exception, so every one it recovers from is reported. + // Outside a boundary, OOM/SO skip the fallback arm below and propagate to the + // host, which also recovers — so they are reported here too. An exception the app + // declined via RenderError.Propagate() was already reported where it was thrown, + // so it passes through unreported (issue #1291). + catch (Exception ex) when ((_errorBoundaryDepth > 0 || ex is OutOfMemoryException or StackOverflowException) + && !RenderErrorDispatch.IsPropagating(ex)) + { + EmitRenderError(componentName ?? Diagnostics.ComponentNames.For(node.Component, newEl), ex); + throw; + } catch (Exception ex) when (_errorBoundaryDepth == 0 && ex is not OutOfMemoryException and not StackOverflowException && !RenderErrorDispatch.IsPropagating(ex)) { - _logger?.LogError(ex, "Component Render() threw: {ComponentName}", newEl.GetType().Name); - if (Diagnostics.ReactorEventSource.Log.IsEnabled( - global::System.Diagnostics.Tracing.EventLevel.Error, - Diagnostics.ReactorEventSource.Keywords.Errors)) - { - Diagnostics.ReactorEventSource.Log.RenderError( - componentName ?? newEl.GetType().Name, ex.GetType().Name, ex.Message); - } + var failedName = componentName ?? Diagnostics.ComponentNames.For(node.Component, newEl); + _logger?.LogError(ex, "Component Render() threw: {ComponentName}", failedName); + // Before the fallback: the app's handler may call Propagate(), which throws. + EmitRenderError(failedName, ex); newChildElement = BuildInTreeFallback(ex, inEffects, node.Component?.GetType().Name); } break; @@ -2767,8 +2802,13 @@ private void UnmountRecursive(UIElement control) if (_componentNodes.TryGetValue(control, out var node)) { - Diagnostics.ReactorEventSource.Log.ComponentUnmount( - node.Component?.GetType().Name ?? node.Element?.GetType().Name ?? "unknown"); + if (Diagnostics.ReactorEventSource.Log.IsEnabled( + global::System.Diagnostics.Tracing.EventLevel.Informational, + Diagnostics.ReactorEventSource.Keywords.Lifecycle)) + { + Diagnostics.ReactorEventSource.Log.ComponentUnmount( + Diagnostics.ComponentNames.For(node.Component, node.Element)); + } RunUnmountCleanups(node); _componentNodes.Remove(control); } @@ -3180,8 +3220,13 @@ private void UnmountAndCollect(UIElement control, List toPool) // Run cleanup logic (component teardown, etc.) if (_componentNodes.TryGetValue(control, out var node)) { - Diagnostics.ReactorEventSource.Log.ComponentUnmount( - node.Component?.GetType().Name ?? node.Element?.GetType().Name ?? "unknown"); + if (Diagnostics.ReactorEventSource.Log.IsEnabled( + global::System.Diagnostics.Tracing.EventLevel.Informational, + Diagnostics.ReactorEventSource.Keywords.Lifecycle)) + { + Diagnostics.ReactorEventSource.Log.ComponentUnmount( + Diagnostics.ComponentNames.For(node.Component, node.Element)); + } RunUnmountCleanups(node); _componentNodes.Remove(control); } diff --git a/src/Reactor/Hosting/ReactorHost.cs b/src/Reactor/Hosting/ReactorHost.cs index b4301832b..3ca783908 100644 --- a/src/Reactor/Hosting/ReactorHost.cs +++ b/src/Reactor/Hosting/ReactorHost.cs @@ -620,6 +620,9 @@ void RecoverFromHookOrder(HookOrderException ex, RenderContext ctx, string mode) // Which phase the outer catch attributes a failure to (issue #1291). var failurePhase = RenderErrorSource.Reconcile; + // The root whose effects are being flushed, captured before app code runs: an effect + // can Mount() a replacement root and then throw, and the failure is the original's. + Component? effectsRoot = null; try { Element? newTree = null; @@ -643,26 +646,31 @@ void RecoverFromHookOrder(HookOrderException ex, RenderContext ctx, string mode) // instead of allocating `() => RequestRender()` every render. Action rerender = _rerenderAction ??= () => RequestRender(); - if (_rootComponent is not null) + // Captured before app code runs: the root's Render() can Mount() a replacement + // root and then throw, and the failure belongs to the component that threw. + if (_rootComponent is { } renderingRoot) { - _rootComponent.Context.BeginRender(rerender); + renderingRoot.Context.BeginRender(rerender); try { using (ValidationRenderScope.Begin(null)) { - newTree = ValidationRenderScope.ApplyProvide(_rootComponent.Render()); + newTree = ValidationRenderScope.ApplyProvide(renderingRoot.Render()); } } catch (HookOrderException ex) when (hotReloadRender) { - RecoverFromHookOrder(ex, _rootComponent.Context, "component"); + RecoverFromHookOrder(ex, renderingRoot.Context, "component"); return; } catch (Exception ex) when (!RenderErrorDispatch.IsPropagating(ex)) { Debugger.BreakForUserUnhandledException(ex); _logger?.LogError(ex, "Component Render() threw"); - ShowErrorFallback(ex, RenderErrorSource.RootRender, _rootComponent.GetType().Name); + // Before the fallback: the app's handler may call Propagate(), which throws. + Reconciler.EmitRenderError( + Microsoft.UI.Reactor.Core.Diagnostics.ComponentNames.For(renderingRoot, element: null), ex); + ShowErrorFallback(ex, RenderErrorSource.RootRender, renderingRoot.GetType().Name); return; } } @@ -684,6 +692,8 @@ void RecoverFromHookOrder(HookOrderException ex, RenderContext ctx, string mode) catch (Exception ex) when (!RenderErrorDispatch.IsPropagating(ex)) { _logger?.LogError(ex, "Function component threw"); + // Before the fallback: the app's handler may call Propagate(), which throws. + Reconciler.EmitRenderError(nameof(FuncElement), ex); ShowErrorFallback(ex, RenderErrorSource.RootRender); return; } @@ -803,8 +813,9 @@ void RecoverFromHookOrder(HookOrderException ex, RenderContext ctx, string mode) _phaseSw.Restart(); failurePhase = RenderErrorSource.Effects; - if (_rootComponent is not null) - _rootComponent.Context.FlushEffects(); + effectsRoot = _rootComponent; + if (effectsRoot is not null) + effectsRoot.Context.FlushEffects(); else if (_funcContext is not null) _funcContext.FlushEffects(); failurePhase = RenderErrorSource.Reconcile; @@ -877,10 +888,20 @@ void RecoverFromHookOrder(HookOrderException ex, RenderContext ctx, string mode) catch (Exception ex) when (!RenderErrorDispatch.IsPropagating(ex)) { _logger?.LogError(ex, "Render FAILED"); - // A root effect failure belongs to the root component; a commit-phase one has no - // single owning component. + // A root effect failure belongs to the root component and is reported like a + // child's effect-flush failure (issue #1321) — before the fallback, because the + // app's handler may call Propagate(), which throws. A commit-phase failure has no + // single owning component and is not a RenderError. + if (failurePhase == RenderErrorSource.Effects) + { + Reconciler.EmitRenderError( + effectsRoot is not null + ? Microsoft.UI.Reactor.Core.Diagnostics.ComponentNames.For(effectsRoot, element: null) + : nameof(FuncElement), + ex); + } ShowErrorFallback(ex, failurePhase, - failurePhase == RenderErrorSource.Effects ? _rootComponent?.GetType().Name : null); + failurePhase == RenderErrorSource.Effects ? effectsRoot?.GetType().Name : null); } finally { diff --git a/src/Reactor/Hosting/ReactorHostControl.cs b/src/Reactor/Hosting/ReactorHostControl.cs index ade54185d..109f4b89e 100644 --- a/src/Reactor/Hosting/ReactorHostControl.cs +++ b/src/Reactor/Hosting/ReactorHostControl.cs @@ -603,31 +603,39 @@ void RecoverFromHookOrder(HookOrderException ex, RenderContext ctx, string mode) // Which phase the outer catch attributes a failure to (issue #1291). var failurePhase = RenderErrorSource.Reconcile; + // The root whose effects are being flushed, captured before app code runs: an effect + // can Mount() a replacement root and then throw, and the failure is the original's. + Component? effectsRoot = null; try { Element? newTree = null; _phaseSw.Restart(); - if (_rootComponent is not null) + // Captured before app code runs: the root's Render() can Mount() a replacement + // root and then throw, and the failure belongs to the component that threw. + if (_rootComponent is { } renderingRoot) { - _rootComponent.Context.BeginRender(_requestRenderAction ??= RequestRender); + renderingRoot.Context.BeginRender(_requestRenderAction ??= RequestRender); try { using (ValidationRenderScope.Begin(null)) { - newTree = ValidationRenderScope.ApplyProvide(_rootComponent.Render()); + newTree = ValidationRenderScope.ApplyProvide(renderingRoot.Render()); } } catch (HookOrderException ex) when (hotReloadRender) { - RecoverFromHookOrder(ex, _rootComponent.Context, "component"); + RecoverFromHookOrder(ex, renderingRoot.Context, "component"); return; } catch (Exception ex) when (!RenderErrorDispatch.IsPropagating(ex)) { _logger?.LogError(ex, "Component Render() threw"); - ShowErrorFallback(ex, RenderErrorSource.RootRender, _rootComponent.GetType().Name); + // Before the fallback: the app's handler may call Propagate(), which throws. + Reconciler.EmitRenderError( + Microsoft.UI.Reactor.Core.Diagnostics.ComponentNames.For(renderingRoot, element: null), ex); + ShowErrorFallback(ex, RenderErrorSource.RootRender, renderingRoot.GetType().Name); return; } } @@ -649,6 +657,8 @@ void RecoverFromHookOrder(HookOrderException ex, RenderContext ctx, string mode) catch (Exception ex) when (!RenderErrorDispatch.IsPropagating(ex)) { _logger?.LogError(ex, "Function component threw"); + // Before the fallback: the app's handler may call Propagate(), which throws. + Reconciler.EmitRenderError(nameof(FuncElement), ex); ShowErrorFallback(ex, RenderErrorSource.RootRender); return; } @@ -756,8 +766,9 @@ void RecoverFromHookOrder(HookOrderException ex, RenderContext ctx, string mode) _phaseSw.Restart(); failurePhase = RenderErrorSource.Effects; - if (_rootComponent is not null) - _rootComponent.Context.FlushEffects(); + effectsRoot = _rootComponent; + if (effectsRoot is not null) + effectsRoot.Context.FlushEffects(); else if (_funcContext is not null) _funcContext.FlushEffects(); failurePhase = RenderErrorSource.Reconcile; @@ -823,10 +834,20 @@ void RecoverFromHookOrder(HookOrderException ex, RenderContext ctx, string mode) catch (Exception ex) when (!RenderErrorDispatch.IsPropagating(ex)) { _logger?.LogError(ex, "Render FAILED"); - // A root effect failure belongs to the root component; a commit-phase one has no - // single owning component. + // A root effect failure belongs to the root component and is reported like a + // child's effect-flush failure (issue #1321) — before the fallback, because the + // app's handler may call Propagate(), which throws. A commit-phase failure has no + // single owning component and is not a RenderError. + if (failurePhase == RenderErrorSource.Effects) + { + Reconciler.EmitRenderError( + effectsRoot is not null + ? Microsoft.UI.Reactor.Core.Diagnostics.ComponentNames.For(effectsRoot, element: null) + : nameof(FuncElement), + ex); + } ShowErrorFallback(ex, failurePhase, - failurePhase == RenderErrorSource.Effects ? _rootComponent?.GetType().Name : null); + failurePhase == RenderErrorSource.Effects ? effectsRoot?.GetType().Name : null); } finally { diff --git a/tests/Reactor.AppTests.Host/SelfTest/Fixtures/RenderErrorNamesFixture.cs b/tests/Reactor.AppTests.Host/SelfTest/Fixtures/RenderErrorNamesFixture.cs new file mode 100644 index 000000000..b28c9e85a --- /dev/null +++ b/tests/Reactor.AppTests.Host/SelfTest/Fixtures/RenderErrorNamesFixture.cs @@ -0,0 +1,661 @@ +using System.Diagnostics.Tracing; +using Microsoft.UI.Reactor.Core; +using Microsoft.UI.Reactor.Core.Diagnostics; +using Microsoft.UI.Reactor.Diagnostics; +using Microsoft.UI.Reactor.Hosting; +using static Microsoft.UI.Reactor.Factories; + +namespace Microsoft.UI.Reactor.AppTests.Host.SelfTest.Fixtures; + +/// +/// ReactorEventSource.RenderError names the component that threw — not its element +/// record (ComponentElement`1) — on every path that replaces a render with the +/// error fallback: a child's first render (mount — class, function and memo components), +/// a child's re-render (update), and the root of both ReactorHost and +/// ReactorHostControl (component and render function). An explicitly thrown +/// OutOfMemoryException outside any boundary — which the host still recovers from — +/// is reported too. Before the fix the mount +/// and root paths emitted nothing at all, +/// and the update path reported the element type unless the Render keyword was also on. +/// +/// Subscribes to Errors ONLY, which is the configuration that exposed the +/// bug: with Render enabled too, the update path borrowed the correct name from +/// the render-span bookkeeping and hid it. +/// +internal class RenderErrorNames_ComponentTypeOnEveryPath(Harness h) : SelfTestFixtureBase(h) +{ + public override async Task RunAsync() + { + var names = new List(); + using var subscription = ReactorTrace.Subscribe( + e => + { + if (e.EventName == nameof(ReactorEventSource.RenderError) + && e.Payload[1] as string is nameof(RenderErrorProbeException) or nameof(OutOfMemoryException)) + { + var name = (string)e.Payload[0]!; + if (e.Payload[1] as string == nameof(OutOfMemoryException)) name += OomTag; + lock (names) names.Add(name); + } + }, + EventLevel.Error, + ReactorEventSource.Keywords.Errors); + + if (!ReactorEventSource.Log.IsEnabled(EventLevel.Error, ReactorEventSource.Keywords.Errors)) + { + H.Skip("RenderErrorNames_Mount", "EventSource disabled (NativeAOT)"); + return; + } + + List Take() + { + lock (names) + { + var copy = names.ToList(); + names.Clear(); + return copy; + } + } + + // ── Mount: Component throwing on its first render ───── + var host = H.CreateHost(); + host.Mount(ctx => + { + var (n, setN) = ctx.UseState(0); + return VStack(4, + Component(n), + Component(n), + RenderEachTime(_ => throw new RenderErrorProbeException("func mount")), + Memo(_ => throw new RenderErrorProbeException("memo mount"), "stable"), + Button("bump", () => setN(n + 1))); + }); + await Harness.Render(); + + var mount = Take(); + Console.WriteLine("# mount RenderError: " + string.Join(", ", mount)); + H.Check("RenderErrorNames_Mount_NamesTheComponent", mount.Contains(nameof(ThrowOnMountCounter))); + // Function and memo components have no type of their own: they report their element. + H.Check("RenderErrorNames_Mount_FuncComponentReported", mount.Count(n => n == nameof(FuncElement)) == 1); + H.Check("RenderErrorNames_Mount_MemoComponentReported", mount.Count(n => n == nameof(MemoElement)) == 1); + H.Check("RenderErrorNames_Mount_NoElementTypeName", + !mount.Any(n => n.StartsWith("ComponentElement", StringComparison.Ordinal))); + + // ── Update: a component that renders once, then throws ─────────── + H.ClickButton("bump"); + await Harness.Render(); + + var update = Take(); + Console.WriteLine("# update RenderError: " + string.Join(", ", update)); + H.Check("RenderErrorNames_Update_NamesTheComponent", update.Contains(nameof(ThrowOnUpdateCounter))); + H.Check("RenderErrorNames_Update_NoElementTypeName", + !update.Any(n => n.StartsWith("ComponentElement", StringComparison.Ordinal))); + + // ── Derived function / memo element records ────────────────────── + // FuncElement / MemoElement are non-sealed; a derived record is named by its own type + // on mount, as it is on update and unmount. + H.CreateHost().Mount(_ => VStack(4, + new DerivedFuncElement(_ => throw new RenderErrorProbeException("derived func")), + new DerivedMemoElement(_ => throw new RenderErrorProbeException("derived memo")))); + await Harness.Render(); + var derived = Take(); + Console.WriteLine("# derived func/memo mount RenderError: " + string.Join(", ", derived)); + H.Check("RenderErrorNames_Mount_DerivedFuncElementNamed", + derived.Count(n => n == nameof(DerivedFuncElement)) == 1); + H.Check("RenderErrorNames_Mount_DerivedMemoElementNamed", + derived.Count(n => n == nameof(DerivedMemoElement)) == 1); + + // ── Fatal exceptions outside any ErrorBoundary ─────────────────── + // OOM/SO skip the fallback arm and propagate to the host, whose outer catch shows + // its own fallback — a recovery, so they must be reported at the throw site too. + // Each case gets its own host: the host-level fallback replaces the whole tree. + async Task> MountAlone(Element child) + { + H.CreateHost().Mount(_ => VStack(child)); + await Harness.Render(); + return Take(); + } + + var oomClass = await MountAlone(Component()); + var oomFunc = await MountAlone(RenderEachTime(_ => throw new OutOfMemoryException("func"))); + var oomMemo = await MountAlone(Memo(_ => throw new OutOfMemoryException("memo"), "stable")); + Console.WriteLine("# unbounded OOM mount RenderError: " + + string.Join(" | ", new[] { oomClass, oomFunc, oomMemo }.Select(l => string.Join(", ", l)))); + H.Check("RenderErrorNames_Mount_OutOfMemory_ReportedOnce", + oomClass.Count(n => n == nameof(ThrowsOutOfMemory) + OomTag) == 1); + H.Check("RenderErrorNames_Mount_FuncOutOfMemory_ReportedOnce", + oomFunc.Count(n => n == nameof(FuncElement) + OomTag) == 1); + H.Check("RenderErrorNames_Mount_MemoOutOfMemory_ReportedOnce", + oomMemo.Count(n => n == nameof(MemoElement) + OomTag) == 1); + + var oomUpdateHost = H.CreateHost(); + oomUpdateHost.Mount(ctx => + { + var (n, setN) = ctx.UseState(0); + return VStack(4, + Component(n), + Button("oom bump", () => setN(n + 1))); + }); + await Harness.Render(); + Take(); + H.ClickButton("oom bump"); + await Harness.Render(); + + var oomUpdate = Take(); + Console.WriteLine("# unbounded OOM update RenderError: " + string.Join(", ", oomUpdate)); + H.Check("RenderErrorNames_Update_OutOfMemory_ReportedOnce", + oomUpdate.Count(n => n == nameof(ThrowsOutOfMemoryOnUpdate) + OomTag) == 1); + + // ── Host root component ────────────────────────────────────────── + var rootHost = H.CreateHost(); + rootHost.Mount(new ThrowingRoot()); + await Harness.Render(); + + var root = Take(); + Console.WriteLine("# root RenderError: " + string.Join(", ", root)); + H.Check("RenderErrorNames_Root_NamesTheComponent", root.Count(n => n == nameof(ThrowingRoot)) == 1); + + // ── Host root render function ──────────────────────────────────── + var rootFuncHost = H.CreateHost(); + rootFuncHost.Mount(_ => throw new RenderErrorProbeException("root func")); + await Harness.Render(); + + var rootFunc = Take(); + Console.WriteLine("# root func RenderError: " + string.Join(", ", rootFunc)); + H.Check("RenderErrorNames_RootFunc_Reported", rootFunc.Count(n => n == nameof(FuncElement)) == 1); + + // ── ReactorHostControl root component and render function ──────── + // A standalone ReactorHostControl doesn't register with ReactorApp.ActiveHost, so + // Harness.Render() can't wait on its render loop — give it wall-clock time + // (same as HostingCoverageFixtures). + var componentControl = new ReactorHostControl(); + componentControl.Mount(new ThrowingRoot()); + var funcControl = new ReactorHostControl(); + funcControl.Mount(_ => throw new RenderErrorProbeException("control root func")); + H.SetContent(new Microsoft.UI.Xaml.Controls.StackPanel + { + Children = { componentControl, funcControl }, + }); + await Harness.Render(200); + + var control = Take(); + Console.WriteLine("# host control RenderError: " + string.Join(", ", control)); + H.Check("RenderErrorNames_HostControl_Root_NamesTheComponent", + control.Count(n => n == nameof(ThrowingRoot)) == 1); + H.Check("RenderErrorNames_HostControl_RootFunc_Reported", + control.Count(n => n == nameof(FuncElement)) == 1); + + componentControl.Dispose(); + funcControl.Dispose(); + H.SetContent(null); + + // ── Root effect failures (component and render-function roots) ─── + // A root's effects are flushed by the host, not inside the root's render catch, so + // the host reports them — like a child's effect-flush failure. Mount-only effects, + // so each throws exactly once. + var effectHost = H.CreateHost(); + effectHost.Mount(new ThrowingRootEffect()); + await Harness.Render(); + var effectRoot = Take(); + var effectFuncHost = H.CreateHost(); + effectFuncHost.Mount(ctx => + { + ctx.UseEffect(() => throw new RenderErrorProbeException("root func effect"), Array.Empty()); + return TextBlock("root func effect rendered"); + }); + await Harness.Render(); + var effectRootFunc = Take(); + Console.WriteLine("# root effect RenderError: " + string.Join(", ", effectRoot) + + " | " + string.Join(", ", effectRootFunc)); + H.Check("RenderErrorNames_RootEffect_NamesTheComponent", + effectRoot.Count == 1 && effectRoot[0] == nameof(ThrowingRootEffect)); + H.Check("RenderErrorNames_RootFuncEffect_Reported", + effectRootFunc.Count == 1 && effectRootFunc[0] == nameof(FuncElement)); + + var effectControl = new ReactorHostControl(); + effectControl.Mount(new ThrowingRootEffect()); + var effectFuncControl = new ReactorHostControl(); + effectFuncControl.Mount(ctx => + { + ctx.UseEffect(() => throw new RenderErrorProbeException("control root func effect"), Array.Empty()); + return TextBlock("control root func effect rendered"); + }); + H.SetContent(new Microsoft.UI.Xaml.Controls.StackPanel + { + Children = { effectControl, effectFuncControl }, + }); + await Harness.Render(200); + var effectControls = Take(); + Console.WriteLine("# host control root effect RenderError: " + string.Join(", ", effectControls)); + H.Check("RenderErrorNames_HostControl_RootEffect_NamesTheComponent", + effectControls.Count(n => n == nameof(ThrowingRootEffect)) == 1); + H.Check("RenderErrorNames_HostControl_RootFuncEffect_Reported", + effectControls.Count(n => n == nameof(FuncElement)) == 1); + + effectControl.Dispose(); + effectFuncControl.Dispose(); + H.SetContent(null); + + // ── A root that mounts a replacement, then throws ──────────────── + // The failure belongs to the component that threw, not to the root the host holds + // by the time the catch runs. + var swapHost = H.CreateHost(); + swapHost.Mount(new RemountThenThrowRoot(() => swapHost.Mount(new ReplacementRoot()))); + await Harness.Render(); + var swapRender = Take(); + var swapEffectHost = H.CreateHost(); + swapEffectHost.Mount(new RemountThenThrowEffectRoot(() => swapEffectHost.Mount(new ReplacementRoot()))); + await Harness.Render(); + var swapEffect = Take(); + Console.WriteLine("# root replaced then threw RenderError: " + string.Join(", ", swapRender) + + " | " + string.Join(", ", swapEffect)); + H.Check("RenderErrorNames_RootReplacedInRender_NamesTheThrower", + swapRender.Count == 1 && swapRender[0] == nameof(RemountThenThrowRoot)); + H.Check("RenderErrorNames_RootReplacedInEffect_NamesTheThrower", + swapEffect.Count == 1 && swapEffect[0] == nameof(RemountThenThrowEffectRoot)); + + var swapControl = new ReactorHostControl(); + swapControl.Mount(new RemountThenThrowRoot(() => swapControl.Mount(new ReplacementRoot()))); + var swapEffectControl = new ReactorHostControl(); + swapEffectControl.Mount(new RemountThenThrowEffectRoot(() => swapEffectControl.Mount(new ReplacementRoot()))); + H.SetContent(new Microsoft.UI.Xaml.Controls.StackPanel + { + Children = { swapControl, swapEffectControl }, + }); + await Harness.Render(200); + var swapControls = Take(); + Console.WriteLine("# host control root replaced then threw RenderError: " + string.Join(", ", swapControls)); + H.Check("RenderErrorNames_HostControl_RootReplacedInRender_NamesTheThrower", + swapControls.Count(n => n == nameof(RemountThenThrowRoot)) == 1); + H.Check("RenderErrorNames_HostControl_RootReplacedInEffect_NamesTheThrower", + swapControls.Count(n => n == nameof(RemountThenThrowEffectRoot)) == 1 + && !swapControls.Contains(nameof(ReplacementRoot))); + + swapControl.Dispose(); + swapEffectControl.Dispose(); + H.SetContent(null); + } + + private const string OomTag = "!oom"; +} + +// Mounts a replacement root from its own Render(), then throws. +internal sealed class RemountThenThrowRoot(Action remount) : Component +{ + public override Element Render() + { + remount(); + throw new RenderErrorProbeException("replaced then threw"); + } +} + +// Mounts a replacement root from its (mount-only) effect, then throws. +internal sealed class RemountThenThrowEffectRoot(Action remount) : Component +{ + public override Element Render() + { + UseEffect(() => { remount(); throw new RenderErrorProbeException("replaced in effect then threw"); }, + Array.Empty()); + return TextBlock("remount effect root rendered"); + } +} + +internal sealed class ReplacementRoot : Component +{ + public override Element Render() => TextBlock("replacement root"); +} + +internal sealed class ThrowingRootEffect : Component +{ + public override Element Render() + { + UseEffect(() => throw new RenderErrorProbeException("root effect"), Array.Empty()); + return TextBlock("root effect rendered"); + } +} + +/// +/// An ErrorBoundary that catches a child's failed first render rolls the child back +/// (issue #1291), unmounting the half-built component. That ComponentUnmount names the +/// component exactly as a normal unmount does (Foo<Int32>, not Foo`1). +/// +internal class RenderErrorNames_BoundaryRollbackUnmountNamesComponent(Harness h) : SelfTestFixtureBase(h) +{ + public override async Task RunAsync() + { + var unmounts = new List(); + using var subscription = ReactorTrace.Subscribe( + e => + { + if (e.EventName == nameof(ReactorEventSource.ComponentUnmount) + && e.Payload[0] is string name && name.StartsWith("RollbackThrower", StringComparison.Ordinal)) + lock (unmounts) unmounts.Add(name); + }, + EventLevel.Informational, + ReactorEventSource.Keywords.Lifecycle); + + if (!ReactorEventSource.Log.IsEnabled(EventLevel.Informational, ReactorEventSource.Keywords.Lifecycle)) + { + H.Skip("RenderErrorNames_BoundaryRollback", "EventSource disabled (NativeAOT)"); + return; + } + + var host = H.CreateHost(); + host.Mount(_ => ErrorBoundary(Component>(), _ => TextBlock("rollback fallback"))); + await Harness.Render(); + + List seen; + lock (unmounts) seen = unmounts.ToList(); + Console.WriteLine("# rollback ComponentUnmount: " + string.Join(", ", seen)); + H.Check("RenderErrorNames_BoundaryRollback_FallbackShown", H.FindText("rollback fallback") is not null); + H.Check("RenderErrorNames_BoundaryRollback_UnmountNamesGenericComponent", + seen.Count == 1 && seen[0] == "RollbackThrower"); + } +} + +internal sealed class RollbackThrower : Component +{ + public override Element Render() => throw new RenderErrorProbeException("rollback"); +} + +internal sealed class RenderErrorProbeException(string message) : Exception(message); + +internal sealed record DerivedFuncElement(Func Render) : FuncElement(Render); + +internal sealed record DerivedMemoElement(Func Render) : MemoElement(Render, new object?[] { "stable" }); + +/// +/// RenderError and an app's RenderErrorHandler (issue #1291) see the same +/// failures. The event is emitted before the handler runs, so it fires once, naming the +/// component, whatever the handler does: replace the fallback (in-tree and at the root), +/// or call Propagate(). An exception the app declined keeps going out +/// through every Reactor frame. The throw site already reported it, so a component frame it +/// crosses on the way — here one inside an ErrorBoundary, whose report-and-rethrow arm +/// would otherwise take it — must not report it again. +/// +internal class RenderErrorNames_RenderErrorHandlerOutcomesReportOnce(Harness h) : SelfTestFixtureBase(h) +{ + public override async Task RunAsync() + { + var names = new List(); + using var subscription = ReactorTrace.Subscribe( + e => + { + if (e.EventName == nameof(ReactorEventSource.RenderError) + && e.Payload[1] as string == nameof(RenderErrorProbeException)) + lock (names) names.Add((string)e.Payload[0]!); + }, + EventLevel.Error, + ReactorEventSource.Keywords.Errors); + + if (!ReactorEventSource.Log.IsEnabled(EventLevel.Error, ReactorEventSource.Keywords.Errors)) + { + H.Skip("RenderErrorNames_Handler", "EventSource disabled (NativeAOT)"); + return; + } + + List Take() + { + lock (names) + { + var copy = names.ToList(); + names.Clear(); + return copy; + } + } + + var previousUnhandled = ReactorApplication.OnUnhandledException; + try + { + // ── App fallback replaces a child's render ─────────────────── + var appChild = H.CreateHost(); + appChild.RenderErrorHandler = _ => TextBlock("app child fallback"); + appChild.Mount(_ => VStack(Component(0))); + await Harness.Render(); + var child = Take(); + Console.WriteLine("# app fallback (child) RenderError: " + string.Join(", ", child)); + H.Check("RenderErrorNames_Handler_AppFallbackChild_ReportedOnceByName", + H.FindText("app child fallback") is not null + && child.Count == 1 && child[0] == nameof(ThrowOnMountCounter)); + + // ── App fallback replaces the root render ──────────────────── + var appRoot = H.CreateHost(); + appRoot.RenderErrorHandler = _ => TextBlock("app root fallback"); + appRoot.Mount(new ThrowingRoot()); + await Harness.Render(); + var root = Take(); + Console.WriteLine("# app fallback (root) RenderError: " + string.Join(", ", root)); + H.Check("RenderErrorNames_Handler_AppFallbackRoot_ReportedOnceByName", + H.FindText("app root fallback") is not null + && root.Count == 1 && root[0] == nameof(ThrowingRoot)); + + // ── Propagate(), handled by the app ────────────────────────── + var handled = new List(); + ReactorApplication.OnUnhandledException = ex => { handled.Add(ex); return true; }; + var propagating = H.CreateHost(); + propagating.RenderErrorHandler = e => { e.Propagate(); return null; }; + propagating.Mount(_ => VStack(Component(0))); + await Harness.Render(); + var propagated = Take(); + Console.WriteLine("# propagate (handled) RenderError: " + string.Join(", ", propagated)); + H.Check("RenderErrorNames_Handler_PropagateHandled_ReportedOnceByName", + handled.Count == 1 + && propagated.Count == 1 && propagated[0] == nameof(ThrowOnMountCounter)); + + // ── Propagate(), declined: escapes through an ErrorBoundary ── + // The nested host's root render reports the throw (FuncElement), then the app + // declines it and it escapes the nested Mount — out through the outer component's + // Render(), which sits inside a boundary. + ReactorApplication.OnUnhandledException = _ => false; + var nestedWindow = new Microsoft.UI.Xaml.Window { Title = "RenderErrorNames declined propagation" }; + nestedWindow.AppWindow.Resize(new global::Windows.Graphics.SizeInt32(300, 200)); + nestedWindow.Activate(); + var created = new List(); + Exception? escaped = null; + try + { + H.CreateHost().Mount(_ => ErrorBoundary( + Component(new NestedDecliningProps(nestedWindow, created)), + TextBlock("declined boundary fallback"))); + } + catch (RenderErrorProbeException ex) { escaped = ex; } + await Harness.Render(); + var declined = Take(); + Console.WriteLine("# propagate (declined, through boundary) RenderError: " + string.Join(", ", declined)); + H.Check("RenderErrorNames_Handler_PropagateDeclined_ReportedOnceAtThrowSite", + escaped is not null + && declined.Count == 1 && declined[0] == nameof(FuncElement)); + + foreach (var nested in created) + nested.Dispose(); + nestedWindow.Close(); + } + finally + { + ReactorApplication.OnUnhandledException = previousUnhandled; + } + } +} + +internal sealed record NestedDecliningProps(Microsoft.UI.Xaml.Window Window, List Created); + +// Starts nested Reactor work during its own render: a host whose first (inline) render +// fails and propagates. +internal sealed class NestedDecliningComponent : Component +{ + public override Element Render() + { + var nested = new ReactorHost(Props.Window) { RenderErrorHandler = e => { e.Propagate(); return null; } }; + Props.Created.Add(nested); + nested.Mount(_ => throw new RenderErrorProbeException("nested declined")); + return TextBlock("unreachable"); + } +} + +/// +/// An error an ErrorBoundary catches and recovers from must still reach a +/// RenderError listener — the user sees the fallback, so an inspector must too — and +/// must name the descendant that threw (the boundary cannot know it). Exactly one event per +/// throw: the component reports at the throw site and the boundary does not report again. +/// Covers the mount path (first render inside the boundary — class, function and memo +/// components, plus a throw that passes through an intermediate component) and the update +/// path (a component inside the boundary that starts throwing on re-render). The boundary +/// recovers from every exception, so an explicitly thrown OutOfMemoryException and an +/// exception whose Message getter throws must be reported too. +/// +internal class RenderErrorNames_ErrorBoundaryCatchIsReported(Harness h) : SelfTestFixtureBase(h) +{ + public override async Task RunAsync() + { + var names = new List(); + using var subscription = ReactorTrace.Subscribe( + e => + { + if (e.EventName == nameof(ReactorEventSource.RenderError) + && e.Payload[1] as string is nameof(RenderErrorProbeException) or nameof(MessageThrowsProbeException) + or nameof(OutOfMemoryException)) + { + // Tag OOM reports so they don't disturb the per-name counts of the other probes. + var name = (string)e.Payload[0]!; + if (e.Payload[1] as string == nameof(OutOfMemoryException)) name += OomTag; + lock (names) names.Add(name); + } + }, + EventLevel.Error, + ReactorEventSource.Keywords.Errors); + + if (!ReactorEventSource.Log.IsEnabled(EventLevel.Error, ReactorEventSource.Keywords.Errors)) + { + H.Skip("RenderErrorNames_Boundary_Mount", "EventSource disabled (NativeAOT)"); + return; + } + + List Take() + { + lock (names) + { + var copy = names.ToList(); + names.Clear(); + return copy; + } + } + + var host = H.CreateHost(); + host.Mount(ctx => + { + var (n, setN) = ctx.UseState(0); + return VStack(4, + ErrorBoundary(Component(n), _ => TextBlock("mount fallback")), + ErrorBoundary(Component(n), _ => TextBlock("update fallback")), + ErrorBoundary(RenderEachTime(_ => throw new RenderErrorProbeException("func")), + _ => TextBlock("func fallback")), + ErrorBoundary(Memo(_ => throw new RenderErrorProbeException("memo"), "stable"), + _ => TextBlock("memo fallback")), + // The throw passes through an intermediate component on its way to the + // boundary; only the component that threw may report it. + ErrorBoundary(Component(), _ => TextBlock("nested fallback")), + // Reporting must not read the exception's Message: an override that throws + // would replace the original exception on the rethrow path. + ErrorBoundary(Component(), _ => TextBlock("bad message fallback")), + // A boundary recovers from every exception, so every one must be reported — + // including an explicitly thrown OutOfMemoryException. + ErrorBoundary(Component(), _ => TextBlock("oom fallback")), + ErrorBoundary(RenderEachTime(_ => throw new OutOfMemoryException("func oom")), + _ => TextBlock("func oom fallback")), + ErrorBoundary(Memo(_ => throw new OutOfMemoryException("memo oom"), "stable"), + _ => TextBlock("memo oom fallback")), + ErrorBoundary(Component(n), _ => TextBlock("update oom fallback")), + Button("bump", () => setN(n + 1))); + }); + await Harness.Render(); + + var mount = Take(); + Console.WriteLine("# boundary mount RenderError: " + string.Join(", ", mount)); + // Positive control: the boundary really caught it (fallback on screen, no raw crash). + H.Check("RenderErrorNames_Boundary_Mount_FallbackShown", H.FindText("mount fallback") is not null); + H.Check("RenderErrorNames_Boundary_Mount_ReportedOnceByName", + mount.Count(n => n == nameof(ThrowOnMountCounter)) == 1); + H.Check("RenderErrorNames_Boundary_Func_ReportedOnce", + H.FindText("func fallback") is not null && mount.Count(n => n == nameof(FuncElement)) == 1); + H.Check("RenderErrorNames_Boundary_Memo_ReportedOnce", + H.FindText("memo fallback") is not null && mount.Count(n => n == nameof(MemoElement)) == 1); + H.Check("RenderErrorNames_Boundary_Nested_ReportedOnceByThrower", + H.FindText("nested fallback") is not null + && mount.Count(n => n == nameof(NestedThrowInner)) == 1 + && !mount.Contains(nameof(NestedThrowWrapper))); + H.Check("RenderErrorNames_Boundary_ThrowingMessage_ReportedOnce", + H.FindText("bad message fallback") is not null + && mount.Count(n => n == nameof(ThrowsBadMessage)) == 1); + H.Check("RenderErrorNames_Boundary_OutOfMemory_ReportedOnce", + H.FindText("oom fallback") is not null + && mount.Count(n => n == nameof(ThrowsOutOfMemory) + OomTag) == 1); + H.Check("RenderErrorNames_Boundary_FuncOutOfMemory_ReportedOnce", + H.FindText("func oom fallback") is not null + && mount.Count(n => n == nameof(FuncElement) + OomTag) == 1); + H.Check("RenderErrorNames_Boundary_MemoOutOfMemory_ReportedOnce", + H.FindText("memo oom fallback") is not null + && mount.Count(n => n == nameof(MemoElement) + OomTag) == 1); + + H.ClickButton("bump"); + await Harness.Render(); + + var update = Take(); + Console.WriteLine("# boundary update RenderError: " + string.Join(", ", update)); + H.Check("RenderErrorNames_Boundary_Update_FallbackShown", H.FindText("update fallback") is not null); + H.Check("RenderErrorNames_Boundary_Update_ReportedOnceByName", + update.Count(n => n == nameof(ThrowOnUpdateCounter)) == 1); + H.Check("RenderErrorNames_Boundary_UpdateOutOfMemory_ReportedOnce", + H.FindText("update oom fallback") is not null + && update.Count(n => n == nameof(ThrowsOutOfMemoryOnUpdate) + OomTag) == 1); + H.Check("RenderErrorNames_Boundary_NoElementTypeName", + !mount.Concat(update).Any(n => n.StartsWith("ComponentElement", StringComparison.Ordinal))); + } + + private const string OomTag = "!oom"; +} + +internal sealed class ThrowOnMountCounter : Component +{ + public override Element Render() => throw new RenderErrorProbeException("mount"); +} + +internal sealed class ThrowOnUpdateCounter : Component +{ + public override Element Render() + => Props == 0 ? TextBlock("fine") : throw new RenderErrorProbeException("update"); +} + +internal sealed class ThrowingRoot : Component +{ + public override Element Render() => throw new RenderErrorProbeException("root"); +} + +internal sealed class NestedThrowWrapper : Component +{ + public override Element Render() => VStack(TextBlock("wrapper"), Component()); +} + +internal sealed class NestedThrowInner : Component +{ + public override Element Render() => throw new RenderErrorProbeException("nested"); +} + +internal sealed class MessageThrowsProbeException : Exception +{ + public override string Message => throw new InvalidOperationException("Message getter threw"); +} + +internal sealed class ThrowsBadMessage : Component +{ + public override Element Render() => throw new MessageThrowsProbeException(); +} + +internal sealed class ThrowsOutOfMemory : Component +{ + public override Element Render() => throw new OutOfMemoryException("probe"); +} + +internal sealed class ThrowsOutOfMemoryOnUpdate : Component +{ + public override Element Render() + => Props == 0 ? TextBlock("fine") : throw new OutOfMemoryException("update probe"); +} diff --git a/tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs b/tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs index b3ec33789..2a7523d7a 100644 --- a/tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs +++ b/tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs @@ -388,6 +388,11 @@ internal static class SelfTestFixtureRegistry "InlineUI_IncrementalUpdate_RunMutatedInPlace", // Reconcile ETW span depth bookkeeping "ReconcileTraceDepth_TopLevelSpansSurviveNestedPasses", + // RenderError names the component (not ComponentElement`1) on mount/update/root + "RenderErrorNames_ComponentTypeOnEveryPath", + "RenderErrorNames_ErrorBoundaryCatchIsReported", + "RenderErrorNames_RenderErrorHandlerOutcomesReportOnce", + "RenderErrorNames_BoundaryRollbackUnmountNamesComponent", "FocusTrapContentDialog_ContainmentProbe", // Issue #487 — RichTextBlock + inline UI inside ScrollViewer scroll anchor "Issue487_ScrollOffsetRestoredAfterRunMutation", @@ -2437,6 +2442,10 @@ internal static string[] StaleTierDeclarations() => "InlineUI_IncrementalUpdate_PreservesChildIdentity" => new InlineUIContainerFixtures.InlineUI_IncrementalUpdate_PreservesChildIdentity(harness), "InlineUI_IncrementalUpdate_RunMutatedInPlace" => new InlineUIContainerFixtures.InlineUI_IncrementalUpdate_RunMutatedInPlace(harness), "ReconcileTraceDepth_TopLevelSpansSurviveNestedPasses" => new ReconcileTraceDepth_TopLevelSpansSurviveNestedPasses(harness), + "RenderErrorNames_ComponentTypeOnEveryPath" => new RenderErrorNames_ComponentTypeOnEveryPath(harness), + "RenderErrorNames_ErrorBoundaryCatchIsReported" => new RenderErrorNames_ErrorBoundaryCatchIsReported(harness), + "RenderErrorNames_RenderErrorHandlerOutcomesReportOnce" => new RenderErrorNames_RenderErrorHandlerOutcomesReportOnce(harness), + "RenderErrorNames_BoundaryRollbackUnmountNamesComponent" => new RenderErrorNames_BoundaryRollbackUnmountNamesComponent(harness), "FocusTrapContentDialog_ContainmentProbe" => new FocusTrapContentDialog_ContainmentProbe(harness), "Issue487_ScrollOffsetRestoredAfterRunMutation" => new Issue487ScrollAnchorFixtures.Issue487_ScrollOffsetRestoredAfterRunMutation(harness), "Issue487_RepeatedMutationDoesNotDrift" => new Issue487ScrollAnchorFixtures.Issue487_RepeatedMutationDoesNotDrift(harness), diff --git a/tests/Reactor.Tests/Diagnostics/ComponentNamesTests.cs b/tests/Reactor.Tests/Diagnostics/ComponentNamesTests.cs new file mode 100644 index 000000000..3a483425e --- /dev/null +++ b/tests/Reactor.Tests/Diagnostics/ComponentNamesTests.cs @@ -0,0 +1,146 @@ +using Microsoft.UI.Reactor.Core; +using Microsoft.UI.Reactor.Core.Diagnostics; +using Xunit; +using static Microsoft.UI.Reactor.Factories; + +namespace Microsoft.UI.Reactor.Tests.Diagnostics; + +/// +/// The component name RenderError (and the other per-component events) report. +/// Before the fix the error path named the ELEMENT, so every class component reported +/// ComponentElement — and ComponentElement`1 for Component<T, TProps> +/// — and an inspector could not tell which component threw. +/// +/// The element records and component instances here are pure managed objects, so +/// this runs headless; the reconciler/host wiring that calls it is covered by the +/// RenderErrorNames_* selftest fixture. +/// +public sealed class ComponentNamesTests +{ + private sealed record CounterProps(int Start); + + private sealed class Counter : Component + { + public override Element Render() => TextBlock($"{Props.Start}"); + } + + private sealed class Propless : Component + { + public override Element Render() => TextBlock("x"); + } + + private sealed class GenericList : Component> + { + public override Element Render() => TextBlock("list"); + } + + private sealed class Outer + { + public sealed class Inner : Component + { + public override Element Render() => TextBlock("inner"); + } + } + + [Fact] + public void GenericName_IsBuiltOnceAndReused() + { + // The name is asked for on every traced render and unmount; a generic name is + // built, so it is cached per type rather than reallocated on each call. + var first = ComponentNames.For(typeof(GenericList)); + var second = ComponentNames.For(typeof(GenericList)); + + Assert.Equal("GenericList", first); + Assert.Same(first, second); + } + + [Fact] + public void PropsComponentElement_NamesTheComponentNotTheElement() + { + // Component() builds a ComponentElement: the reported case. + var element = Component(new CounterProps(1)); + Assert.Equal("ComponentElement`1", element.GetType().Name); // the old, wrong answer + + Assert.Equal("Counter", ComponentNames.For(instance: null, element)); + } + + [Fact] + public void ProplessComponentElement_NamesTheComponent() + { + var element = Component(); + Assert.Equal("ComponentElement", element.GetType().Name); + + Assert.Equal("Propless", ComponentNames.For(instance: null, element)); + } + + [Fact] + public void LiveInstance_WinsOverTheElement() + { + // After a hot-reload type swap the node's instance is the edited type while the + // element still names the old one; the instance is what actually rendered. + var element = Component(); + + Assert.Equal("Counter", ComponentNames.For(new Counter(), element)); + } + + [Fact] + public void RootInstanceWithoutElement_IsNamed() + => Assert.Equal("Propless", ComponentNames.For(new Propless(), element: null)); + + [Fact] + public void FunctionAndMemoComponents_KeepTheirElementNames() + { + Assert.Equal("FuncElement", ComponentNames.For(null, new FuncElement(_ => TextBlock("f")))); + Assert.Equal("MemoElement", ComponentNames.For(null, Memo(_ => TextBlock("m"), 1))); + } + + private sealed record DerivedFunc(Func Body) : FuncElement(Body); + + private sealed record DerivedMemo(Func Body) : MemoElement(Body); + + [Fact] + public void GenericDerivedElements_UseTheGenericFormat() + { + // A derived element record carries no component type, so the element's own type is + // the name — formatted like any other generic, not as the raw DerivedFunc`1. + Assert.Equal("DerivedFunc", ComponentNames.For(null, new DerivedFunc(_ => TextBlock("f")))); + Assert.Equal("DerivedMemo", ComponentNames.For(null, new DerivedMemo(_ => TextBlock("m")))); + } + + [Fact] + public void NonGenericName_IsTheSimpleTypeNameAsBefore() + { + // Non-generic components report exactly Type.Name, so existing traces and greps + // keyed on ComponentRenderStart/ComponentUnmount names do not change. + Assert.Equal(typeof(Counter).Name, ComponentNames.For(typeof(Counter))); + } + + [Fact] + public void GenericComponent_RendersArgumentsInsteadOfTheArity() + { + Assert.Equal("GenericList", ComponentNames.For(typeof(GenericList))); + Assert.Equal("GenericList>", + ComponentNames.For(typeof(GenericList>))); + } + + [Fact] + public void NestedGeneric_ReportsOnlyItsOwnArguments() + { + Assert.Equal("Inner", ComponentNames.For(typeof(Outer.Inner))); + } + + [Fact] + public void ArrayArguments_KeepTheirElementTypeAndRank() + { + // An array of a constructed generic is not itself generic, but its metadata name + // (List`1[]) still carries the arity; it must not be cut down to "List". + Assert.Equal("GenericList[]>", ComponentNames.For(typeof(GenericList[]>))); + Assert.Equal("GenericList", ComponentNames.For(typeof(GenericList))); + Assert.Equal("GenericList", ComponentNames.For(typeof(GenericList))); + Assert.Equal("GenericList[][]>", ComponentNames.For(typeof(GenericList[][]>))); + } + + [Fact] + public void NullEverything_IsUnknownRatherThanAThrow() + => Assert.Equal("unknown", ComponentNames.For(null, null)); +} diff --git a/tests/Reactor.Tests/Docs/SwallowedErrorAuditTests.cs b/tests/Reactor.Tests/Docs/SwallowedErrorAuditTests.cs index 3d50fafd3..f0f7586c9 100644 --- a/tests/Reactor.Tests/Docs/SwallowedErrorAuditTests.cs +++ b/tests/Reactor.Tests/Docs/SwallowedErrorAuditTests.cs @@ -185,7 +185,7 @@ public void Every_ledger_table_uses_the_canonical_schema() // alone does not enforce that: once the ledger grows past a stale floor, // a later shrink back down to it passes silently. Equality makes every // change to the total a deliberate, reviewable edit in this file. - const int LedgerSiteTotal = 124; + const int LedgerSiteTotal = 138; const int LedgerSectionFloor = 19; const int LedgerRowFloor = 50;