Surface diagnostics when endpoint references fail to resolve#17265
Surface diagnostics when endpoint references fail to resolve#17265IEvangelist wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
Improves diagnostics when an EndpointReference cannot resolve (e.g. consumer calls GetEndpoint("https") but the producer only has http). Previously, the resulting FailedToApplyEnvironmentException was thrown without a message and the orchestrator silently set state to FailedToStart with no logging. Now the underlying InvalidOperationException lists available endpoint names, the DCP executor logs an error and propagates the flattened message through OnResourceFailedToStartContext.ErrorMessage, and the orchestrator marks the state snapshot with KnownResourceStateStyles.Error so the dashboard surfaces the failure.
Changes:
EndpointReference.EndpointAnnotationnow throws anInvalidOperationExceptionenumerating the available endpoint names (or "no endpoints defined").DcpExecutorwrapsFailedToApplyEnvironmentExceptionwith a flattened message + inner exception, logs via the resource logger, and propagatesex.Messagethrough the new optionalErrorMessagefield onOnResourceFailedToStartContext;ApplicationOrchestratorapplies theErrorstyle when a message is present (keepingText = FailedToStart).- Tests cover the new
EndpointReferencemessage variants, the orchestrator style behavior, andFormatConfigurationExceptionMessagefor single/multi/nested/blank aggregates plus an integration test through the gatherer/resolve path.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/Aspire.Hosting/ApplicationModel/EndpointReference.cs | Builds a descriptive missing-endpoint message that lists available endpoint names. |
| src/Aspire.Hosting/Dcp/DcpExecutorEvents.cs | Adds optional ErrorMessage to OnResourceFailedToStartContext. |
| src/Aspire.Hosting/Dcp/DcpExecutor.cs | Wraps FailedToApplyEnvironmentException with message+inner, logs on catch, threads message through event; adds FormatConfigurationExceptionMessage helper. |
| src/Aspire.Hosting/Orchestrator/ApplicationOrchestrator.cs | When ErrorMessage set, applies KnownResourceStateStyles.Error to the FailedToStart snapshot. |
| tests/Aspire.Hosting.Tests/EndpointReferenceTests.cs | New tests for descriptive endpoint-not-found error message. |
| tests/Aspire.Hosting.Tests/ExecutionConfigurationGathererTests.cs | Integration test through gatherer + unit tests for FormatConfigurationExceptionMessage. |
| tests/Aspire.Hosting.Tests/Orchestrator/ApplicationOrchestratorTests.cs | Tests verifying Error style with/without ErrorMessage. |
There was a problem hiding this comment.
Thanks @davidfowl — moved the per-value logging back into ExecutionConfigurationGathererContext.ResolveAsync (restoring @afscrome's pattern from #7032 that was lost in the builder refactor) and dropped the catch-site LogError calls. The catch sites now just forward ex.Message via OnResourceFailedToStartContext.ErrorMessage so the orchestrator can apply the Error style to the FailedToStart state. Also rebased onto latest main, which moved the throw sites into ExecutableCreator/ContainerCreator.
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 17265Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 17265" |
Fixes microsoft#15486 When a TypeScript (or C#) AppHost calls getEndpoint() with an endpoint name that doesn't exist on the target resource, the dependent resource silently transitioned to FailedToStart with no useful diagnostics. The underlying exception (e.g. `The endpoint `https` is not defined for the resource `api`.`) was completely lost. This change surfaces the diagnostic end-to-end: * EndpointReference.EndpointAnnotation now includes the list of available endpoint names (or `no endpoints defined`) in the InvalidOperationException message. * ExecutionConfigurationGathererContext.ResolveAsync logs per-value errors against the resource logger when an argument or env var fails to resolve, restoring the pattern previously added by @afscrome and removed during the builder-pattern refactor. * ExecutableCreator wraps the underlying configuration exception into FailedToApplyEnvironmentException with the inner exception preserved (matching ContainerCreator). * OnResourceFailedToStartContext gains an optional ErrorMessage forwarded from the catch site so consumers can react to the descriptive message. * ApplicationOrchestrator.OnResourceFailedToStart applies KnownResourceStateStyles.Error to the state snapshot when an error message is provided, so the dashboard renders FailedToStart with the error style. The state Text remains KnownResourceStates.FailedToStart so existing checks against KnownResourceStates.TerminalStates continue to work. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
e18178c to
149fd2c
Compare
|
Re-running the failed jobs in the CI workflow for this pull request because 1 job was identified as retry-safe transient failures in the CI run attempt.
|
karolz-ms
left a comment
There was a problem hiding this comment.
Looks good, thanks for taking care of this, David
Description
Fixes #15486
When a TypeScript (or C#) AppHost calls
getEndpoint('https')on a resource that only exposes'http', the dependent resource silently transitions toFailedToStartwith no useful diagnostics — no PID, no exit code,aspire describeshows an empty environment,aspire logs <resource>is empty/uninformative, and the actual underlying exception ("The endpoint 'https' is not defined for the resource 'api-boston'.") is completely lost.Root cause
GetEndpoint(name)intentionally returns a deferredEndpointReference(so forward-references work). The missing annotation only surfaces later, whenEndpointReference.EndpointAnnotationis read during environment resolution.ExecutionConfigurationGathererContext.ResolveAsynccatches thatInvalidOperationExceptionper-value and packages everything intoConfiguration.Exception(anAggregateException).DcpExecutorthen threw a parameter-lessFailedToApplyEnvironmentException()— no message, no inner — and the threecatchblocks published anOnResourceFailedToStartContextevent with no logging and no error detail.ApplicationOrchestrator.OnResourceFailedToStartonly set the state text toFailedToStart.Fix
End-to-end diagnostic propagation:
EndpointReference.cs— TheEndpointAnnotationgetter now throws anInvalidOperationExceptionwhose message lists the available endpoint names (or"The resource has no endpoints defined."for resources with zero endpoints).DcpExecutorEvents.cs—OnResourceFailedToStartContextgains an optionalstring? ErrorMessage = nullfield (no breaking change for existing subscribers).DcpExecutor.cs— The twoFailedToApplyEnvironmentExceptionthrow sites now pass a flattened message and the originalconfiguration.Exceptionas the inner. All threecatch (FailedToApplyEnvironmentException)blocks now log via the resource logger (LogError, no stack-trace noise) and propagateex.Messagethrough the newErrorMessagefield on the event. A newFormatConfigurationExceptionMessagehelper flattensAggregateException(1 inner → its.Message; multiple → joined with";"), filters null/blank inner messages, and falls back to the aggregate's own message (or the exception type name) when nothing meaningful is available.ApplicationOrchestrator.cs— WhenOnResourceFailedToStartreceives a non-emptyErrorMessage, the resultingResourceStateSnapshotcarriesKnownResourceStateStyles.Error. The stateTextdeliberately staysKnownResourceStates.FailedToStartso existingKnownResourceStates.TerminalStateschecks and other callers that compare by exact text continue to work.After this change
aspire logs <consumer>includesFailed to apply environment configuration for resource 'consumer'. The endpoint 'https' is not defined for the resource 'producer'. Available endpoints: 'http'.FailedToStartrow with the error style.InvalidOperationExceptionis preserved asInnerExceptiononFailedToApplyEnvironmentExceptionfor anything inspecting it programmatically.Validation
EndpointReferenceTests+ 16ApplicationOrchestratorTests+ 26ExecutionConfigurationGathererTests).EndpointReferenceTests— 4 tests covering the new error message (single available endpoint, multiple, no endpoints defined, customErrorMessageoverride preserved).ApplicationOrchestratorTests— 2 tests verifying theErrorstyle is applied whenErrorMessageis set andnullstyle is preserved when it isn't.ExecutionConfigurationGathererTests— 1 integration test exercising the fullWithEnvironment(producer.GetEndpoint("https"))→ gatherer →ResolveAsyncpath (verifiesconfiguration.Exceptionis anAggregateExceptionwhose inner is the descriptiveInvalidOperationExceptionand that wrapping it inFailedToApplyEnvironmentException(formatted, inner)preserves the original), plus 5 unit tests forFormatConfigurationExceptionMessagecovering single-inner / multi-inner / nested / non-aggregate / all-blank-inner.Checklist
<remarks />and<code />elements on your triple slash comments?aspire.devissue: