Repository navigation
fix(orleans): the SECOND reporter of a teardown race is not a fault either (#3243) - #4929
Conversation
…ither (#3243) #3243 was fixed on 2026-09-04 for the missing-hub line: `HostedHubResult` carries `HostedHubOutcome` from the collection that owns the container to the grain, and `HubConstructionFailureLevel(HostShuttingDown)` reports it at Debug. The incident reopened on 2026-09-18 anyway, and the reason is worth stating because it looks like a regression and is not one. `MessageHubGrain`'s ACTIVATION chain has its own error arm a few lines away, and it was an unconditional `LogError` — so the same teardown-race `ObjectDisposedException` was still reported at fail level by the sibling the classification had not reached. It lands on the SAME incident by design: the fingerprint identifies the fault, not the reporter (Doc/Architecture/LogWatchTriage — the category is excluded once a frame is present, and the discriminating text is the EXCEPTION's message rather than the reporter's prose, so one fault printed by two catch sites stays one ticket, per #1170/#1171). Measured: the 2026-09-18 05:39:52Z sighting on `Admin/_LogIncident/e2028eb86d6a85a6` is that arm's line, `[ACTIVATE] Grain Admin/_Notification/…: activation faulted for …`, on a pod rolling. `ActivationFaultLevel` classifies it, sharing the two predicates that already exist rather than re-deriving the fact a third time: `HubDisposingException.IsHubDisposal` (the hub announced its own disposal) and `IsDisposedContainer` (an `ObjectDisposedException` naming a disposed Autofac `LifetimeScope` — #3243's exact signature). Both walk the exception CHAIN, because this arm receives the fault wrapped. `ActivationFaultReason` says WHICH condition fired instead of handing the reader a union; that wording splits no incident and is not trying to — the level is what decides whether this is ticketed at all, since the watcher ingests `fail:`/ `crit:` only. 🚨 This is a TICKETING decision, not a verbosity knob, and it is the narrower of the two possible changes: BEHAVIOUR is untouched on both branches. The fault is still recorded on `activationFailures`, still pushed to `_hubReadyRaw.OnError`, and the grain still deactivates so the next access re-runs resolution. A real failure — a configuration that threw, a node that never resolved, a `TimeoutException`, or a null fault — stays at Error with the wording unchanged, so an existing search still finds it. CONTROL, measured both ways on the same three facts by temporarily restoring the pre-fix `LogLevel.Error`: before: total 10, FAILED 2 — AnActivationAbandonedByTeardown_IsNotReportedAsAFault, AHubDisposalRace_CountsToo_EvenWrapped after: total 13, failed 0 (with HubConstructionFailureReasonTest) The 8 that passed in both runs include the new negative controls, so the diff moved only the benign branch. Verified: `MeshWeaver.Hosting.Orleans` and `MeshWeaver.Hosting.Orleans.Test` build Release -warnaserror with 0 Error(s) / 0 Warning(s) (assembly mtime confirms a real compile, not an up-to-date no-op); `MeshWeaver.Documentation.Test` 601/601 — which is how the front-matter guard caught an unquoted colon in the What's New Description before it ever reached CI. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
Resolve the activation-fault reason ambiguity and preserve the structured Path log property.
Review effort: Lite
Findings: None
What changed in this PR
Updates Orleans activation-fault logging so expected teardown races are debug-level while genuine failures remain errors.
Changes:
- Adds teardown classification and regression tests.
- Preserves fault handling behavior.
- Updates triage documentation and What's New.
| File | Summary |
|---|---|
test/MeshWeaver.Hosting.Orleans.Test/HubConstructionOutcomeReportingTest.cs |
Tests teardown and genuine activation-fault classifications. |
src/MeshWeaver.Hosting.Orleans/MessageHubGrain.cs |
Classifies activation faults and adjusts logging. |
src/MeshWeaver.Documentation/Data/WhatsNew/2026-09-19-a-pod-shutting-down-is-no-longer-an-error-on-the-other-path-too.md |
Adds the user-facing fix note. |
src/MeshWeaver.Documentation/Data/Architecture/LogWatchTriage.md |
Documents the sibling-reporter lesson. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Test Results (shard 5)0 tests 0 ✅ 0s ⏱️ Results for commit 8427013. |
Test Results (shard 3)0 tests 0 ✅ 0s ⏱️ Results for commit 8427013. |
Test Results (shard 1)0 tests 0 ✅ 0s ⏱️ Results for commit 8427013. |
Test Results (shard 4)0 tests 0 ✅ 0s ⏱️ Results for commit 8427013. |
Test Results (shard 2)601 tests 601 ✅ 23s ⏱️ Results for commit 8427013. |
Test Results (shard 0)270 tests 270 ✅ 2m 39s ⏱️ Results for commit 8427013. |
Test Results871 tests 871 ✅ 3m 3s ⏱️ Results for commit 8427013. |
The finding
#3243 was fixed on 2026-09-04 (
97635cc901) for the missing-hub line:HostedHubResultcarriesHostedHubOutcomefrom the collection that owns the container to the grain, andHubConstructionFailureLevel(HostShuttingDown)reports it atDebug. The incident reopened on2026-09-18 anyway, and that reads exactly like the fix having come undone. It has not.
MessageHubGrain's activation chain has its own error arm a few lines away, and it was anunconditional
LogError. So the same teardown-raceObjectDisposedExceptionwas still reported atfail level — by the sibling the classification never reached.
It lands on the same incident by design, and that is the part worth reading. Per
Doc/Architecture/LogWatchTriage, the fingerprint identifies the fault, not the reporter: thereporting category is excluded from the identity once a frame is present, and the discriminating text
is the exception's message rather than the reporter's prose — deliberately, so that one fault
printed by two catch sites stays one ticket (#1170/#1171, which were two issues for one
ObjectDisposedException). The consequence nobody had drawn out is the converse: downgrading onereporter changes nothing while a sibling still shouts.
Measured — the 2026-09-18T05:39:52Z sighting on
Admin/_LogIncident/e2028eb86d6a85a6is that arm'sline, on a pod rolling:
against the 2026-09-03 sample on the same incident, which is the other line — the one already fixed.
The change
ActivationFaultLevelclassifies that arm, sharing the two predicates that already exist ratherthan re-deriving the fact a third time:
HubDisposingException.IsHubDisposal— the hub announced its own disposal;HubDisposingException.IsDisposedContainer— anObjectDisposedExceptionnaming a disposed AutofacLifetimeScope, which is MessageHubGrain treats expected host-disposal race as a fail-level hub-construction error #3243's exact filed signature.Both walk the exception chain, because this arm receives the fault wrapped.
ActivationFaultReasonsays which condition fired instead of handing the reader a union — the same correction
HubConstructionFailureReasonmade for the sibling line. 🚨 That wording splits no incident and is nottrying to: with an exception present the identity uses the exception's message, so the level is what
decides whether this is ticketed at all (the watcher ingests
fail:/crit:only).🚨 This is a ticketing decision, not a verbosity knob — the same judgement recorded in the code for
the first half — and it is the narrower of the two possible changes: behaviour is untouched on both
branches. The fault is still recorded on
activationFailures, still pushed to_hubReadyRaw.OnError,and the grain still deactivates so the next access re-runs resolution.
Control — measured both ways, on the same tests
By temporarily restoring the pre-fix
LogLevel.Error:AnActivationAbandonedByTeardown_IsNotReportedAsAFault,AHubDisposalRace_CountsToo_EvenWrappedHubConstructionFailureReasonTest)The 8 that passed in both runs include the new negative controls, so the diff moved only the benign
branch. Those controls are the point: a
TimeoutExceptionor anInvalidOperationExceptionstays atErrorwith the wording byte-identical ("activation faulted for Admin/PlatformVersion", assertedwith
Be, notContain) so an existing search still finds it, and a null fault stays atError—no evidence of a teardown is not evidence of one, the same trap
EverythingElse_StaysAtFailLevelguards on the other arm.
Verification
dotnet build src/MeshWeaver.Hosting.Orleans -c Release -warnaserrordotnet build test/MeshWeaver.Hosting.Orleans.Test -c Release -warnaserrordotnet test …Hosting.Orleans.Test --filter HubConstruction*dotnet test test/MeshWeaver.Documentation.Test(full project)Both builds were re-run on this branch after it was cut from a newer
main. The Orleans assembly'smtime was checked against the build's start time, because a sub-3-second "Build succeeded" is
indistinguishable from an up-to-date no-op.
The doc project was run in full rather than filtered — which is how the front-matter guard caught an
unquoted colon in the What's New
Descriptionbefore it reached CI. A filtered run can pass havingmatched nothing.
Conserved
Doc/Architecture/LogWatchTriagegains the general lesson beside the machinery it is about:classify every reporter of the fault, or the ticket does not stop — with this as the measured
instance, and the instruction to grep a component for its sibling arms before believing a downgrade
is complete.
Category: Fix.No public surface is removed or retyped and no interface member is added, so
Pairs-with:andImplementers:do not apply; no i18n catalog key changes, so neither doesMirror-sync:.I am not asserting the incident will now go quiet — it stays open until a roll on an image carrying
this produces no sighting, and per the dormancy lesson recorded on #1540 today, a teardown fault is
silent between rolls whatever its state.
🤖 Generated with Claude Code