-
Notifications
You must be signed in to change notification settings - Fork 6.3k
Added /healthz JSON response for heartbeat data. #1940 #1984
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great! All my comments but one are just from the linter.
As for the new release, we should have one out this week.
One interesting note is that if you use a browser then it requests the service worker which triggers a heart beat. But since this is going to be used by things that aren't browsers I think it should be fine.
I'm so sorry about this! Deleted the dev branch was gonna switch this back to base on master but GitHub doesn't let you do that.... After you delete the PR base.
🤦
Please open a new PR with these changes! Once again, I'm so sorry!
Nvm, got it!
61fab6b to
54ddd8b
Compare
Figured out how to run the linter 😅
Hopefully this will make it into 3.5.1! Love the project, I might try to tackle some more milestones, time permitting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good from my glance. Thanks @JacobGo!
Everything looks great. Thank you for the contribution!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this endpoint work regardless of whether or not we're authenticated? @code-asher
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah crap you're right.
Uh oh!
There was an error while loading. Please reload this page.
I added a JSON object with heartbeat information by passing the server's Heart to the new HealthHttpProvider. I wasn't quite sure what status code to use. I was considering
503for expired health checks but settled on200with state in the response content.I haven't written TypeScript before, perhaps a type for the
resultobject would be better?I made sure not to heartbeat for onRequest with the
/healthzroute.Closes #1940
Side note: when can we expect a new release? 3.4.1 is two months old and still has a broken heart 🙃 (zombie connections fixed by ebef18d)