Skip to content

Name the component, not its element, in RenderError — and report mount, host-root and ErrorBoundary-caught render errors - #1323

Merged
Alexandre Zollinger Chohfi (azchohfi) merged 19 commits into
mainfrom
azchohfi-render-error-component-name
Oct 7, 2026
Merged

Alexandre Zollinger Chohfi (azchohfi) merged 19 commits into
mainfrom
azchohfi-render-error-component-name

Conversation

@azchohfi

@azchohfi Alexandre Zollinger Chohfi (azchohfi) commented Oct 5, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

ReactorEventSource.RenderError (EventId 9) reported the element type for class components — ComponentElement`1 for every Component<T, TProps>(…), ComponentElement for Component<T>() — so winapp devtools logs and the overlay error badge could not name <Counter>. Mount-time and host-root render errors emitted no event at all, and neither did any render error an ErrorBoundary caught. This PR reports the component's type name on every path where a component's Render() throws, including inside a boundary, with exactly one event per throw.

Linked issue / spec

Fixes #1321

Spec 044 §6.2.1 (PII) is respected: payloads carry developer-authored type names only.

What changed

  • ComponentNames.For(Component? instance, Element? element) (internal, src/Reactor/Core/Diagnostics/ComponentNames.cs), precedence: the live instance's type → ComponentElement.ComponentType → the element type (FuncElement / MemoElement). Non-generic names are exactly Type.Name, as before; a generic component renders as ItemList<Int32> instead of ItemList`1. A nested generic shows only its own type arguments. Reflects only Name / GetGenericArguments, so it is AOT/trim-safe. Generic names are cached per type in a ConditionalWeakTable, so collectible or hot-reloaded types are never rooted.

  • RenderError uses that name, and is now emitted from:

    • the update path, which no longer depends on the Render keyword being on;
    • the three mount paths — Component, Func, Memo (new);
    • the root render catch blocks (component and render function) in ReactorHost and ReactorHostControl (new), and those hosts' outer catch when the root's effect flush throws (new). Reconcile / Cleanup failures have no failing component and are not reported;
    • inside an ErrorBoundary (new): each component catch gains a sibling clause, when (_errorBoundaryDepth > 0), that emits RenderError and rethrows unchanged. The throw site is the only place that knows which descendant threw, and the boundary does not emit again, so there is exactly one event per throw.

    Still RenderError (EventId 9, Errors keyword, Error level, same payload). Existing subscribers pick up the new emissions with no change.

  • ComponentRenderStart / Stop, EffectsFlush*, ComponentUnmount (including the ErrorBoundary rollback unmount from [Feature] Let apps replace the built-in error fallback that shows the full exception text #1291) and the reconciler's ILogger messages use the same helper, so every event agrees on the name. ComponentUnmount now computes the name only when its Lifecycle keyword is enabled.

  • SwallowedError is unchanged (its payload is category / operation / exception type; no call site passes a component).

  • Guide: source-mapping (template + compiled + diagram) — the component-name paragraph, the attribution diagram label/alt text, and the RenderError row. CHANGELOG.md [Unreleased] → Fixed entry. advanced guide: how the ETW event relates to RenderErrorHandler. Spec-044 swallowed-error ledger rows for every render-error catch this PR touches.

Test plan

  • New unit tests tests/Reactor.Tests/Diagnostics/ComponentNamesTests.cs (12). Mutation check: reverting to element naming / raw Type.Name fails 4 of the original 9; GenericName_IsBuiltOnceAndReused covers the per-type name cache.
  • New selftest fixtures tests/Reactor.AppTests.Host/SelfTest/Fixtures/RenderErrorNamesFixture.cs, subscribed to Errors only:
    • RenderErrorNames_ComponentTypeOnEveryPath — class/func/memo mount, update, ReactorHost root component + render function, ReactorHostControl root component + render function.
    • RenderErrorNames_ErrorBoundaryCatchIsReported — inside a boundary on mount (class, func, memo, and a throw that crosses an intermediate component) and on update: fallback shown, reported exactly once, by the thrower's name. Also an exception whose Message getter throws, and an explicitly thrown OutOfMemoryException on every boundary arm (class/func/memo mount and update).
  • dotnet run --project tests/Reactor.AppTests.Host --no-build -c Debug -p:Platform=x64 -- --self-test --filter "RenderError" — passed, 0 failures (at the merge 4075055bf), together with main's RenderErrorHandler_* and ErrorBoundary* fixtures.
  • Mutation check of the new selftest oracles: removing the FuncElement mount emission and the ReactorHostControl root-function emission fails RenderErrorNames_Mount_FuncComponentReported and RenderErrorNames_HostControl_RootFunc_Reported (2 failures); reading ex.Message in EmitRenderError fails RenderErrorNames_Boundary_ThrowingMessage_ReportedOnce; restoring the OOM/SO carve-out on the boundary arms fails all 4 boundary *OutOfMemory_ReportedOnce checks; limiting the report-and-rethrow arms to boundaries again fails the 4 non-boundary OOM checks. Restored afterwards.
  • dotnet test tests/Reactor.Tests -p:Platform=x64 — passed: 14435 total, 14371 succeeded, 64 skipped, 0 failed (at d8496be71, on origin/main 1e8c6b196). dotnet test tests/Reactor.DocPipeline.Tests -p:Platform=x64 — passed 479/479. SwallowedErrorAuditTests — passed 11/11.
  • dotnet build src/Reactor/Reactor.csproj --no-restore -c Release -p:Platform=x64 and dotnet build tests/Reactor.AppTests.Host --no-restore -c Release -p:Platform=x64 — passed, 0 errors (warnings-as-errors on). The full dotnet build Reactor.slnx --no-restore -c Release could not be completed locally: dotnet restore Reactor.slnx fails with NU1301 (TLS handshake to nuget.org) for the vs-reactor projects in this environment; relying on CI's Build solution job for that gate.
  • REACTOR_SELFTEST_SHARD=1/2 dotnet test tests/Reactor.SelfTests -p:Platform=x64 at 78a593d2e — passed: 930 total, 929 succeeded, 1 skipped (the intentional SelfTestVerdict_OnlySkips_PositiveControl), 0 failed.
  • No public API added; reactor.api.txt unchanged.
  • Merged origin/main (00cd48920: Let apps replace the built-in render-error fallback #1311 RenderErrorHandler, Add pr-lifecycle contributor skill #1346). Conflicts at every render catch site were resolved keeping both intents. Main's BuildInTreeFallback / ShowErrorFallback dispatch and !RenderErrorDispatch.IsPropagating(ex) filters are unchanged. RenderError is emitted before the handler runs, so it fires once per throw whatever the handler returns. The report-and-rethrow arms also let a declined propagation pass, since its throw site already reported it. New fixture RenderErrorNames_RenderErrorHandlerOutcomesReportOnce covers an app fallback (child and root), a handled Propagate(), and a declined Propagate() escaping through an ErrorBoundary; each is reported once, by name. Mutation: dropping the IsPropagating filter reports the declined case twice (FuncElement, NestedDecliningComponent). RenderError.ComponentName (the handler API) keeps main's values. API index regenerated with mur --regen-api: unchanged. Copilot's round on the merge raised 3 findings, fixed in 9ee41eaa4. (1) The [Feature] Let apps replace the built-in error fallback that shows the full exception text #1291 rollback ComponentUnmount used the old naming; it now goes through ComponentNames + the keyword guard, covered by RenderErrorNames_BoundaryRollbackUnmountNamesComponent. (2, 3) Host-root effect failures reached the handler with no RenderError; they now emit, covered by RenderErrorNames_*RootEffect* / *RootFuncEffect*. All 5 new checks fail with the fixes reverted. The docs scope the event to component Render()/effect failures, and the spec-044 ledger records the hosts' outer catches (LedgerSiteTotal 136 → 138). The next round, fixed in 0469209dc: (a) both hosts read _rootComponent after app code ran, so a root that Mount()s a replacement and then throws was attributed to the replacement. The root is now captured before Render() / FlushEffects(), and the same instance names the event and the handler's RenderError.ComponentName; covered by RenderErrorNames_*RootReplaced* (4 checks, all red on the mutant). (b) "Previously missed": ComponentNames cut an array argument down to its generic name (GenericList<List<int>[]> → GenericList<List>); it now formats element type and rank, covered by the unit test ArrayArguments_KeepTheirElementTypeAndRank (red on the mutant). Merged origin/main again (fix(datagrid): keep inline editor text aligned with the display cell #1349, Enable IDE0330 and move lock gates to System.Threading.Lock #1351): the CHANGELOG conflict was resolved by keeping both Fixed entries. The next round found that the func/memo mount arms hardcoded FuncElement / MemoElement, while those records are non-sealed and update/unmount name derived records by their own type. Fixed by routing them through ComponentNames.For, covered by RenderErrorNames_Mount_Derived*ElementNamed (red on the mutant). Release builds of src/Reactor, tests/Reactor.AppTests.Host and tests/Reactor.Tests are clean under the newly enabled IDE0330. The next round asked for generic derived element records to use the generic format (DerivedFunc<Int32>, not DerivedFunc1). Fixed in b585258and covered by the unit testGenericDerivedElements_UseTheGenericFormat(red on the mutant). Mergedorigin/mainagain (#1347, #1318) with no conflicts. #1318's new publicReactorHostInfo.RootComponentName` deliberately uses its own fully qualified format and is untouched.
  • Includes a one-line fix for a red main. At 22f267926, main's CI fails on src/Reactor/Core/Diagnostics/ReactorDiagnostics.Hosts.cs(305): error IDE0330. Diagnostics: enumerate Reactor hosts and their roots; tag component boundaries for inspectors #1318 added ReactorHostRegistry's object gate after Enable IDE0330 and move lock gates to System.Threading.Lock #1351 enabled IDE0330. Commit 3cc699958 makes it a System.Threading.Lock, the same change Enable IDE0330 and move lock gates to System.Threading.Lock #1351 made to every other gate. It's unrelated to this PR. Main then fixed the same break in Fix IDE0330 build break in ReactorHostRegistry #1356; the next merge (671fed2dc) took main's version of that file, so the one-liner merged away. After that merge: the API index was regenerated (unchanged), and the only lock statements this PR adds are test-local locks on List instances, not object gates, so IDE0330 is clean.
  • After the merge: dotnet test tests/Reactor.Tests -p:Platform=x64 passed 14579 total, 0 failed, 64 skipped (at 3cc699958). Release builds of src/Reactor, tests/Reactor.AppTests.Host and tests/Reactor.Tests are clean, and the RenderError / ErrorBoundary / HostDiagnostics selftests have 0 failures. dotnet test tests/Reactor.DocPipeline.Tests -p:Platform=x64 passed 493/493. Release src/Reactor builds clean. dotnet test tests/Reactor.SelfTests -p:Platform=x64 (full): 1785 total, 1777 passed, 5 skipped, 3 failed. The failures are Issue487_ScrollViewOffsetRestoredAfterRunMutation, ItemsView_Parking_PendingBringThenKeyedReset and ItemsView_Parking_VariableHeightReset; a clean origin/main 00cd48920 worktree fails them identically on this machine, so they are environmental and unrelated to this PR. CI is the gate.
  • Copilot review round 1 (3 comments): two said RenderError leaks ex.Message — not the case, the ReactorEventSource.RenderError sink writes string.Empty for the message (TASK-064); replied and noted it on EmitRenderError. One asked to cache generic names — done in 78a593d2e (ConditionalWeakTable per type), and both ComponentUnmount call sites now skip computing the name when the Lifecycle keyword is off. Round 2: "Previously missed" — CHANGELOG entry; added under [Unreleased] → Fixed. Round 3: the new catches were missing from the spec-044 swallowed-error ledger (docs/specs/044/swallowed-error-audit.md); recorded 12 sites (8 Keep / user-callback isolation fallback catches incl. the host roots, 4 PromoteEvent report-and-rethrow arms), distribution re-derived, LedgerSiteTotal 124 → 136. Round 4: don't read the virtual ex.Message on the error path (a throwing override would replace the original exception) — fixed, selftest added. Round 5 "Previously missed": the boundary arms' OOM/SO carve-out let an explicitly thrown OutOfMemoryException be recovered silently — carve-out removed (the boundary's own catch takes everything), selftests added; and the source-mapping diagram label + alt text still said Component.GetType().Name — updated in template, guide and both SVG copies. Round 6 "Previously missed": outside a boundary an explicitly thrown OOM/SO skipped the fallback arm and was recovered by the host's outer catch without an event — the report-and-rethrow arm now takes OOM/SO anywhere (they still propagate exactly as before); selftests cover class/func/memo mount and update. Round 7: Findings: None, nothing previously missed.
  • Pre-PR review with the repo's pr-review skill: 0 critical / 0 high. Four medium test-coverage gaps (func/memo mount, ReactorHost root render function, ReactorHostControl roots) fixed by the third commit. One low finding dismissed: "ComponentNames.For(Type) duplicates Reconciler.Mount.FriendlyTypeName" — FriendlyTypeName prints the declaring chain (Outer.Inner<T>) for copy-pasteable handler diagnostics, while component event names deliberately keep Type.Name for nested types so existing trace names stay byte-identical; sharing would need a mode switch that changes one or the other. Multi-model cross-check agreed.

Risk / breaking changes

  • Listeners now receive previously silent errors. RenderError also fires for render errors at mount time (class, function, memo), in the host root (ReactorHost and ReactorHostControl), and inside an ErrorBoundary (once, at the throw site, even though the app recovered and shows the fallback). Errors-keyword consumers will see more events.
  • The component name changes from ComponentElement / ComponentElement`1 to the component's type name. That is the fix, but greps keyed on the old string will stop matching.
  • Generic component names change from Foo`1 to Foo<Int32> in all component events. Non-generic names are byte-identical.
  • No new public API; same event, same payload shape.
  • Follow-up (not in this PR): the pending ComponentRendered event change should use the same ComponentNames helper; that will be folded into whichever of the two lands second.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note

Copilot was unable to run its full agentic suite in this review.

Copilot review overview

Review effort: Lite
Findings: 1 High severity · 2 Medium severity

Open (3)
What changed in this PR

This PR improves Reactor diagnostics by consistently reporting component names (not ComponentElement / ComponentElement\1) across render tracing, unmount, and RenderErroremission paths, including behavior insideErrorBoundary`.

Changes:

  • Introduces ComponentNames helper to compute stable, readable component names (including generic argument formatting).
  • Updates reconciler + hosts to emit RenderError consistently on mount/update/root, including when an ErrorBoundary catches and recovers.
  • Adds unit + self-test fixtures and updates documentation to reflect the new naming behavior.
File Description
tests/​Reactor.Tests/​Diagnostics/​ComponentNamesTests.cs Adds unit tests validating ComponentNames output across element/instance/generics/null cases.
tests/​Reactor.AppTests.Host/​SelfTest/​SelfTestFixtureRegistry.cs Registers new self-test fixtures covering RenderError name attribution paths.
tests/​Reactor.AppTests.Host/​SelfTest/​Fixtures/​RenderErrorNamesFixture.cs Adds end-to-end ETW subscription tests for RenderError naming (mount/update/root, and ErrorBoundary).
src/​Reactor/​Hosting/​ReactorHostControl.cs Emits RenderError for host-control root component/function paths using the new naming rules.
src/​Reactor/​Hosting/​ReactorHost.cs Emits RenderError for host root component/function paths using the new naming rules.
src/​Reactor/​Core/​Reconciler.cs Centralizes RenderError emission, switches diagnostics naming to ComponentNames, and reports errors inside ErrorBoundary at throw site.
src/​Reactor/​Core/​Reconciler.Mount.cs Aligns mount-time error reporting with update-time behavior (including ErrorBoundary path).
src/​Reactor/​Core/​Diagnostics/​ComponentNames.cs New helper to attribute diagnostics to component types rather than element carrier types.
docs/​guide/​source-mapping.md Documents updated component naming behavior across diagnostics events.
docs/​_pipeline/​templates/​source-mapping.md.dt Mirrors doc updates in the template used to generate the guide.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/Reactor/Core/Reconciler.cs
Comment thread docs/guide/source-mapping.md Outdated
Comment thread src/Reactor/Core/Diagnostics/ComponentNames.cs

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

The user-visible diagnostics change needs the repository-required [Unreleased] changelog entry.

Review effort: Balanced
Findings: None

Resolved since last review (3)
Previously missed (1)

In code that hasn't changed since last review

Low severity Document diagnostics changes in CHANGELOG.md

docs/​guide/​source-mapping.md:82

This externally visible diagnostics change is missing from CHANGELOG.md. The repo requires notable changes under [Unreleased] in a Keep-a-Changelog bucket with the originating reference (CHANGELOG.md:3,13-24), and this change both adds previously silent RenderError emissions and changes names consumed by existing listeners. Please add a Fixed entry citing issue #1321 (and spec 044 §6.2.1).

@github-actions

github-actions Bot commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

📦 Build metrics

Artifact sizes for 671fed2 vs the base branch (12d0816).

Packages (compressed .nupkg)

Artifact base PR Δ
Microsoft.UI.Reactor.nupkg 1.79 MB 1.80 MB +1.62 KB (+0.09%) ⚠️
Microsoft.UI.Reactor.Advanced.nupkg 493.4 KB 493.3 KB -13 B (0.00%) ≈
Microsoft.UI.Reactor.Devtools.nupkg 288.2 KB 288.2 KB +5 B (0.00%) ≈

Assemblies in Microsoft.UI.Reactor

Artifact base PR Δ
Reactor.Analyzers.dll 386.0 KB 386.0 KB +0 B (0.00%) ≈
Reactor.dll 2.52 MB 2.52 MB +1.50 KB (+0.06%) ⚠️
Reactor.Localization.Generator.dll 16.0 KB 16.0 KB +0 B (0.00%) ≈
Reactor.Wrappers.Abstractions.dll 10.5 KB 10.5 KB +0 B (0.00%) ≈
Reactor.Wrappers.Generator.dll 99.5 KB 99.5 KB +0 B (0.00%) ≈

Assemblies in Microsoft.UI.Reactor.Advanced

Artifact base PR Δ
Reactor.Advanced.dll 1.00 MB 1.00 MB +0 B (0.00%) ≈

Assemblies in Microsoft.UI.Reactor.Devtools

Artifact base PR Δ
Microsoft.UI.Reactor.Devtools.dll 788.0 KB 788.0 KB +0 B (0.00%) ≈

✅ smaller / ⚠️ larger / ≈ within noise. Sizes come from a Release dotnet pack on the CI runner: packages are the compressed .nupkg download size, assemblies the uncompressed DLL inside it.
workflow run.

RenderError reported the ELEMENT type for class components: "ComponentElement`1"
for every Component<T, TProps>(...) and "ComponentElement" for Component<T>(),
unless the Render keyword was also enabled (the update path only borrowed the
real name from the render-span bookkeeping). The mount path and the hosts' root
render emitted no RenderError at all, so a component that throws on its first
render, or a throwing root, was invisible to an Errors-only listener.

- ComponentNames.For(instance, element): live instance type, else
  ComponentElement.ComponentType, else the element type (FuncElement /
  MemoElement). Non-generic names are unchanged (Type.Name); generic components
  render "ItemList<Int32>" instead of "ItemList`1". Type names only (PII-safe).
- RenderError now uses it on the update path and is emitted on the mount paths
  (Component / Func / Memo) and for ReactorHost / ReactorHostControl roots.
- ComponentRenderStart/Stop, EffectsFlush and ComponentUnmount, plus the
  reconciler's ILogger messages, use the same name.
- SwallowedError carries no component name (category / operation / exception
  type only): unchanged.
- Unit tests for the naming (mutation-checked); selftest fixture
  RenderErrorNames_ComponentTypeOnEveryPath for mount/update/root emission;
  source-mapping guide.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
A component render error inside an ErrorBoundary propagates to the boundary,
which logs and renders its fallback. The component's own catch is filtered on
_errorBoundaryDepth == 0, so no RenderError was emitted: the user saw a
fallback while an inspector (overlay error badge, devtools logs) saw nothing.

- Each component render catch (update path, and Component / Func / Memo mount)
  gains a sibling clause for _errorBoundaryDepth > 0 that emits RenderError with
  the failing component's ComponentNames name and rethrows unchanged. The throw
  site is the only place that knows which descendant threw; the boundary does
  not emit again, so it is exactly one event per throw.
- Same event (RenderError, EventId 9, Errors, Error level, same payload): every
  existing subscriber picks it up with no change, and the event's meaning (a
  component's Render() threw) is unchanged.
- Selftest fixture RenderErrorNames_ErrorBoundaryCatchIsReported (mount and
  update inside a boundary, once by name); source-mapping guide row.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…h hosts' roots

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…racing is off

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@github-actions

github-actions Bot commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

🧪 Merged coverage

Coverage for 671fed2 vs the base branch (12d0816) — unit + selftest merged.

Metric base PR Δ
Line 86.52% 86.65% +0.13 pp ✅
Branch 77.36% (967/1250) 77.44% (968/1250) +0.08 pp ≈

✅ higher / ⚠️ lower / ≈ within noise. Δ is in percentage points; coverage is unit + selftest merged (Debug x64) on the CI runner. Cobertura reports attached to the workflow run as artifacts.

…rror (#1321)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

The four new broad exception catches must be added to the repository’s required spec-044 audit ledger.

Review effort: Balanced
Findings: 2 Low severity

Open (2)

Comment thread src/Reactor/Core/Reconciler.Mount.cs Outdated
Comment thread src/Reactor/Core/Reconciler.cs Outdated
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Error emission unnecessarily evaluates Exception.Message, which can disrupt recovery for custom exceptions.

Review effort: Balanced
Findings: 1 High severity

Open (1)
Resolved since last review (2)

Comment thread src/Reactor/Core/Reconciler.cs Outdated
The sink redacts the message, and an app exception whose Message override
throws would otherwise replace the original exception on the
ErrorBoundary rethrow path.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

Boundary-caught fatal exceptions can still recover without emitting RenderError, and the source-mapping alt text remains stale.

Review effort: Balanced
Findings: None

Resolved since last review (1)
Previously missed (4)

In code that hasn't changed since last review

Medium severity Boundary-caught OutOfMemoryException remains invisible to Errors listeners

src/​Reactor/​Core/​Reconciler.Mount.cs:871

An ErrorBoundary catches every Exception in MountErrorBoundary, including an explicitly thrown OutOfMemoryException, but this filter bypasses the only throw-site RenderError emission for those exceptions. The boundary then renders its fallback, so the error is handled yet remains invisible to Errors listeners, contradicting the new “including errors an ErrorBoundary catches” contract. This boundary arm should report every exception that the enclosing boundary will catch (or the boundary itself must be narrowed consistently).

Medium severity OutOfMemoryException bypasses boundary-caught render error emission

src/​Reactor/​Core/​Reconciler.Mount.cs:919

The enclosing ErrorBoundary catches every Exception, but this filter skips RenderError for an explicitly thrown OutOfMemoryException before the boundary recovers with its fallback. As a result, this function-component mount path still has a boundary-caught render error with no event, contrary to the documented behavior.

This issue also appears on line 967 of the same file.

Medium severity Update boundary misses RenderError for OutOfMemoryException

src/​Reactor/​Core/​Reconciler.cs:2173

UpdateErrorBoundary catches every Exception, so an explicitly thrown OutOfMemoryException bypasses this emission and is nevertheless recovered by the boundary. That produces no RenderError for a boundary-caught update error, contrary to the new event contract; this throw-site arm should cover the same exception set as the enclosing boundary.

Low severity Update image alt text to reflect ComponentNames formatting

docs/​_pipeline/​templates/​source-mapping.md.dt:62

The image alt text immediately below still says component attribution flows via Component.GetType().Name, which would produce the raw Foo`1 form this paragraph now says is not emitted. Update the template to name the ComponentNames formatting path, then regenerate the compiled guide so the accessible description matches the documented behavior.

…ibution alt text

The report-and-rethrow arms excluded OutOfMemoryException and
StackOverflowException, but the boundary's own catch takes everything, so an
explicitly thrown OOM was recovered without a RenderError. The source-mapping
diagram and its alt text still named Component.GetType().Name.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Component attribution can still name a replacement root, and generic array arguments are formatted incorrectly.

Review effort: Balanced
Findings: 2 Medium severity

Open (2)
Resolved since last review (3)
Previously missed (1)

In code that hasn't changed since last review

Medium severity Handle arrays before stripping generic type arity

src/​Reactor/​Core/​Diagnostics/​ComponentNames.cs:66

Append drops constructed-generic array arguments. For example, typeof(List<int>[]).Name contains a backtick while IsGenericType is false, so this code appends only List and GenericList<List<int>[]> is reported as GenericList<List>. Handle arrays before stripping generic arity, recursively formatting their element type and then appending the array rank.

Comment thread src/Reactor/Hosting/ReactorHost.cs Outdated
Comment thread src/Reactor/Hosting/ReactorHostControl.cs Outdated
…arguments

Capture the root component before app code runs in both hosts, so a root that
Mount()s a replacement from Render() or an effect and then throws is reported
(on RenderError and to the RenderErrorHandler) as itself, not the replacement.
ComponentNames now formats array arguments (List<Int32>[]) instead of cutting
them to the bare generic name.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

The reporting paths are consistent, comprehensively tested, and synchronized with the documentation and error audit.

Review effort: Balanced
Findings: None

Resolved since last review (2)

Resolves the CHANGELOG conflict with #1349 by keeping both Fixed entries.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Mount events hardcode function and memo base names, causing inconsistent diagnostics for derived element records.

2 open findings

🧠 Review effort: Balanced

Comment thread src/Reactor/Core/Reconciler.Mount.cs Outdated
Comment thread src/Reactor/Core/Reconciler.Mount.cs Outdated
FuncElement and MemoElement are non-sealed; the mount arms hardcoded the base
names while update and unmount use ComponentNames.For, so a derived record had
two names. Route the mount arms through the helper too.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Generic derived function and memo elements still produce raw arity-based diagnostic names.

1 open finding
2 resolved since last review

🧠 Review effort: Balanced

Comment thread src/Reactor/Core/Diagnostics/ComponentNames.cs Outdated
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
main is red at 22f2679: #1318 added ReactorHostRegistry's object gate after
#1351 enabled IDE0330, so the Release build fails. Same one-line change #1351
applied to every other gate.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The implementation is internally consistent, thoroughly tested, and synchronized with diagnostics documentation and auditing.

0 open findings

1 resolved since last review

🧠 Review effort: Balanced

…-component-name

# Conflicts:
#	src/Reactor/Core/Diagnostics/ReactorDiagnostics.Hosts.cs

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Core exception routing changes span multiple reconciler and host recovery paths, warranting final human review despite strong coverage.

0 open findings

🧠 Review effort: Balanced

@azchohfi
Alexandre Zollinger Chohfi (azchohfi) merged commit b507677 into main Oct 7, 2026
46 checks passed
@azchohfi
Alexandre Zollinger Chohfi (azchohfi) deleted the azchohfi-render-error-component-name branch October 7, 2026 22:46
Alexandre Zollinger Chohfi (azchohfi) added a commit that referenced this pull request Oct 7, 2026
…ComponentNames

Keeps #1323's RenderError emission (before the fallback, including ErrorBoundary-caught throws) alongside ComponentRendered: the boundary arms main added now also report the render. Folds in the cross-dependency change: ComponentRendered names components with ComponentNames.For (generic classes as Foo<Int32>, FuncElement/MemoElement fallbacks), for child components and host roots, so it agrees with RenderError. Adds ComponentRendered_GenericNames_MatchComponentNames.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Alexandre Zollinger Chohfi (azchohfi) added a commit that referenced this pull request Oct 7, 2026
Conflicts kept both sides: CHANGELOG entries; NeedsTag keeps the Native AOT
call-site-only skip and adds #1318's component-boundary tag; the generator test
stub has both the static-info and root-mount-site members; one combined
InternalsVisibleTo comment. API index regenerated. K's tests now pass the
column that SourceLocation requires positionally.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Alexandre Zollinger Chohfi (azchohfi) added a commit that referenced this pull request Oct 7, 2026
…component names

Folds in the integration follow-up (2495eb4b7) now that #1318 and #1323 are
on main: a root render function's hooks= resolves from its recorded mount
site, and owner/mounts/root names forward to ComponentNames.For. Also adopts
System.Threading.Lock for the static-facts gate (IDE0330, now enforced in
Release).

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Alexandre Zollinger Chohfi (azchohfi) added a commit that referenced this pull request Oct 14, 2026
…ohfi-reactor-component-diagnostics

- Hosts: keep #1328/#1323's root lifecycle (RetireRoot, deferred
  retirement, _releaseReplacedTreeOnNullRender, RootRenderDiagnostics)
  alongside the diagnostics rendered-root publication. The published root
  is snapshotted at the start of the pass (beside _renderingRoot), so a
  Mount() from the root's own Render() does not relabel the tree that pass
  commits; RetireRoot clears it once the root is retired (immediately or
  in the deferred pass), so a retired root is never described or written.
- Selftests: the remount gap now describes no root (the old one is
  retired at Mount); new mid-pass Mount case; mutation-checked.
- SourceLocation's new required ColumnNumber in a unit test.
- CHANGELOG keeps both entries; API index regenerated (unchanged).

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
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.

[Bug] RenderError names ComponentElement`1 instead of the component, and misses mount, host-root and ErrorBoundary-caught errors

2 participants