DefaultHealthCheckService throws exception

[Vshishkar]

Is there an existing issue for this?

• I have searched the existing issues

Describe the bug

Based on the code at HealthCheckPubliserHosterService DefaultHealthCheckService should not throw exceptions, but if there is exception on initialization of a health check service doesn't properly handle it, which leads to failure of publishing health report.

For example if there is a dependency of a health check which is not registered in container, health check initialization would throw exception.

Expected Behavior

HealthCheckService would return failed with failure of health check.

It's arguable whether failure of health check initialization should be reported the same as failure of logic within health check, but it must be reported in some way.

Steps To Reproduce

Create a simple health check with missing dependency:

	public class NonWorkingHealthCheck : IHealthCheck
	{
		public NonWorkingHealthCheck(string param, IDependency dep) {}
		
		public Task<HealthCheckResult> CheckHealthAsync(
		    HealthCheckContext context,
		    CancellationToken cancellationToken = default)
		         => Task.FromResult(HealthCheckResult.Healthy());

        interface IDependency {}

And register this health check:

builder.AddTypeActivatedCheck<NonWorkingHealthCheck>("NonWorking", "param1");

Exceptions (if any)

No response

.NET Version

7.0

Anything else?

No response

[halter73]

Is this exception being thrown at startup or during the health check request? Doesn't this result in a empty 500 response?

Hi @Vshishkar. We have added the "Needs: Author Feedback" label to this issue, which indicates that we have an open question for you before we can take further action. This issue will be closed automatically in 7 days if we do not hear back from you by then - please feel free to re-open it if you come back to this issue after that time.

[Vshishkar]

@halter73 this exception been thrown during health check request.
My undesrtanding of health check desing were that if they throw exceptions, exception will be handled and unhealthy report will be published.

In my team we build our custom health report publisher (inspired by HealthCheckPubliserHosterService) and we never expected the exception from HealthCheckService. In the result our publisher never published health report to service fabric cluster and our services were reported healthy.

I have a proposed fix: just move health check creating into try/catch which is already in place in the DefaultHealthCheckService or create health check validation at healthCheckBuilder. I would love to contribute these changes myself.

[Vshishkar]

@halter73 I also want to add that not only DI mssconfiguration could lead to exception. Basically any code that throws exception in the contructor of health check won't produce the report. I feel that this should be changed, because the design of health checks makes me depend on health reports.

Right know exception will be thrown and it would be very hard to find it and analyze that it was connected with health checks. And you will have to specifically look for it.

On the other hand it's more likely that you will be monitoring health of your app and will fix problem much quicker.

[Vshishkar]

Hi @halter73 could you please address my comments?