Skip to content

feat(backend, auth): /healthz fails if a database is not connected #3717

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.

Sign up for GitHub

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

feat(backend, auth): /healthz fails if a database is not connected #3717

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d68911f
feat(backend, auth): /healthz fails if a database is not connected
njlie Oct 23, 2025
fac692b
chore: remove comments
njlie Oct 24, 2025
1314802
chore: cleanup
njlie Oct 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 30 additions & 6 deletions packages/auth/src/app.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,8 +268,16 @@ export class App {

const router = new Router<DefaultState, AppContext>()
router.use(bodyParser())
router.get('/healthz', (ctx: AppContext): void => {
ctx.status = 200
router.get('/healthz', async (ctx: AppContext): Promise<void> => {
const redis = await this.container.use('redis')
const knex = await this.container.use('knex')
try {
await redis.ping()
await knex.raw('SELECT 1')
ctx.status = 200
} catch (e) {
ctx.status = 500
}
})
router.use(gnapServerErrorMiddleware)

Expand Down Expand Up @@ -413,8 +421,16 @@ export class App {

const router = new Router<DefaultState, AppContext>()
router.use(bodyParser())
router.get('/healthz', (ctx: AppContext): void => {
ctx.status = 200
router.get('/healthz', async (ctx: AppContext): Promise<void> => {
const redis = await this.container.use('redis')
const knex = await this.container.use('knex')
try {
await redis.ping()
await knex.raw('SELECT 1')
ctx.status = 200
} catch (e) {
ctx.status = 500
}
})

const accessTokenRoutes = await this.container.use('accessTokenRoutes')
Expand Down Expand Up @@ -505,8 +521,16 @@ export class App {

koa.use(errorHandler)

router.get('/healthz', (ctx: AppContext): void => {
ctx.status = 200
router.get('/healthz', async (ctx: AppContext): Promise<void> => {
const redis = await this.container.use('redis')
const knex = await this.container.use('knex')
try {
await redis.ping()
await knex.raw('SELECT 1')
ctx.status = 200
} catch (e) {
ctx.status = 500
}
})

const tenantRoutes = await this.container.use('tenantRoutes')
Expand Down
22 changes: 18 additions & 4 deletions packages/backend/src/app.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,6 @@ export class App {
*/
public async boot(): Promise<void> {
this.config = await this.container.use('config')
this.logger = await this.container.use('logger')
Copy link
Contributor

@BlairCurrey BlairCurrey Oct 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this removal seems to be resulting in the 500 errors on backend/integration/performance tests.

I had assumed the 500 errors were coming from the new healthcheck but as I traced it that didnt make much sense and reverting the healthz changes didnt fix the issue.

I guess this was probably removed in error, and the non-null assertion on the logger class property didnt help. We can simply return it.

It's a small thing, but we could probably just assign logger/config in the constructor to bypass the need for the non-null assertion. That would require changing the container singletons to not (unnecessarily?) return promises. Which should be fine. We have this pattern of making them return promises but it seems entirely unnecessary for logger, config, and most others.

export interface AppServices {
  logger: Promise<Logger>
  telemetry: Promise<TelemetryService>
  internalRatesService: Promise<RatesService>
  knex: Promise<Knex>
  axios: Promise<AxiosInstance>
  config: Promise<IAppConfig>
  httpTokenService: Promise<HttpTokenService>
  ... etc

Then that makes it looks like maybe we can even just move the entire boot function to the constructor, since it no longer needs to be async. That would also change some timing though (currently we do constructor, then other stuff, then boot) but I dont think that should matter. Guess it's a little can of worms and we should at least just return the line here.


// Workers are in the way during tests.
if (this.config.env !== 'test') {
Expand Down Expand Up @@ -403,7 +402,14 @@ export class App {
next: Koa.Next
): Promise<void> => {
if (ctx.path === '/healthz') {
ctx.status = 200
const knex = await this.container.use('knex')
try {
await redis.ping()
await knex.raw('SELECT 1')
ctx.status = 200
} catch (err) {
ctx.status = 500
}
} else if (ctx.path !== '/graphql') {
ctx.status = 404
} else {
Expand Down Expand Up @@ -479,8 +485,16 @@ export class App {

const router = new Router<DefaultState, AppContext>()
router.use(bodyParser())
router.get('/healthz', (ctx: AppContext): void => {
ctx.status = 200
router.get('/healthz', async (ctx: AppContext): Promise<void> => {
const redis = await ctx.container.use('redis')
const knex = await ctx.container.use('knex')
try {
await redis.ping()
await knex.raw('SELECT 1')
ctx.status = 200
} catch (e) {
ctx.status = 500
}
})
router.use(openPaymentsServerErrorMiddleware)

Expand Down
Loading