7.0.0 proposal - Removal of @godaddy/terminus

[BrunnerLivio]

Limitations

At the moment @nestjs/terminus is underlying some harsh limitations.

Some of which are:

• Swagger compatibility Swagger compatibility #32
• Compatibility with middlewares Compatibility with middlewares #33
• OnApplicationShutdown hook OnApplicationShutdown hook #312
• Get Health Status programmatically Get Health Status programmatically #9

The named issues are hardly or impossible to fix because of restrictions of the underlying @godaddy/terminus library. Surely, it would be possible to create pull requests to the underlying library, so the named issues would be possible. Though in that case there would not be much benefit of relying on the library at all.

Proposal

What I am proposing is the removal of the dependent @godaddy/terminus library.

The main points which is currently handled by @godaddy/terminus and would need to take over to the codebase of @nestjs/terminus:

• Register the health check route to the http server (enabler of Swagger compatibility #32, Compatibility with middlewares #33)
  @godaddy/terminus registers health checks directly to the underlying http instance.
  The problem with that approach is, NestJS "does not know" these routes exist.
  Currently it is not possible to let NestJS know about these routes, and therefore there
  is no way to add Swagger, Middleware or else compatibility. Related issue nestjs/nest#1438.

• Handle life-cycle management (enabler of OnApplicationShutdown hook #312)
  @godaddy/terminus registers listeners to system signals. Unfortunately NestJS should need
  to handle the system signals handling by itself, using onApplicationShutdown or beforeApplicationShutdown.

• Handle the health indicator execution manager (enabler of Get Health Status programmatically #9)
  Currently @godaddy/terminus does not allow to compute the current outcome of
  the health status, without requesting the health check url.

• Handle response codes on failure / succcess.
  No benefit in handling it ourselves, but it still allows for flexibility in the future.

Breaking changes

Most users most probably won't need to migrate. The only thing which would be beneficial for all users is to remove the @godaddy/terminus so your project does not have an unneeded dependency (npm uninstall @godaddy/terminus).

For all users who have built custom health indicators will need to change how you had to handle unhealthy response codes. Essentially it boils down to changing the following line:

import { HealthCheckError } from '@godaddy/terminus';

to

import { HealthCheckError } from '@nestjs/terminus';

and that's it.

Since this is a breaking change, I want to align with you before-hand. Please let me know whether my strategy is sufficient for your project. Maybe you have a better idea? Let me know :)

[iangregsondev]

@BrunnerLivio I am using 2 custom indicators that i created (using the doc recipes), so doing the above seems fine for me.

Here is an example of an Indicator right now

@Injectable()
export class ExchangeRateRedisIndicator extends HealthIndicator {
  constructor(private readonly serverSharedExchangeRateRedisService: ServerSharedExchangeRateRedisService) {
    super()
  }
  async isHealthy(): Promise<HealthIndicatorResult> {
    const healthy = !this.serverSharedExchangeRateRedisService.hasConnectionError()
    let message: string | undefined = undefined
    if (!healthy) {
      message = this.serverSharedExchangeRateRedisService.connectionErrorMessage()
    }
    const result = this.getStatus("exchange_rate_redis_connection", healthy, { message })

    if (healthy) {
      return result
    }
    throw new HealthCheckError("ExchangeRateRedisIndicator failed", result)
  }
}

I am not using anything special apart from what is discussed in the recipes.

Thanks.

[eropple]

I like the idea of ditching @godaddy/terminus, but given the breaking changes you suggest I'd suggest not calling this terminus at all--that may lead to confusion. @nestjs/health-checks, perhaps?

[eropple]

Also, pursuant to these changes I'd like to work with you on surfacing information from this library into @eropple/nestjs-openapi3; please feel free to reach out to me via email or Discord (Ed#7871).

[BrunnerLivio]

@eropple

given the breaking changes you suggest I'd suggest not calling this terminus at all--that may lead to confusion. @nestjs/health-checks, perhaps?

@nestjs/terminus does more than just provide health checks in the future. It will provide a service or similar for handling graceful shutdown. In my opinion we should keep the name to minimize the migration steps.

---

To keep you aligned with the strategy & status of this proposal

• Supposedly no blockers present (beforeApplicationShutdown-feature has been merged on nestjs/nest, so that would enable this proposal)
• To be aligned with the other packages from NestJS we will release this major change once 7.0.0 has been release on nestjs/nest. Unfortunately we do not have a timeline for this yet, but most probably it will happen soon, since we already have proposed breaking changes on nestjs/nest and nestjs/swagger.
• Because there is no timeline for 7.0.0, I will try to push forward on nestjs/terminus and release a prerelease which runs on 6.x.x, but already includes the breaking changes. (e.g. @nestjs/terminus@next)

Currently I am fully consumed by my day-to-day job unfortunately. I try to take a day off, or work on weekends to make this happening though. Feel free to contact me on Discord, if you want to contribute to this proposal, but I think the easiest way to implement the initial steps for this proposal would require me.

I'll try to be as transparent as possible with this breaking change, so it hopefully runs down smoothly. 🍺

[BrunnerLivio]

Resolved in 7.0.0 🎉