Handle healthcheck errors gracefully

[kibertoad]

Changes

Do not produce unhandler errors when checker fails

Checklist

• Apply one of following labels; major, minor, patch or skip-release
• I've updated the documentation, or no changes were necessary
• I've updated the tests, or no changes were necessary

[coderabbitai]

📝 Walkthrough

Summary by CodeRabbit

• Bug Fixes
  • Improved healthcheck resilience: Health checks that throw exceptions are now treated as failed checks rather than crashing the health status endpoint or blocking service startup. This ensures reliable health monitoring even when individual checks encounter errors.

Walkthrough

This pull request adds exception handling to health check plugins across four plugin variants (common, sync, public, and startup). Previously, if a health check's checker function threw an exception, it would crash the request or application startup. The changes now wrap each checker invocation in a try/catch block, converting any thrown exceptions into a consistent failure state. Non-Error values are coerced to Error instances, and all results are normalized into an Either format for consistent downstream processing. Tests are added to verify that thrown exceptions result in failed health checks while allowing the application to continue operating.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Handle healthcheck errors gracefully' clearly summarizes the main change: adding error handling to prevent unhandled exceptions from health check failures.
Description check	✅ Passed	The description includes all required sections from the template: Changes section explaining the purpose, and Checklist with all items marked complete.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/handle-errors-gracefully

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (2)

lib/plugins/healthcheck/startupHealthcheckPlugin.ts (1)

8-17: Consider extracting shared executeHealthCheck helper.

The same try/catch + isError pattern is duplicated inline in commonHealthcheckPlugin.ts (lines 100-105) and publicHealthcheckPlugin.ts (lines 51-55). This helper could be moved to healthcheckCommons.ts and reused across all async healthcheck plugins to reduce duplication.

♻️ Potential shared helper in healthcheckCommons.ts

// In healthcheckCommons.ts
import { type Either, isError } from '@lokalise/node-core'
import type { AnyFastifyInstance } from '../pluginsCommon.js'

export async function executeHealthCheck(
  checker: HealthChecker,
  app: AnyFastifyInstance,
): Promise<Either<Error, true>> {
  try {
    return await checker(app)
  } catch (err) {
    return { error: isError(err) ? err : new Error(String(err)) }
  }
}

Also applies to: 34-34

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@lib/plugins/healthcheck/startupHealthcheckPlugin.ts` around lines 8 - 17,
Extract the duplicated try/catch + isError pattern into a shared helper named
executeHealthCheck placed in healthcheckCommons.ts and reuse it from
commonHealthcheckPlugin.ts and publicHealthcheckPlugin.ts; specifically, create
executeHealthCheck(checker: HealthChecker, app: AnyFastifyInstance):
Promise<Either<Error, true>> that wraps await checker(app) and on throw returns
{ error: isError(err) ? err : new Error(String(err)) }, then import and call
this helper from the existing locations (replace the inline logic in
commonHealthcheckPlugin.ts and publicHealthcheckPlugin.ts with calls to
executeHealthCheck).

lib/plugins/healthcheck/commonHealthcheckPlugin.spec.ts (1)

324-351: Good test coverage for exception handling.

The test correctly validates that a throwing optional checker results in PARTIALLY_HEALTHY status. Consider adding a complementary test case where a mandatory checker throws to verify it returns 500 with FAIL heartbeat, ensuring both paths are covered.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@lib/plugins/healthcheck/commonHealthcheckPlugin.spec.ts` around lines 324 -
351, Add a complementary test that verifies a throwing mandatory checker causes
a 500 response and FAIL heartbeat: call initApp with healthChecks including a
checker that throws (use throwingHealthcheckChecker) and set isMandatory: true
for that check, then perform the GET on PRIVATE_ENDPOINT and assert
response.statusCode is 500 and response.json() contains heartbeat 'FAIL' and the
failing check set to 'FAIL'; place the test alongside the existing 'handles
checker that throws an error as a failed healthcheck' spec so it covers the
mandatory-error path.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@lib/plugins/healthcheck/commonHealthcheckPlugin.spec.ts`:
- Around line 324-351: Add a complementary test that verifies a throwing
mandatory checker causes a 500 response and FAIL heartbeat: call initApp with
healthChecks including a checker that throws (use throwingHealthcheckChecker)
and set isMandatory: true for that check, then perform the GET on
PRIVATE_ENDPOINT and assert response.statusCode is 500 and response.json()
contains heartbeat 'FAIL' and the failing check set to 'FAIL'; place the test
alongside the existing 'handles checker that throws an error as a failed
healthcheck' spec so it covers the mandatory-error path.

In `@lib/plugins/healthcheck/startupHealthcheckPlugin.ts`:
- Around line 8-17: Extract the duplicated try/catch + isError pattern into a
shared helper named executeHealthCheck placed in healthcheckCommons.ts and reuse
it from commonHealthcheckPlugin.ts and publicHealthcheckPlugin.ts; specifically,
create executeHealthCheck(checker: HealthChecker, app: AnyFastifyInstance):
Promise<Either<Error, true>> that wraps await checker(app) and on throw returns
{ error: isError(err) ? err : new Error(String(err)) }, then import and call
this helper from the existing locations (replace the inline logic in
commonHealthcheckPlugin.ts and publicHealthcheckPlugin.ts with calls to
executeHealthCheck).

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository: lokalise/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: b0f9ef45-7128-4e01-9d8c-37f348db2dab

📥 Commits

Reviewing files that changed from the base of the PR and between 9dda393 and 394720a.

📒 Files selected for processing (8)

• lib/plugins/healthcheck/commonHealthcheckPlugin.spec.ts
• lib/plugins/healthcheck/commonHealthcheckPlugin.ts
• lib/plugins/healthcheck/commonSyncHealthcheckPlugin.spec.ts
• lib/plugins/healthcheck/commonSyncHealthcheckPlugin.ts
• lib/plugins/healthcheck/publicHealthcheckPlugin.spec.ts
• lib/plugins/healthcheck/publicHealthcheckPlugin.ts
• lib/plugins/healthcheck/startupHealthcheckPlugin.spec.ts
• lib/plugins/healthcheck/startupHealthcheckPlugin.ts