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

use with nestjs #70

Closed
xmlking opened this issue Jul 30, 2018 · 16 comments
Closed

use with nestjs #70

xmlking opened this issue Jul 30, 2018 · 16 comments

Comments

@xmlking
Copy link

xmlking commented Jul 30, 2018

if it possible to use terminus with https://nestjs.com/ ?
i am interested in health checks

@gergelyke
Copy link
Collaborator

hi @xmlking !

i am not familiar with that framework, but if they expose the http server, you should be able to wrap that with terminus.

if you figure it out, can you send a PR that adds an example with nest?

@weeco
Copy link

weeco commented Aug 24, 2018

@xmlking See nestjs/nest#966

@gergelyke
Copy link
Collaborator

also, #75

@BrunnerLivio
Copy link
Contributor

BrunnerLivio commented Sep 26, 2018

Hello guys,

I am currently the maintainer of nest/terminus. We're running in some issues which heavily involves the general codebase of @godaddy/terminus.

The problems are described on nest/terminus#2

The main problem is, we want to support subsystem of a health check e.g.

{  
  "status":"DOWN",
  "reason":"fortniteClient is DOWN",
  "diskSpace":{  
    "status":"UP",
    "free":56443746,
    "threshold":1345660
  },
  "fortniteClient":{  
    "status":"DOWN",
    "error": {....}
  }
}

So for example when the status of a subsystem (e.g. forniteClient or diskSpace) equals "down", the status of the health response should also be "down". Also if a system is down, it should be able to print a detailed error message. These things are currently not possible with @godaddy/terminus.

If these kind of requirements are out of your scope, we would have to go with our own implementation..

@gergelyke
Copy link
Collaborator

@BrunnerLivio can you help me understand what API changes you'd need? I am happy to incorporate them!

@gergelyke
Copy link
Collaborator

Would it be enough, if the rejection error message from the healthchecks be part of the error message? or would you like to have the option to provide some kind of template for the response?

@weeco
Copy link

weeco commented Sep 26, 2018

Having the possibility to return objects would be nice because that certainly helps to understand why exactly the status is down (e. g. only 100mb diskspace left, but you need 200mb for your service to operate).

@gergelyke
Copy link
Collaborator

Sounds good - I can draft up a PR today to address this

@BrunnerLivio
Copy link
Contributor

BrunnerLivio commented Sep 26, 2018

@gergelyke
Thanks a lot for your quick response & actions.

One problem I still see is, who verifies if the overall status is “up” or “down”?

Easiest way is if the user can explicitly say whats the overall status. So he can decide if Subsystem A is up but Subsystem B is down, if that would be crucial for the overall system or not.

At the moment a User can only say if the overall status is up or down by throwing an error. Probably we need to find another solution for this, because the user also needs to send the status of the subsystems in addition as desribed before.

I think this could lead into breaking changes. Do you have an idea how you want to solve this?

@gergelyke
Copy link
Collaborator

Just to make sure we are on the same page - no matter which check fails, you want to fail the healthcheck, but you want to have the result from all the checks. Is that correct?

@BrunnerLivio
Copy link
Contributor

BrunnerLivio commented Sep 26, 2018

@gergelyke exactly

@gergelyke
Copy link
Collaborator

how about something like this: #81?

if you like the idea, I can expand on the docs / examples

@gergelyke
Copy link
Collaborator

@gergelyke
Copy link
Collaborator

@BrunnerLivio can you give it a try and let me know what that worked?

@BrunnerLivio
Copy link
Contributor

BrunnerLivio commented Sep 28, 2018

@gergelyke yup got it working, thanks a lot!

Only problem I currently have is the typings. Seems like they're not updated yet. I can do a PR for that. I think since this release #75 is doable.

EDIT: There you go #82

@gergelyke
Copy link
Collaborator

released under https://github.com/godaddy/terminus/releases/tag/v3.0.1

thanks a lot for the contrib!

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

4 participants