Repository navigation
Fix SurrealDB server readiness: add health check, remove provisioning retry loop - #1503
Merged
Merged
Conversation
… retry loop - Add WithHealthCheck to SurrealDbServerResource so OnResourceReady only fires after SurrealDB WebSocket/RPC is confirmed usable via SurrealDbHealthCheck (Health() + RETURN 1 over WebSocket) - Add OnConnectionStringAvailable to populate SurrealDbOptions for the server health check, matching the pattern used by namespace/database resources - Remove the open-ended namespace-existence retry loop in EnsuresNsDbCreated; replace with a single Use() call that fails immediately on error since the health check already proves the server is ready - Make CreateNamespaceAsync throw DistributedApplicationException on failure instead of silently logging and continuing - Make CreateDatabaseAsync throw DistributedApplicationException on failure instead of silently logging and continuing; error messages include resource name, namespace, and database for easy diagnosis - Remove noisy LogInformation calls from SurrealDbHealthCheck that fired on every health check poll - Add AddSurrealServerRegistersHealthCheck test to verify the annotation Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/CommunityToolkit/Aspire/main/eng/scripts/dogfood-pr.sh | bash -s -- 1503Or
iex "& { $(irm https://raw.githubusercontent.com/CommunityToolkit/Aspire/main/eng/scripts/dogfood-pr.ps1) } 1503" |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves the SurrealDB hosting integration’s startup reliability by gating SurrealDbServerResource readiness on an actual WS/RPC health probe, and by making provisioning failures fail fast instead of retrying indefinitely.
Changes:
- Register and attach a new server-level health check for
AddSurrealServer, and use it to gateOnResourceReady. - Remove the open-ended namespace provisioning retry loop and throw
DistributedApplicationExceptionwith more context on provisioning failures. - Reduce health-check log noise by removing per-poll
LogInformationlines.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| tests/CommunityToolkit.Aspire.Hosting.SurrealDb.Tests/AddSurrealServerTests.cs | Adds a test asserting the server resource has a health check annotation (suggestion: also assert DI registration). |
| src/CommunityToolkit.Aspire.SurrealDb/SurrealDbHealthCheck.cs | Removes noisy per-poll info logging from the DB-level health check. |
| src/CommunityToolkit.Aspire.Hosting.SurrealDb/SurrealDbBuilderExtensions.cs | Adds server readiness health check, removes provisioning retry loop, and improves fail-fast exceptions for namespace/database creation. |
Comment on lines
+732
to
+736
| catch (Exception ex) | ||
| { | ||
| logger.LogError(ex, "SurrealDB server health check for '{ResourceName}' raised an exception.", server.Name); | ||
| return new HealthCheckResult(context.Registration.FailureStatus, exception: ex); | ||
| } |
Comment on lines
+89
to
+94
| var appModel = app.Services.GetRequiredService<DistributedApplicationModel>(); | ||
|
|
||
| var containerResource = Assert.Single(appModel.Resources.OfType<SurrealDbServerResource>()); | ||
| var healthCheckAnnotation = Assert.Single(containerResource.Annotations.OfType<HealthCheckAnnotation>()); | ||
| Assert.Equal("surreal_server_check", healthCheckAnnotation.Key); | ||
| } |
afscrome
temporarily deployed
to
azure-artifacts
August 1, 2026 22:51 — with 
GitHub Actions
Inactive
GitHub Actions
Inactive
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
afscrome
temporarily deployed
to
azure-artifacts
August 1, 2026 23:19 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
Odonno
requested changes
Aug 2, 2026
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
|
afscrome
temporarily deployed
to
azure-artifacts
August 2, 2026 10:53 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
Odonno
approved these changes
Aug 2, 2026
This branch was previously deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Based on analysis from #1497
The
SurrealDbServerResourcehad a startup race:OnResourceReadyfired as soon as the container was running, before SurrealDB was actually accepting WebSocket/RPC traffic. Namespace/database bootstrapping could then fail at the connection level, and the fallback retry loop (while (!ct.IsCancellationRequested)) could hang indefinitely until the outer test timeout.This PR fixes the race cleanly without adding more recovery logic:
Server health check --
AddSurrealServernow registers aSurrealDbServerHealthCheck(viaWithHealthCheck) that callsclient.Health()over WebSocket. The check resolves the connection string lazily viaConnectionStringExpression.GetValueAsync(ct)on each poll, so noOnConnectionStringAvailableplumbing is needed. A 5-second linkedCancellationTokenSourcebounds each probe, sinceSurrealDbClient.Health()has no internal timeout on the WS engine.OnResourceReadynow only fires after the health check passes, i.e. after the RPC channel is confirmed usable.Provisioning fail-fast -- The open-ended
while (!ct.IsCancellationRequested)retry loop inEnsuresNsDbCreatedis removed and replaced with a singleUse()call.CreateNamespaceAsyncandCreateDatabaseAsyncno longer swallow exceptions; failures throwDistributedApplicationExceptionimmediately with resource name, namespace, and database in the message.Log cleanup -- Two
LogInformationcalls inSurrealDbHealthCheckthat fired on every poll are removed.PR Checklist
Other information
The
SurrealDbServerHealthCheckprivate class is self-contained insideSurrealDbBuilderExtensionsand is not part of the public API. The existingSurrealDbHealthCheck(used for database-level health checks) is unchanged except for removal of the noisy info logs.Docker integration tests (
AppHostTests) are the authoritative end-to-end validation and require Docker to run.