Skip to content

feat(health): return structured probe outcomes - #665

Merged
SunSi12138 merged 5 commits into
devfrom
feat/655-structured-health-probe
Sep 12, 2026
Merged

feat(health): return structured probe outcomes#665
SunSi12138 merged 5 commits into
devfrom
feat/655-structured-health-probe

Conversation

@SunSi12138

@SunSi12138 SunSi12138 commented Sep 12, 2026

Copy link
Copy Markdown
Owner

Closes #655

Summary

  • add SharpLinkHealthProbeOutcome with Success, NotReady, Unavailable, and Unsupported
  • keep remote Ready / Draining / Unhealthy in nullable SharpLinkHealthCheckResult.Status, populated only for Success
  • make zero-ready, in-flight remote connection loss/deadline, and missing HealthCheck capability normal structured results
  • preserve caller cancellation, canonical terminal lifecycle, protocol corruption, programmer/configuration error, and unexpected runtime failures as exceptions
  • keep protocol v2 health wire grammar unchanged and avoid ordinary RPC endpoint-selection/hot-path changes
  • map structured results explicitly in SharpLinkRemoteHealthCheck; Unsupported has a documented stable Unhealthy mapping
  • document the result/exception/Hosting contract and [Runtime Architecture][Phase 18] 完成 public API/package 审计、发布门禁与迁移文档 #86 migration note

Validation coverage

  • remote Ready / Draining / Unhealthy => Success + remote status
  • zero Ready connection => NotReady, with no connection side effect
  • hosted Client Running + NotReady => structured NotReady
  • LifecycleState=Running while legacy connection State=Faulted => structured NotReady
  • remote connection closes during probe => Unavailable
  • local StopAsync races an in-flight probe => terminal ConnectionClosed remains exceptional
  • peer does not negotiate HealthCheck => Unsupported and no health frame is sent
  • caller cancellation => OperationCanceledException
  • malformed HealthResponse => SharpLinkErrorCode.ProtocolViolation
  • multi-cluster scoped health preserves the same structured result contract
  • Hosting maps all six normal result combinations without attaching expected exceptions

Pre-PR self-check

  • rebuilt on latest dev after feat(api): add non-throwing multi-cluster status query #664 before opening the PR; current branch is ahead 5 / behind 0
  • final diff is limited to 6 files: health result, health query, Hosting mapping, focused tests, and health contract documentation
  • checked namespace/type references against current dev and existing test helpers/fakes
  • checked indentation/braces and nullable Status call sites touched by this change; CI formatting gate passes
  • Release build passes with 0 warnings and 0 errors
  • HealthResponseCodec refuses to serialize non-success local outcomes, so local outcome values cannot leak onto the protocol wire
  • no RpcChannel, pending-call terminal machinery, retry/admission, or ordinary RPC invocation files changed

Acceptance self-review

  • structured result represents remote status plus local NotReady / Unavailable / Unsupported
  • local probe outcomes never masquerade as remote Unhealthy
  • single-cluster and multi-cluster scoped health use the same contract
  • caller cancellation remains cancellation
  • malformed/protocol-invalid response remains a protocol failure
  • canonical local terminal lifecycle remains exceptional before and during a probe
  • Hosting uses structured mapping for normal states instead of broad exception handling
  • health query does not reconnect or make the Client Ready
  • Protocol v2 wire format and ordinary RPC hot path remain unchanged
  • XML/result documentation and dedicated migration/contract documentation added

CI follow-up

Two initial PR Fast failures were followed to root cause and fixed before first review readiness. Both were setup defects in the new multi-cluster test, not production-code failures:

  1. the test omitted the required explicit multi-cluster request-timeout policy; fixed with DisableRequestTimeout()
  2. the test used a slot without a static route; fixed with the repository-standard AllowDynamicContracts dynamic-only slot configuration

Review follow-up

Review found two lifecycle-domain races after #658:

  1. legacy connection State == Faulted can coexist with canonical LifecycleState == Running; health preflight now uses LifecycleState, so zero-ready connectivity fault remains structured NotReady
  2. local shutdown can close an already-selected probe connection; expected-unavailability mapping now re-checks canonical terminal lifecycle and rethrows the winning terminal exception instead of returning Unavailable

Regressions freeze the actual Running / connection Faulted / NotReady supervisor window and race an in-flight health probe against local StopAsync.

Latest exact head 4b1feb2d1cdfbb2c2e69916194f021c4c6f0d8d3 is green on:

  • PR Fast — success, including formatting, Release build, Unit Tests, generator/load tests, and allocation gate
  • PR Quick — success, including Debug/Release builds, Unit/Integration tests, NativeAOT, pack/NuGet contract + smoke, load and chaos smoke
  • Pending and codec validation — success
  • Extension fault containment — success
  • CodeQL — success

Both review threads were replied to and resolved. Ready for review.

@SunSi12138
SunSi12138 marked this pull request as ready for review September 12, 2026 05:04

@SunSi12138 SunSi12138 left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

整体 result/Hosting 方向符合 #655,但发现 1 个状态域 blocker:health preflight 仍把 legacy connection State.Faulted 当成 terminal lifecycle,导致合法 Running+NotReady 窗口异常泄漏。

Comment thread src/SharpLink.Client/SharpLinkClient.Health.cs

@SunSi12138 SunSi12138 left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

补充同一 lifecycle/result 边界的另一处 race:probe 启动后本地 Stop/terminal shutdown 会被当前 catch 重新分类为普通 Unavailable,与 PR 文档声称的 terminal lifecycle exceptional contract 不一致。

Comment thread src/SharpLink.Client/SharpLinkClient.Health.cs
@SunSi12138
SunSi12138 merged commit 2dfe658 into dev Sep 12, 2026
12 checks passed
@SunSi12138
SunSi12138 deleted the feat/655-structured-health-probe branch September 12, 2026 05:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant