Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HttpHealthIndicator.responseCheck not handles network errors #2395

Closed
2 of 4 tasks
yedlosh opened this issue Sep 27, 2023 · 1 comment
Closed
2 of 4 tasks

HttpHealthIndicator.responseCheck not handles network errors #2395

yedlosh opened this issue Sep 27, 2023 · 1 comment

Comments

@yedlosh
Copy link

yedlosh commented Sep 27, 2023

Is there an existing issue for this?

  • I have searched the existing issues

Current behavior

Given I use a HttpHealthIndicator.pingCheck health indicator,
When a network error occurs for the checked url,
Then it is handled and the health check route reports "down" for the given key.

When I use a HttpHealthIndicator.responseCheck health indicator,
When a network error occurs for the checked url,
Then it is not handled, and the health check route throws a 500 internal error.

Minimum reproduction code

@Get()
@HealthCheck()
check() {
  // There is nothing running on localhost:1234, so it will throw a "connect ECONNREFUSED ::1:1234"
  const hostWhichWontConnect = 'http://localhost:1234'

  return this.health.check([
    () => this.http.pingCheck('this will report status: "down"', hostWhichWontConnect),
    () => this.http.responseCheck('this will cause a 500 internal error', hostWhichWontConnect, (res) => res.status === 200),
  ])
}

Steps to reproduce

No response

Expected behavior

I'd expect a HttpHealthIndicator.responseCheck health indicator to not throw an error if it results from the checked network call being made, and instead reporting the result with status: "down".

If the error originates from the user defined response callback however, then not handling it is a reasonable behaviour.

Package version

10.1.1

NestJS version

10.2.6

Node.js version

18.18.0

In which operating systems have you tested?

  • macOS
  • Windows
  • Linux

Other

We've faced this issue when one service which was being checked had some network issues, which resulted in error:
Client network socket disconnected before secure TLS connection was established
..and the health check route started crashing instead of reporting the culprit as down.

chamsou123 pushed a commit to chamsou123/terminus that referenced this issue Nov 27, 2023
BrunnerLivio pushed a commit that referenced this issue Jan 26, 2024
Co-authored-by: chamsou123 <agounichams@gmail.com>
@BrunnerLivio
Copy link
Member

BrunnerLivio commented Jan 26, 2024

Released with v10.2.1 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants