Throwing HealthCheckError in synchronous HealthIndicatorFunction does not work properly

[andreialecu]

Is there an existing issue for this?

• I have searched the existing issues

Current behavior

There is an issue with non-async health indicator functions due to the following:

terminus/lib/health-check/health-check-executor.service.ts

Line 63 in ea06b34

const result = await Promise.allSettled(healthIndicators.map((h) => h()));

If an error is thrown synchronously, then the health check will reply with {"statusCode":500,"message":"Internal server error"} and the server will log the error.

Minimum reproduction code

@Get()
  @HealthCheck()
  async check() {
    const result = await this.health.check([
      () => throw new HealthCheckError('error', {}), // if changed to `async () => `, it works
    ])
  }

Steps to reproduce

No response

Expected behavior

It should not depend on the function being async.

Package version

9.1.3

NestJS version

not relevant

Node.js version

not relevant

In which operating systems have you tested?

• macOS
• Windows
• Linux

Other

No response

[andreialecu]

Fixed in #2084 (/cc @BrunnerLivio)

[BrunnerLivio]

@andreialecu Curious; why would you want to run a synchronous health check?

[andreialecu]

Because I have a specific case where I have a stream that sometimes randomly stops working, so I store a lastMessageReceived timestamp and check if nothing got in in the past 2 minutes, then fail the health check because of it.

I had to dig through the Terminus code to see why this wasn't working right.

If non async healthchecks should not be supported, the types could prevent a non-promise from being passed. But they specifically allow it:

terminus/lib/health-indicator/health-indicator.ts

Lines 8 to 10 in 8fdf979

	export type HealthIndicatorFunction = () =>
	| PromiseLike<HealthIndicatorResult>
	| HealthIndicatorResult;

So this seemed like a bug to me.

PS: I'm currently working around it by passing an async function, but I thought someone else might run into this in the future, and it was worth fixing.

[BrunnerLivio]

@andreialecu Alright makes sense to me! Let's go forward with your PR then :)

[BrunnerLivio]

Released with 9.1.4 🎉