From 138f11393e210198c4609aa757a7d023a5ffba11 Mon Sep 17 00:00:00 2001
From: Iain Sproat <68657+iainsproat@users.noreply.github.com>
Date: Wed, 25 Sep 2024 15:39:44 +0100
Subject: [PATCH 01/14] chore(server): graceful shutdown - stop() on the apollo
 server should be called

---
 packages/server/app.ts        | 21 ++++++++++++++-------
 packages/server/bin/ts-www    |  2 +-
 packages/server/bin/www       |  2 +-
 packages/server/test/hooks.js |  2 +-
 4 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/packages/server/app.ts b/packages/server/app.ts
index 25840d341a..ca5d58c50f 100644
--- a/packages/server/app.ts
+++ b/packages/server/app.ts
@@ -420,8 +420,13 @@ export async function init() {
   return { app, graphqlServer, server, subscriptionServer }
 }
 
-export async function shutdown(): Promise<void> {
-  await ModulesSetup.shutdown()
+export async function shutdown(params: {
+  app: Express
+  graphqlServer: ApolloServer<GraphQLContext>
+}): Promise<void> {
+  const { graphqlServer } = params
+  graphqlServer.stop()
+  ModulesSetup.shutdown()
 }
 
 const shouldUseFrontendProxy = () =>
@@ -447,11 +452,13 @@ async function createFrontendProxy() {
 /**
  * Starts a http server, hoisting the express app to it.
  */
-export async function startHttp(
-  server: http.Server,
-  app: Express,
+export async function startHttp(params: {
+  server: http.Server
+  app: Express
+  graphqlServer: ApolloServer<GraphQLContext>
   customPortOverride?: number
-) {
+}) {
+  const { server, app, graphqlServer, customPortOverride } = params
   let bindAddress = process.env.BIND_ADDRESS || '127.0.0.1'
   let port = process.env.PORT ? toNumber(process.env.PORT) : 3000
 
@@ -481,7 +488,7 @@ export async function startHttp(
       shutdownLogger.info('Shutting down (signal received)...')
     },
     onSignal: async () => {
-      await shutdown()
+      await shutdown({ app, graphqlServer })
     },
     onShutdown: () => {
       shutdownLogger.info('Shutdown completed')
diff --git a/packages/server/bin/ts-www b/packages/server/bin/ts-www
index cbddd719df..e0f2d44738 100755
--- a/packages/server/bin/ts-www
+++ b/packages/server/bin/ts-www
@@ -11,7 +11,7 @@ const { logger } = require('../logging/logging')
 const { init, startHttp } = require('../app')
 
 init()
-  .then(({ app, server }) => startHttp(server, app))
+  .then(({ app, graphqlServer, server }) => startHttp({server, app, graphqlServer}))
   .catch((err) => {
     logger.error(err, 'Failed to start server. Exiting with non-zero exit code...')
 
diff --git a/packages/server/bin/www b/packages/server/bin/www
index 3309271a43..9b4bbd2741 100755
--- a/packages/server/bin/www
+++ b/packages/server/bin/www
@@ -5,7 +5,7 @@ const { logger } = require('../dist/logging/logging')
 const { init, startHttp } = require('../dist/app')
 
 init()
-  .then(({ app, server }) => startHttp(server, app))
+  .then(({ app, graphqlServer, server }) => startHttp({server, app, graphqlServer}))
   .catch((err) => {
     logger.error(err, 'Failed to start server. Exiting with non-zero exit code...')
 
diff --git a/packages/server/test/hooks.js b/packages/server/test/hooks.js
index 713d1282b6..c537c448e4 100644
--- a/packages/server/test/hooks.js
+++ b/packages/server/test/hooks.js
@@ -51,7 +51,7 @@ exports.truncateTables = async (tableNames) => {
  * @param {import('express').Express} app
  */
 const initializeTestServer = async (server, app) => {
-  await startHttp(server, app, 0)
+  await startHttp({ server, app, customPortOverride: 0 })
 
   await once(app, 'appStarted')
   const port = server.address().port

From 1e73a998c6f98d1cd26d9e2063720b6b6bb5f7a0 Mon Sep 17 00:00:00 2001
From: Iain Sproat <68657+iainsproat@users.noreply.github.com>
Date: Wed, 25 Sep 2024 16:07:28 +0100
Subject: [PATCH 02/14] chore(server): gracefully drain apollo server

---
 packages/server/app.ts | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/packages/server/app.ts b/packages/server/app.ts
index ca5d58c50f..7c37902ea7 100644
--- a/packages/server/app.ts
+++ b/packages/server/app.ts
@@ -12,7 +12,12 @@ import cookieParser from 'cookie-parser'
 
 import { createTerminus } from '@godaddy/terminus'
 import Logging from '@/logging'
-import { startupLogger, shutdownLogger, subscriptionLogger } from '@/logging/logging'
+import {
+  startupLogger,
+  shutdownLogger,
+  subscriptionLogger,
+  graphqlLogger
+} from '@/logging/logging'
 import {
   DetermineRequestIdMiddleware,
   LoggingExpressMiddleware,
@@ -27,6 +32,7 @@ import { expressMiddleware } from '@apollo/server/express4'
 import { ApolloServerPluginLandingPageLocalDefault } from '@apollo/server/plugin/landingPage/default'
 import { ApolloServerPluginUsageReporting } from '@apollo/server/plugin/usageReporting'
 import { ApolloServerPluginUsageReportingDisabled } from '@apollo/server/plugin/disabled'
+import { ApolloServerPluginDrainHttpServer } from '@apollo/server/plugin/drainHttpServer'
 
 import type { ConnectionContext, ExecutionParams } from 'subscriptions-transport-ws'
 import { SubscriptionServer } from 'subscriptions-transport-ws'
@@ -289,9 +295,11 @@ function buildApolloSubscriptionServer(server: http.Server): SubscriptionServer
  */
 export async function buildApolloServer(options?: {
   subscriptionServer?: SubscriptionServer
+  httpServer?: http.Server
 }): Promise<ApolloServer<GraphQLContext>> {
   const includeStacktraceInErrorResponses = isDevEnv() || isTestEnv()
   const subscriptionServer = options?.subscriptionServer
+  const httpServer = options?.httpServer
   const schema = ModulesSetup.graphSchema(await buildMocksConfig())
 
   const server = new ApolloServer({
@@ -323,14 +331,18 @@ export async function buildApolloServer(options?: {
               sendHeaders: { all: true }
             })
           ]
-        : [ApolloServerPluginUsageReportingDisabled()])
+        : [ApolloServerPluginUsageReportingDisabled()]),
+      ...(!!httpServer ? [ApolloServerPluginDrainHttpServer({ httpServer })] : [])
     ],
     introspection: true,
     cache: 'bounded',
     persistedQueries: false,
     csrfPrevention: true,
     formatError: buildErrorFormatter({ includeStacktraceInErrorResponses }),
-    includeStacktraceInErrorResponses
+    includeStacktraceInErrorResponses,
+    status400ForVariableCoercionErrors: true,
+    stopOnTerminationSignals: false, // handled by terminus and shutdown function
+    logger: graphqlLogger
   })
   await server.start()
 
@@ -395,6 +407,7 @@ export async function init() {
 
   // Initialize graphql server
   graphqlServer = await buildApolloServer({
+    httpServer: server,
     subscriptionServer
   })
   app.use(
@@ -425,8 +438,8 @@ export async function shutdown(params: {
   graphqlServer: ApolloServer<GraphQLContext>
 }): Promise<void> {
   const { graphqlServer } = params
-  graphqlServer.stop()
-  ModulesSetup.shutdown()
+  await graphqlServer.stop()
+  await ModulesSetup.shutdown()
 }
 
 const shouldUseFrontendProxy = () =>

From 034152c6d4c2f91d4a07665cff6db02b0a366d70 Mon Sep 17 00:00:00 2001
From: Iain Sproat <68657+iainsproat@users.noreply.github.com>
Date: Thu, 26 Sep 2024 10:15:57 +0100
Subject: [PATCH 03/14] Allow grace period to be configured

---
 packages/server/app.ts                        | 34 +++++++++++++------
 .../modules/shared/helpers/envHelper.ts       |  4 +++
 2 files changed, 27 insertions(+), 11 deletions(-)

diff --git a/packages/server/app.ts b/packages/server/app.ts
index 7c37902ea7..5aa6932d00 100644
--- a/packages/server/app.ts
+++ b/packages/server/app.ts
@@ -47,7 +47,8 @@ import {
   isTestEnv,
   useNewFrontend,
   isApolloMonitoringEnabled,
-  enableMixpanel
+  enableMixpanel,
+  shutdownTimeoutSeconds
 } from '@/modules/shared/helpers/envHelper'
 import * as ModulesSetup from '@/modules'
 import { GraphQLContext, Optional } from '@/modules/shared/helpers/typeHelper'
@@ -332,7 +333,14 @@ export async function buildApolloServer(options?: {
             })
           ]
         : [ApolloServerPluginUsageReportingDisabled()]),
-      ...(!!httpServer ? [ApolloServerPluginDrainHttpServer({ httpServer })] : [])
+      ...(!!httpServer
+        ? [
+            ApolloServerPluginDrainHttpServer({
+              httpServer,
+              stopGracePeriodMillis: shutdownTimeoutSeconds() * 1000
+            })
+          ]
+        : [])
     ],
     introspection: true,
     cache: 'bounded',
@@ -433,12 +441,7 @@ export async function init() {
   return { app, graphqlServer, server, subscriptionServer }
 }
 
-export async function shutdown(params: {
-  app: Express
-  graphqlServer: ApolloServer<GraphQLContext>
-}): Promise<void> {
-  const { graphqlServer } = params
-  await graphqlServer.stop()
+export async function shutdown(): Promise<void> {
   await ModulesSetup.shutdown()
 }
 
@@ -494,18 +497,27 @@ export async function startHttp(params: {
   app.set('port', port)
 
   // large timeout to allow large downloads on slow connections to finish
-  createTerminus(server, {
+  createTerminus(null, {
     signals: ['SIGTERM', 'SIGINT'],
-    timeout: 5 * 60 * 1000,
+    timeout: shutdownTimeoutSeconds() * 1000,
     beforeShutdown: async () => {
       shutdownLogger.info('Shutting down (signal received)...')
+      await graphqlServer.stop() //handles draining of connections on the server
     },
     onSignal: async () => {
-      await shutdown({ app, graphqlServer })
+      // called after server.stop() resolves
+      await shutdown()
     },
     onShutdown: () => {
       shutdownLogger.info('Shutdown completed')
       process.exit(0)
+    },
+    logger: (message, err) => {
+      if (err) {
+        shutdownLogger.error({ err }, message)
+      } else {
+        shutdownLogger.info(message)
+      }
     }
   })
 
diff --git a/packages/server/modules/shared/helpers/envHelper.ts b/packages/server/modules/shared/helpers/envHelper.ts
index 71c80b0331..e1233ddd5e 100644
--- a/packages/server/modules/shared/helpers/envHelper.ts
+++ b/packages/server/modules/shared/helpers/envHelper.ts
@@ -413,3 +413,7 @@ export function highFrequencyMetricsCollectionPeriodMs() {
 export function maximumObjectUploadFileSizeMb() {
   return getIntFromEnv('MAX_OBJECT_UPLOAD_FILE_SIZE_MB', '100')
 }
+
+export function shutdownTimeoutSeconds() {
+  return getIntFromEnv('SHUTDOWN_TIMEOUT_SECONDS', `${5 * 60}`)
+}

From 1ccc67780d1b5f6785e28fdde3c07557b7df56ff Mon Sep 17 00:00:00 2001
From: Iain Sproat <68657+iainsproat@users.noreply.github.com>
Date: Thu, 26 Sep 2024 18:49:02 +0100
Subject: [PATCH 04/14] Terminus manages the readiness and liveness endpoints

---
 packages/server/app.ts                        |  26 ++-
 packages/server/bin/ts-www                    |   2 +-
 packages/server/bin/www                       |   2 +-
 packages/server/modules/core/index.ts         |   4 -
 .../{core/rest => healthchecks}/health.ts     | 150 +++++++-----------
 packages/server/modules/healthchecks/index.ts |  54 +++++++
 packages/server/modules/index.ts              |   1 +
 .../modules/notifications/services/queue.ts   |   7 +-
 .../modules/shared/errors/ensureError.ts      |  11 ++
 .../modules/shared/helpers/envHelper.ts       |   2 +-
 10 files changed, 137 insertions(+), 122 deletions(-)
 rename packages/server/modules/{core/rest => healthchecks}/health.ts (54%)
 create mode 100644 packages/server/modules/healthchecks/index.ts
 create mode 100644 packages/server/modules/shared/errors/ensureError.ts

diff --git a/packages/server/app.ts b/packages/server/app.ts
index 9e124e0ca8..6eea565070 100644
--- a/packages/server/app.ts
+++ b/packages/server/app.ts
@@ -32,7 +32,6 @@ import { expressMiddleware } from '@apollo/server/express4'
 import { ApolloServerPluginLandingPageLocalDefault } from '@apollo/server/plugin/landingPage/default'
 import { ApolloServerPluginUsageReporting } from '@apollo/server/plugin/usageReporting'
 import { ApolloServerPluginUsageReportingDisabled } from '@apollo/server/plugin/disabled'
-import { ApolloServerPluginDrainHttpServer } from '@apollo/server/plugin/drainHttpServer'
 
 import type { ConnectionContext, ExecutionParams } from 'subscriptions-transport-ws'
 import { SubscriptionServer } from 'subscriptions-transport-ws'
@@ -72,6 +71,7 @@ import { BaseError, ForbiddenError } from '@/modules/shared/errors'
 import { loggingPlugin } from '@/modules/core/graph/plugins/logging'
 import { shouldLogAsInfoLevel } from '@/logging/graphqlError'
 import { getUser } from '@/modules/core/repositories/users'
+import { isAlive, isReady } from '@/modules/healthchecks'
 
 const GRAPHQL_PATH = '/graphql'
 
@@ -301,7 +301,6 @@ export async function buildApolloServer(options?: {
 }): Promise<ApolloServer<GraphQLContext>> {
   const includeStacktraceInErrorResponses = isDevEnv() || isTestEnv()
   const subscriptionServer = options?.subscriptionServer
-  const httpServer = options?.httpServer
   const schema = ModulesSetup.graphSchema(await buildMocksConfig())
 
   const server = new ApolloServer({
@@ -333,15 +332,7 @@ export async function buildApolloServer(options?: {
               sendHeaders: { all: true }
             })
           ]
-        : [ApolloServerPluginUsageReportingDisabled()]),
-      ...(!!httpServer
-        ? [
-            ApolloServerPluginDrainHttpServer({
-              httpServer,
-              stopGracePeriodMillis: shutdownTimeoutSeconds() * 1000
-            })
-          ]
-        : [])
+        : [ApolloServerPluginUsageReportingDisabled()])
     ],
     introspection: true,
     cache: 'bounded',
@@ -416,7 +407,6 @@ export async function init() {
 
   // Initialize graphql server
   graphqlServer = await buildApolloServer({
-    httpServer: server,
     subscriptionServer
   })
   app.use(
@@ -472,10 +462,9 @@ async function createFrontendProxy() {
 export async function startHttp(params: {
   server: http.Server
   app: Express
-  graphqlServer: ApolloServer<GraphQLContext>
   customPortOverride?: number
 }) {
-  const { server, app, graphqlServer, customPortOverride } = params
+  const { server, app, customPortOverride } = params
   let bindAddress = process.env.BIND_ADDRESS || '127.0.0.1'
   let port = process.env.PORT ? toNumber(process.env.PORT) : 3000
 
@@ -498,21 +487,24 @@ export async function startHttp(params: {
   app.set('port', port)
 
   // large timeout to allow large downloads on slow connections to finish
-  createTerminus(null, {
+  createTerminus(server, {
     signals: ['SIGTERM', 'SIGINT'],
     timeout: shutdownTimeoutSeconds() * 1000,
     beforeShutdown: async () => {
       shutdownLogger.info('Shutting down (signal received)...')
-      await graphqlServer.stop() //handles draining of connections on the server
     },
     onSignal: async () => {
-      // called after server.stop() resolves
       await shutdown()
     },
     onShutdown: () => {
       shutdownLogger.info('Shutdown completed')
       process.exit(0)
     },
+    healthChecks: {
+      '/readiness': isReady,
+      '/liveness': isAlive,
+      verbatim: true
+    },
     logger: (message, err) => {
       if (err) {
         shutdownLogger.error({ err }, message)
diff --git a/packages/server/bin/ts-www b/packages/server/bin/ts-www
index e0f2d44738..aebc00b5d7 100755
--- a/packages/server/bin/ts-www
+++ b/packages/server/bin/ts-www
@@ -11,7 +11,7 @@ const { logger } = require('../logging/logging')
 const { init, startHttp } = require('../app')
 
 init()
-  .then(({ app, graphqlServer, server }) => startHttp({server, app, graphqlServer}))
+  .then(({ app, server }) => startHttp({server, app}))
   .catch((err) => {
     logger.error(err, 'Failed to start server. Exiting with non-zero exit code...')
 
diff --git a/packages/server/bin/www b/packages/server/bin/www
index 9b4bbd2741..0dc0089461 100755
--- a/packages/server/bin/www
+++ b/packages/server/bin/www
@@ -5,7 +5,7 @@ const { logger } = require('../dist/logging/logging')
 const { init, startHttp } = require('../dist/app')
 
 init()
-  .then(({ app, graphqlServer, server }) => startHttp({server, app, graphqlServer}))
+  .then(({ app, server }) => startHttp({server, app}))
   .catch((err) => {
     logger.error(err, 'Failed to start server. Exiting with non-zero exit code...')
 
diff --git a/packages/server/modules/core/index.ts b/packages/server/modules/core/index.ts
index fe2b81ab12..58f100cdc5 100644
--- a/packages/server/modules/core/index.ts
+++ b/packages/server/modules/core/index.ts
@@ -11,7 +11,6 @@ import uploadRest from '@/modules/core/rest/upload'
 import downloadRest from '@/modules/core/rest/download'
 import diffUpload from '@/modules/core/rest/diffUpload'
 import diffDownload from '@/modules/core/rest/diffDownload'
-import healthRest from '@/modules/core/rest/health'
 import scopes from '@/modules/core/scopes'
 import roles from '@/modules/core/roles'
 import Redis from 'ioredis'
@@ -33,9 +32,6 @@ const coreModule: SpeckleModule<{
     // Initialize the static route
     staticRest(app)
 
-    // Initialize the health check route
-    healthRest(app)
-
     // Initialises the two main bulk upload/download endpoints
     uploadRest(app)
     downloadRest(app)
diff --git a/packages/server/modules/core/rest/health.ts b/packages/server/modules/healthchecks/health.ts
similarity index 54%
rename from packages/server/modules/core/rest/health.ts
rename to packages/server/modules/healthchecks/health.ts
index 609917b493..cb265f61fe 100644
--- a/packages/server/modules/core/rest/health.ts
+++ b/packages/server/modules/healthchecks/health.ts
@@ -1,85 +1,52 @@
-import * as express from 'express'
 import { getServerInfo } from '@/modules/core/services/generic'
 import { createRedisClient } from '@/modules/shared/redis/redis'
-import {
-  getRedisUrl,
-  highFrequencyMetricsCollectionPeriodMs,
-  postgresMaxConnections
-} from '@/modules/shared/helpers/envHelper'
+import { getRedisUrl, postgresMaxConnections } from '@/modules/shared/helpers/envHelper'
 import type { Redis } from 'ioredis'
 import { numberOfFreeConnections } from '@/modules/shared/helpers/dbHelper'
-import { db } from '@/db/knex'
+
 import type { Knex } from 'knex'
+import { BaseError } from '@/modules/shared/errors'
+import { ensureErrorOrWrapAsCause } from '@/modules/shared/errors/ensureError'
 
 type FreeConnectionsCalculator = {
   mean: () => number
 }
 
-export default (app: express.Application) => {
-  const knexFreeDbConnectionSamplerLiveness = knexFreeDbConnectionSamplerFactory({
-    db,
-    collectionPeriod: highFrequencyMetricsCollectionPeriodMs(),
-    sampledDuration: 600000 //number of ms over which to average the database connections, before declaring not alive. 10 minutes.
-  })
-  knexFreeDbConnectionSamplerLiveness.start()
-
-  const knexFreeDbConnectionSamplerReadiness = knexFreeDbConnectionSamplerFactory({
-    db,
-    collectionPeriod: highFrequencyMetricsCollectionPeriodMs(),
-    sampledDuration: 20000 //number of ms over which to average the database connections, before declaring unready. 20 seconds.
-  })
-  knexFreeDbConnectionSamplerReadiness.start()
-
-  app.options('/liveness')
-  app.get(
-    '/liveness',
-    handleLivenessFactory({
-      isRedisAlive,
-      isPostgresAlive,
-      freeConnectionsCalculator: knexFreeDbConnectionSamplerLiveness
-    })
-  )
-  app.options('/readiness')
-  app.get(
-    '/readiness',
-    handleReadinessFactory({
-      isRedisAlive,
-      isPostgresAlive,
-      freeConnectionsCalculator: knexFreeDbConnectionSamplerReadiness
-    })
-  )
+class LivenessError extends BaseError {
+  static defaultMessage = 'The application is not yet alive. Please try again later.'
+  static code = 'LIVENESS_ERROR'
+  static statusCode = 500
+}
+
+class ReadinessError extends BaseError {
+  static defaultMessage =
+    'The application is not ready to accept requests. Please try again later.'
+  static code = 'READINESS_ERROR'
+  static statusCode = 500
 }
 
-const handleLivenessFactory =
+export const handleLivenessFactory =
   (deps: {
     isRedisAlive: RedisCheck
     isPostgresAlive: DBCheck
     freeConnectionsCalculator: FreeConnectionsCalculator
-  }): express.RequestHandler =>
-  async (req, res) => {
+  }) =>
+  async () => {
     const postgres = await deps.isPostgresAlive()
     if (!postgres.isAlive) {
-      req.log.error(
-        postgres.err,
-        'Liveness health check failed. Postgres is not available.'
+      throw new LivenessError(
+        'Liveness health check failed. Postgres is not available.',
+        {
+          cause: ensureErrorOrWrapAsCause(postgres.err, 'Unknown postgres error.')
+        }
       )
-      res.status(500).json({
-        message: 'Postgres is not available',
-        error: postgres.err
-      })
-      res.send()
-      return
     }
 
     const redis = await deps.isRedisAlive()
     if (!redis.isAlive) {
-      req.log.error(redis.err, 'Liveness health check failed. Redis is not available.')
-      res.status(500).json({
-        message: 'Redis is not available.',
-        error: redis.err
+      throw new LivenessError('Liveness health check failed. Redis is not available.', {
+        cause: ensureErrorOrWrapAsCause(redis.err, 'Unknown redis error.')
       })
-      res.send()
-      return
     }
 
     const numFreeConnections = await deps.freeConnectionsCalculator.mean()
@@ -88,49 +55,40 @@ const handleLivenessFactory =
     )
     //unready if less than 10%
     if (percentageFreeConnections < 10) {
-      const message =
+      throw new LivenessError(
         'Liveness health check failed. Insufficient free database connections for a sustained duration.'
-      req.log.error(message)
-      res.status(500).json({
-        message
-      })
-      res.send()
-      return
+      )
     }
 
-    res.status(200)
-    res.send()
+    return {
+      details: {
+        postgres: 'true',
+        redis: 'true',
+        percentageFreeConnections: percentageFreeConnections.toFixed(0)
+      }
+    }
   }
 
-const handleReadinessFactory = (deps: {
+export const handleReadinessFactory = (deps: {
   isRedisAlive: RedisCheck
   isPostgresAlive: DBCheck
   freeConnectionsCalculator: FreeConnectionsCalculator
-}): express.RequestHandler => {
-  return async (req, res) => {
+}) => {
+  return async () => {
     const postgres = await deps.isPostgresAlive()
     if (!postgres.isAlive) {
-      req.log.error(
-        postgres.err,
-        'Readiness health check failed. Postgres is not available.'
+      throw new ReadinessError(
+        'Readiness health check failed. Postgres is not available.',
+        { cause: ensureErrorOrWrapAsCause(postgres.err, 'Unknown postgres error.') }
       )
-      res.status(500).json({
-        message: 'Postgres is not available',
-        error: postgres.err
-      })
-      res.send()
-      return
     }
 
     const redis = await deps.isRedisAlive()
     if (!redis.isAlive) {
-      req.log.error(redis.err, 'Readiness health check failed. Redis is not available.')
-      res.status(500).json({
-        message: 'Redis is not available.',
-        error: redis.err
-      })
-      res.send()
-      return
+      throw new ReadinessError(
+        'Readiness health check failed. Redis is not available.',
+        { cause: ensureErrorOrWrapAsCause(redis.err, 'Unknown Redis error.') }
+      )
     }
 
     const numFreeConnections = await deps.freeConnectionsCalculator.mean()
@@ -141,16 +99,16 @@ const handleReadinessFactory = (deps: {
     if (percentageFreeConnections < 10) {
       const message =
         'Readiness health check failed. Insufficient free database connections for a sustained duration.'
-      req.log.error(message)
-      res.status(500).json({
-        message
-      })
-      res.send()
-      return
+      throw new ReadinessError(message)
     }
 
-    res.status(200)
-    res.send()
+    return {
+      details: {
+        postgres: 'true',
+        redis: 'true',
+        percentageFreeConnections: percentageFreeConnections.toFixed(0)
+      }
+    }
   }
 }
 
@@ -158,7 +116,7 @@ type CheckResponse = { isAlive: true } | { isAlive: false; err: unknown }
 
 type DBCheck = () => Promise<CheckResponse>
 
-const isPostgresAlive: DBCheck = async (): Promise<CheckResponse> => {
+export const isPostgresAlive: DBCheck = async (): Promise<CheckResponse> => {
   try {
     await getServerInfo()
   } catch (err) {
@@ -169,7 +127,7 @@ const isPostgresAlive: DBCheck = async (): Promise<CheckResponse> => {
 
 type RedisCheck = () => Promise<CheckResponse>
 
-const isRedisAlive: RedisCheck = async (): Promise<CheckResponse> => {
+export const isRedisAlive: RedisCheck = async (): Promise<CheckResponse> => {
   let client: Redis | undefined = undefined
   let result: CheckResponse = { isAlive: true }
   try {
diff --git a/packages/server/modules/healthchecks/index.ts b/packages/server/modules/healthchecks/index.ts
new file mode 100644
index 0000000000..4e528ec8a2
--- /dev/null
+++ b/packages/server/modules/healthchecks/index.ts
@@ -0,0 +1,54 @@
+import { moduleLogger, shutdownLogger } from '@/logging/logging'
+import { SpeckleModule } from '@/modules/shared/helpers/typeHelper'
+import { db } from '@/db/knex'
+import { highFrequencyMetricsCollectionPeriodMs } from '@/modules/shared/helpers/envHelper'
+import {
+  handleLivenessFactory,
+  handleReadinessFactory,
+  knexFreeDbConnectionSamplerFactory,
+  isRedisAlive,
+  isPostgresAlive
+} from '@/modules/healthchecks/health'
+
+let livenessHandler: () => Promise<{ details: Record<string, string> }>
+let readinessHandler: () => Promise<{ details: Record<string, string> }>
+export const isAlive: () => Promise<{ details: Record<string, string> }> = () =>
+  livenessHandler()
+export const isReady: () => Promise<{ details: Record<string, string> }> = () =>
+  readinessHandler()
+
+export const init: SpeckleModule['init'] = async (_, isInitial) => {
+  moduleLogger.info('💓 Init health check module')
+  if (isInitial) {
+    const knexFreeDbConnectionSamplerLiveness = knexFreeDbConnectionSamplerFactory({
+      db,
+      collectionPeriod: highFrequencyMetricsCollectionPeriodMs(),
+      sampledDuration: 600000 //number of ms over which to average the database connections, before declaring not alive. 10 minutes.
+    })
+    knexFreeDbConnectionSamplerLiveness.start()
+
+    const knexFreeDbConnectionSamplerReadiness = knexFreeDbConnectionSamplerFactory({
+      db,
+      collectionPeriod: highFrequencyMetricsCollectionPeriodMs(),
+      sampledDuration: 20000 //number of ms over which to average the database connections, before declaring unready. 20 seconds.
+    })
+    knexFreeDbConnectionSamplerReadiness.start()
+
+    livenessHandler = handleLivenessFactory({
+      isRedisAlive,
+      isPostgresAlive,
+      freeConnectionsCalculator: knexFreeDbConnectionSamplerLiveness
+    })
+
+    readinessHandler = handleReadinessFactory({
+      isRedisAlive,
+      isPostgresAlive,
+      freeConnectionsCalculator: knexFreeDbConnectionSamplerReadiness
+    })
+  }
+}
+
+export const shutdown: SpeckleModule['shutdown'] = async () => {
+  shutdownLogger.info('💓 Shutting down health check module')
+  //no-op
+}
diff --git a/packages/server/modules/index.ts b/packages/server/modules/index.ts
index c8f323ed94..3809f3ec39 100644
--- a/packages/server/modules/index.ts
+++ b/packages/server/modules/index.ts
@@ -70,6 +70,7 @@ const getEnabledModuleNames = () => {
     'cross-server-sync',
     'emails',
     'fileuploads',
+    'healthchecks',
     'notifications',
     'previews',
     'pwdreset',
diff --git a/packages/server/modules/notifications/services/queue.ts b/packages/server/modules/notifications/services/queue.ts
index 65eb3c4834..6ab010a69c 100644
--- a/packages/server/modules/notifications/services/queue.ts
+++ b/packages/server/modules/notifications/services/queue.ts
@@ -17,6 +17,7 @@ import Bull from 'bull'
 import { buildBaseQueueOptions } from '@/modules/shared/helpers/bullHelper'
 import cryptoRandomString from 'crypto-random-string'
 import { logger, notificationsLogger, Observability } from '@/logging/logging'
+import { ensureErrorOrWrapAsCause } from '@/modules/shared/errors/ensureError'
 
 export type NotificationJobResult = {
   status: NotificationJobResultsStatus
@@ -153,8 +154,10 @@ export async function consumeIncomingNotifications() {
       }
     } catch (e: unknown) {
       notificationsLogger.error(e)
-      const err =
-        e instanceof Error ? e : new Error('Unexpected notification consumption error')
+      const err = ensureErrorOrWrapAsCause(
+        e,
+        'Unexpected notification consumption error'
+      )
 
       if (!(err instanceof NotificationValidationError)) {
         throw err
diff --git a/packages/server/modules/shared/errors/ensureError.ts b/packages/server/modules/shared/errors/ensureError.ts
new file mode 100644
index 0000000000..8a9e7e7f89
--- /dev/null
+++ b/packages/server/modules/shared/errors/ensureError.ts
@@ -0,0 +1,11 @@
+/**
+ * In JS catch clauses can receive not only Errors, but pretty much any other
+ * kind of data type, so you can use this helper to ensure that
+ * whatever is passed in is a real error.
+ * If it is not a real error, it will be wrapped in a new error
+ * with the provided message and the original error as the cause.
+ */
+export function ensureErrorOrWrapAsCause(e: unknown, fallbackMessage?: string): Error {
+  if (e instanceof Error) return e
+  return new Error(fallbackMessage, { cause: e })
+}
diff --git a/packages/server/modules/shared/helpers/envHelper.ts b/packages/server/modules/shared/helpers/envHelper.ts
index ba4583ee3a..e1cf812ab7 100644
--- a/packages/server/modules/shared/helpers/envHelper.ts
+++ b/packages/server/modules/shared/helpers/envHelper.ts
@@ -453,5 +453,5 @@ export function createS3Bucket() {
 }
 
 export function shutdownTimeoutSeconds() {
-  return getIntFromEnv('SHUTDOWN_TIMEOUT_SECONDS', `${5 * 60}`)
+  return getIntFromEnv('SHUTDOWN_TIMEOUT_SECONDS', '300')
 }

From bcaec34b38f9eda394660d465d316bad072e9c4d Mon Sep 17 00:00:00 2001
From: Iain Sproat <68657+iainsproat@users.noreply.github.com>
Date: Thu, 26 Sep 2024 19:03:24 +0100
Subject: [PATCH 05/14] terminus is responsible for stopping the graphql server

---
 packages/server/app.ts     | 17 ++++++++++-------
 packages/server/bin/ts-www |  2 +-
 packages/server/bin/www    |  2 +-
 3 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/packages/server/app.ts b/packages/server/app.ts
index 6eea565070..00f86a6585 100644
--- a/packages/server/app.ts
+++ b/packages/server/app.ts
@@ -75,8 +75,6 @@ import { isAlive, isReady } from '@/modules/healthchecks'
 
 const GRAPHQL_PATH = '/graphql'
 
-let graphqlServer: ApolloServer<GraphQLContext>
-
 // eslint-disable-next-line @typescript-eslint/no-explicit-any
 type SubscriptionResponse = { errors?: GraphQLError[]; data?: any }
 
@@ -406,7 +404,7 @@ export async function init() {
   const subscriptionServer = buildApolloSubscriptionServer(server)
 
   // Initialize graphql server
-  graphqlServer = await buildApolloServer({
+  const graphqlServer = await buildApolloServer({
     subscriptionServer
   })
   app.use(
@@ -432,7 +430,10 @@ export async function init() {
   return { app, graphqlServer, server, subscriptionServer }
 }
 
-export async function shutdown(): Promise<void> {
+export async function shutdown(params: {
+  graphqlServer: ApolloServer<GraphQLContext>
+}): Promise<void> {
+  await params.graphqlServer.stop()
   await ModulesSetup.shutdown()
 }
 
@@ -462,9 +463,10 @@ async function createFrontendProxy() {
 export async function startHttp(params: {
   server: http.Server
   app: Express
+  graphqlServer: ApolloServer<GraphQLContext>
   customPortOverride?: number
 }) {
-  const { server, app, customPortOverride } = params
+  const { server, app, graphqlServer, customPortOverride } = params
   let bindAddress = process.env.BIND_ADDRESS || '127.0.0.1'
   let port = process.env.PORT ? toNumber(process.env.PORT) : 3000
 
@@ -494,11 +496,12 @@ export async function startHttp(params: {
       shutdownLogger.info('Shutting down (signal received)...')
     },
     onSignal: async () => {
-      await shutdown()
+      await shutdown({ graphqlServer })
     },
     onShutdown: () => {
       shutdownLogger.info('Shutdown completed')
-      process.exit(0)
+      shutdownLogger.flush()
+      return Promise.resolve()
     },
     healthChecks: {
       '/readiness': isReady,
diff --git a/packages/server/bin/ts-www b/packages/server/bin/ts-www
index aebc00b5d7..1573145649 100755
--- a/packages/server/bin/ts-www
+++ b/packages/server/bin/ts-www
@@ -11,7 +11,7 @@ const { logger } = require('../logging/logging')
 const { init, startHttp } = require('../app')
 
 init()
-  .then(({ app, server }) => startHttp({server, app}))
+  .then(({ app, graphqlServer, server }) => startHttp({ server, graphqlServer, app }))
   .catch((err) => {
     logger.error(err, 'Failed to start server. Exiting with non-zero exit code...')
 
diff --git a/packages/server/bin/www b/packages/server/bin/www
index 0dc0089461..eb3d954f8b 100755
--- a/packages/server/bin/www
+++ b/packages/server/bin/www
@@ -5,7 +5,7 @@ const { logger } = require('../dist/logging/logging')
 const { init, startHttp } = require('../dist/app')
 
 init()
-  .then(({ app, server }) => startHttp({server, app}))
+  .then(({ app, graphqlServer, server }) => startHttp({ server, graphqlServer, app }))
   .catch((err) => {
     logger.error(err, 'Failed to start server. Exiting with non-zero exit code...')
 

From 2210e53fcef31ff25491606d0711583bebb07268 Mon Sep 17 00:00:00 2001
From: Iain Sproat <68657+iainsproat@users.noreply.github.com>
Date: Thu, 26 Sep 2024 19:09:07 +0100
Subject: [PATCH 06/14] remove logging on shutdown

---
 packages/server/modules/healthchecks/index.ts | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/packages/server/modules/healthchecks/index.ts b/packages/server/modules/healthchecks/index.ts
index 4e528ec8a2..c5f93a7e6b 100644
--- a/packages/server/modules/healthchecks/index.ts
+++ b/packages/server/modules/healthchecks/index.ts
@@ -1,4 +1,4 @@
-import { moduleLogger, shutdownLogger } from '@/logging/logging'
+import { moduleLogger } from '@/logging/logging'
 import { SpeckleModule } from '@/modules/shared/helpers/typeHelper'
 import { db } from '@/db/knex'
 import { highFrequencyMetricsCollectionPeriodMs } from '@/modules/shared/helpers/envHelper'
@@ -49,6 +49,5 @@ export const init: SpeckleModule['init'] = async (_, isInitial) => {
 }
 
 export const shutdown: SpeckleModule['shutdown'] = async () => {
-  shutdownLogger.info('💓 Shutting down health check module')
   //no-op
 }

From 6609628f18a2db1bef7efe714e2d1036e5310e42 Mon Sep 17 00:00:00 2001
From: Iain Sproat <68657+iainsproat@users.noreply.github.com>
Date: Thu, 26 Sep 2024 19:10:42 +0100
Subject: [PATCH 07/14] Remove redundant parameter

---
 packages/server/app.ts | 1 -
 1 file changed, 1 deletion(-)

diff --git a/packages/server/app.ts b/packages/server/app.ts
index 00f86a6585..091182b9b5 100644
--- a/packages/server/app.ts
+++ b/packages/server/app.ts
@@ -295,7 +295,6 @@ function buildApolloSubscriptionServer(server: http.Server): SubscriptionServer
  */
 export async function buildApolloServer(options?: {
   subscriptionServer?: SubscriptionServer
-  httpServer?: http.Server
 }): Promise<ApolloServer<GraphQLContext>> {
   const includeStacktraceInErrorResponses = isDevEnv() || isTestEnv()
   const subscriptionServer = options?.subscriptionServer

From e7355b11d0cbe2f17222fa102ab0f8c5699cbb45 Mon Sep 17 00:00:00 2001
From: Iain Sproat <68657+iainsproat@users.noreply.github.com>
Date: Fri, 27 Sep 2024 12:06:34 +0100
Subject: [PATCH 08/14] move healthchecks out of business modules to top-level
 directory - terminus can only handle readiness check, not liveness - app
 needs to return readiness handler, so that server terminus can use it

---
 packages/server/app.ts                        | 24 ++++++--
 packages/server/bin/ts-www                    |  4 +-
 packages/server/bin/www                       |  4 +-
 .../{modules => }/healthchecks/health.ts      |  6 +-
 packages/server/healthchecks/index.ts         | 60 +++++++++++++++++++
 packages/server/logging/logging.ts            |  1 +
 .../server/modules/core/tests/health.spec.ts  |  8 ++-
 packages/server/modules/healthchecks/index.ts | 53 ----------------
 packages/server/modules/index.ts              |  1 -
 packages/server/test/hooks.js                 | 10 ++--
 packages/server/tsconfig.json                 |  1 +
 11 files changed, 101 insertions(+), 71 deletions(-)
 rename packages/server/{modules => }/healthchecks/health.ts (97%)
 create mode 100644 packages/server/healthchecks/index.ts
 delete mode 100644 packages/server/modules/healthchecks/index.ts

diff --git a/packages/server/app.ts b/packages/server/app.ts
index 091182b9b5..e2bf7d8ea5 100644
--- a/packages/server/app.ts
+++ b/packages/server/app.ts
@@ -71,7 +71,8 @@ import { BaseError, ForbiddenError } from '@/modules/shared/errors'
 import { loggingPlugin } from '@/modules/core/graph/plugins/logging'
 import { shouldLogAsInfoLevel } from '@/logging/graphqlError'
 import { getUser } from '@/modules/core/repositories/users'
-import { isAlive, isReady } from '@/modules/healthchecks'
+import { initFactory as healthchecksInitFactory } from '@/healthchecks'
+import type { ReadinessHandler } from '@/healthchecks/health'
 
 const GRAPHQL_PATH = '/graphql'
 
@@ -398,6 +399,9 @@ export async function init() {
   // Initialize default modules, including rest api handlers
   await ModulesSetup.init(app)
 
+  // Initialize healthchecks
+  const healthchecks = await healthchecksInitFactory()(app, true)
+
   // Init HTTP server & subscription server
   const server = http.createServer(app)
   const subscriptionServer = buildApolloSubscriptionServer(server)
@@ -426,13 +430,19 @@ export async function init() {
   // At the very end adding default error handler middleware
   app.use(defaultErrorHandler)
 
-  return { app, graphqlServer, server, subscriptionServer }
+  return {
+    app,
+    graphqlServer,
+    server,
+    subscriptionServer,
+    readinessCheck: healthchecks.isReady
+  }
 }
 
 export async function shutdown(params: {
   graphqlServer: ApolloServer<GraphQLContext>
 }): Promise<void> {
-  await params.graphqlServer.stop()
+  await params.graphqlServer?.stop()
   await ModulesSetup.shutdown()
 }
 
@@ -463,9 +473,10 @@ export async function startHttp(params: {
   server: http.Server
   app: Express
   graphqlServer: ApolloServer<GraphQLContext>
+  readinessCheck: ReadinessHandler
   customPortOverride?: number
 }) {
-  const { server, app, graphqlServer, customPortOverride } = params
+  const { server, app, graphqlServer, readinessCheck, customPortOverride } = params
   let bindAddress = process.env.BIND_ADDRESS || '127.0.0.1'
   let port = process.env.PORT ? toNumber(process.env.PORT) : 3000
 
@@ -503,8 +514,9 @@ export async function startHttp(params: {
       return Promise.resolve()
     },
     healthChecks: {
-      '/readiness': isReady,
-      '/liveness': isAlive,
+      '/readiness': readinessCheck,
+      // '/liveness' should return true even if in shutdown phase, so app does not get restarted while draining connections
+      // therefore we cannot use terminus to handle liveness checks.
       verbatim: true
     },
     logger: (message, err) => {
diff --git a/packages/server/bin/ts-www b/packages/server/bin/ts-www
index 1573145649..47e6c36048 100755
--- a/packages/server/bin/ts-www
+++ b/packages/server/bin/ts-www
@@ -11,7 +11,9 @@ const { logger } = require('../logging/logging')
 const { init, startHttp } = require('../app')
 
 init()
-  .then(({ app, graphqlServer, server }) => startHttp({ server, graphqlServer, app }))
+  .then(({ app, graphqlServer, server, readinessCheck }) =>
+    startHttp({ app, graphqlServer, server, readinessCheck })
+  )
   .catch((err) => {
     logger.error(err, 'Failed to start server. Exiting with non-zero exit code...')
 
diff --git a/packages/server/bin/www b/packages/server/bin/www
index eb3d954f8b..d8744a143b 100755
--- a/packages/server/bin/www
+++ b/packages/server/bin/www
@@ -5,7 +5,9 @@ const { logger } = require('../dist/logging/logging')
 const { init, startHttp } = require('../dist/app')
 
 init()
-  .then(({ app, graphqlServer, server }) => startHttp({ server, graphqlServer, app }))
+  .then(({ app, graphqlServer, server, readinessCheck }) =>
+    startHttp({ app, graphqlServer, server, readinessCheck })
+  )
   .catch((err) => {
     logger.error(err, 'Failed to start server. Exiting with non-zero exit code...')
 
diff --git a/packages/server/modules/healthchecks/health.ts b/packages/server/healthchecks/health.ts
similarity index 97%
rename from packages/server/modules/healthchecks/health.ts
rename to packages/server/healthchecks/health.ts
index cb265f61fe..c55af69e6b 100644
--- a/packages/server/modules/healthchecks/health.ts
+++ b/packages/server/healthchecks/health.ts
@@ -8,7 +8,9 @@ import type { Knex } from 'knex'
 import { BaseError } from '@/modules/shared/errors'
 import { ensureErrorOrWrapAsCause } from '@/modules/shared/errors/ensureError'
 
-type FreeConnectionsCalculator = {
+export type ReadinessHandler = () => Promise<{ details: Record<string, string> }>
+
+export type FreeConnectionsCalculator = {
   mean: () => number
 }
 
@@ -73,7 +75,7 @@ export const handleReadinessFactory = (deps: {
   isRedisAlive: RedisCheck
   isPostgresAlive: DBCheck
   freeConnectionsCalculator: FreeConnectionsCalculator
-}) => {
+}): ReadinessHandler => {
   return async () => {
     const postgres = await deps.isPostgresAlive()
     if (!postgres.isAlive) {
diff --git a/packages/server/healthchecks/index.ts b/packages/server/healthchecks/index.ts
new file mode 100644
index 0000000000..cae174b59e
--- /dev/null
+++ b/packages/server/healthchecks/index.ts
@@ -0,0 +1,60 @@
+import { healthCheckLogger } from '@/logging/logging'
+import { db } from '@/db/knex'
+import { highFrequencyMetricsCollectionPeriodMs } from '@/modules/shared/helpers/envHelper'
+import {
+  handleLivenessFactory,
+  handleReadinessFactory,
+  knexFreeDbConnectionSamplerFactory,
+  isRedisAlive,
+  isPostgresAlive,
+  ReadinessHandler,
+  FreeConnectionsCalculator
+} from '@/healthchecks/health'
+import { Application } from 'express'
+
+export const initFactory: () => (
+  app: Application,
+  isInitial: boolean
+) => Promise<{ isReady: ReadinessHandler }> = () => {
+  let knexFreeDbConnectionSamplerLiveness: FreeConnectionsCalculator & {
+    start: () => void
+  }
+  let knexFreeDbConnectionSamplerReadiness: FreeConnectionsCalculator & {
+    start: () => void
+  }
+  return async (app, isInitial) => {
+    healthCheckLogger.info('💓 Init health check')
+    if (isInitial) {
+      knexFreeDbConnectionSamplerLiveness = knexFreeDbConnectionSamplerFactory({
+        db,
+        collectionPeriod: highFrequencyMetricsCollectionPeriodMs(),
+        sampledDuration: 600000 //number of ms over which to average the database connections, before declaring not alive. 10 minutes.
+      })
+      knexFreeDbConnectionSamplerLiveness.start()
+
+      knexFreeDbConnectionSamplerReadiness = knexFreeDbConnectionSamplerFactory({
+        db,
+        collectionPeriod: highFrequencyMetricsCollectionPeriodMs(),
+        sampledDuration: 20000 //number of ms over which to average the database connections, before declaring unready. 20 seconds.
+      })
+      knexFreeDbConnectionSamplerReadiness.start()
+    }
+    const livenessHandler = handleLivenessFactory({
+      isRedisAlive,
+      isPostgresAlive,
+      freeConnectionsCalculator: knexFreeDbConnectionSamplerLiveness
+    })
+    app.get('/liveness', async (req, res) => {
+      const result = await livenessHandler()
+      res.status(200).json({ status: 'ok', ...result })
+    })
+
+    return {
+      isReady: handleReadinessFactory({
+        isRedisAlive,
+        isPostgresAlive,
+        freeConnectionsCalculator: knexFreeDbConnectionSamplerReadiness
+      })
+    }
+  }
+}
diff --git a/packages/server/logging/logging.ts b/packages/server/logging/logging.ts
index 2bee9ebbe6..34390451ec 100644
--- a/packages/server/logging/logging.ts
+++ b/packages/server/logging/logging.ts
@@ -30,6 +30,7 @@ export const authLogger = extendLoggerComponent(logger, 'auth')
 export const crossServerSyncLogger = extendLoggerComponent(logger, 'cross-server-sync')
 export const automateLogger = extendLoggerComponent(logger, 'automate')
 export const subscriptionLogger = extendLoggerComponent(logger, 'subscription')
+export const healthCheckLogger = extendLoggerComponent(logger, 'healthcheck')
 
 export type Logger = typeof logger
 export { extendLoggerComponent, Observability }
diff --git a/packages/server/modules/core/tests/health.spec.ts b/packages/server/modules/core/tests/health.spec.ts
index 3e02617760..5a2165b9f5 100644
--- a/packages/server/modules/core/tests/health.spec.ts
+++ b/packages/server/modules/core/tests/health.spec.ts
@@ -1,12 +1,14 @@
 /* istanbul ignore file */
+import { ReadinessHandler } from '@/healthchecks/health'
 import { beforeEachContext } from '@/test/hooks'
 import { expect } from 'chai'
 import request from 'supertest'
 
 describe('Health Routes @api-rest', () => {
   let app: Express.Application
+  let readinessCheck: ReadinessHandler
   before(async () => {
-    ;({ app } = await beforeEachContext())
+    ;({ app, readinessCheck } = await beforeEachContext())
   })
 
   it('Should response to liveness endpoint', async () => {
@@ -15,7 +17,7 @@ describe('Health Routes @api-rest', () => {
   })
 
   it('Should response to readiness endpoint', async () => {
-    const res = await request(app).get('/readiness')
-    expect(res).to.have.status(200)
+    const res = await readinessCheck()
+    expect(res).to.have.property('details')
   })
 })
diff --git a/packages/server/modules/healthchecks/index.ts b/packages/server/modules/healthchecks/index.ts
deleted file mode 100644
index c5f93a7e6b..0000000000
--- a/packages/server/modules/healthchecks/index.ts
+++ /dev/null
@@ -1,53 +0,0 @@
-import { moduleLogger } from '@/logging/logging'
-import { SpeckleModule } from '@/modules/shared/helpers/typeHelper'
-import { db } from '@/db/knex'
-import { highFrequencyMetricsCollectionPeriodMs } from '@/modules/shared/helpers/envHelper'
-import {
-  handleLivenessFactory,
-  handleReadinessFactory,
-  knexFreeDbConnectionSamplerFactory,
-  isRedisAlive,
-  isPostgresAlive
-} from '@/modules/healthchecks/health'
-
-let livenessHandler: () => Promise<{ details: Record<string, string> }>
-let readinessHandler: () => Promise<{ details: Record<string, string> }>
-export const isAlive: () => Promise<{ details: Record<string, string> }> = () =>
-  livenessHandler()
-export const isReady: () => Promise<{ details: Record<string, string> }> = () =>
-  readinessHandler()
-
-export const init: SpeckleModule['init'] = async (_, isInitial) => {
-  moduleLogger.info('💓 Init health check module')
-  if (isInitial) {
-    const knexFreeDbConnectionSamplerLiveness = knexFreeDbConnectionSamplerFactory({
-      db,
-      collectionPeriod: highFrequencyMetricsCollectionPeriodMs(),
-      sampledDuration: 600000 //number of ms over which to average the database connections, before declaring not alive. 10 minutes.
-    })
-    knexFreeDbConnectionSamplerLiveness.start()
-
-    const knexFreeDbConnectionSamplerReadiness = knexFreeDbConnectionSamplerFactory({
-      db,
-      collectionPeriod: highFrequencyMetricsCollectionPeriodMs(),
-      sampledDuration: 20000 //number of ms over which to average the database connections, before declaring unready. 20 seconds.
-    })
-    knexFreeDbConnectionSamplerReadiness.start()
-
-    livenessHandler = handleLivenessFactory({
-      isRedisAlive,
-      isPostgresAlive,
-      freeConnectionsCalculator: knexFreeDbConnectionSamplerLiveness
-    })
-
-    readinessHandler = handleReadinessFactory({
-      isRedisAlive,
-      isPostgresAlive,
-      freeConnectionsCalculator: knexFreeDbConnectionSamplerReadiness
-    })
-  }
-}
-
-export const shutdown: SpeckleModule['shutdown'] = async () => {
-  //no-op
-}
diff --git a/packages/server/modules/index.ts b/packages/server/modules/index.ts
index 3809f3ec39..c8f323ed94 100644
--- a/packages/server/modules/index.ts
+++ b/packages/server/modules/index.ts
@@ -70,7 +70,6 @@ const getEnabledModuleNames = () => {
     'cross-server-sync',
     'emails',
     'fileuploads',
-    'healthchecks',
     'notifications',
     'previews',
     'pwdreset',
diff --git a/packages/server/test/hooks.js b/packages/server/test/hooks.js
index c537c448e4..a4fb6beb5e 100644
--- a/packages/server/test/hooks.js
+++ b/packages/server/test/hooks.js
@@ -77,6 +77,7 @@ const initializeTestServer = async (server, app) => {
   }
 }
 
+let graphqlServer = undefined
 exports.mochaHooks = {
   beforeAll: async () => {
     logger.info('running before all')
@@ -84,18 +85,19 @@ exports.mochaHooks = {
     await exports.truncateTables()
     await knex.migrate.rollback()
     await knex.migrate.latest()
-    await init()
+    const initValues = await init()
+    graphqlServer = initValues.graphqlServer
   },
   afterAll: async () => {
     logger.info('running after all')
     await unlock()
-    await shutdown()
+    await shutdown({ graphqlServer })
   }
 }
 
 exports.buildApp = async () => {
-  const { app, graphqlServer, server } = await init()
-  return { app, graphqlServer, server }
+  const { app, graphqlServer, server, readinessCheck } = await init()
+  return { app, graphqlServer, server, readinessCheck }
 }
 
 exports.beforeEachContext = async () => {
diff --git a/packages/server/tsconfig.json b/packages/server/tsconfig.json
index ab9da3003f..4c91904f42 100644
--- a/packages/server/tsconfig.json
+++ b/packages/server/tsconfig.json
@@ -106,6 +106,7 @@
   },
   "include": [
     "db/**/*",
+    "healthchecks/**/*",
     "logging/**/*",
     "modules/**/*",
     "bin/**/*",

From 2aedf55a3ad51ee743e9fc347b461494cd9d3bda Mon Sep 17 00:00:00 2001
From: Iain Sproat <68657+iainsproat@users.noreply.github.com>
Date: Fri, 27 Sep 2024 13:46:17 +0100
Subject: [PATCH 09/14] fix tests

---
 packages/server/app.ts                        |  2 +-
 .../activitystream/tests/activity.spec.js     | 19 ++++++++++++++-----
 .../modules/auth/tests/apps.graphql.spec.js   | 16 +++++++++++++---
 .../server/modules/auth/tests/auth.spec.js    | 16 ++++++++++++++--
 .../server/modules/core/tests/graph.spec.js   | 16 ++++++++++++++--
 .../tests/fileuploads.integration.spec.ts     | 16 ++++++++++++++--
 .../server/modules/stats/tests/stats.spec.ts  | 19 ++++++++++++++-----
 .../modules/webhooks/tests/webhooks.spec.js   | 17 ++++++++++++++---
 packages/server/test/hooks.js                 |  8 ++------
 9 files changed, 100 insertions(+), 29 deletions(-)

diff --git a/packages/server/app.ts b/packages/server/app.ts
index e2bf7d8ea5..1dbd0ea902 100644
--- a/packages/server/app.ts
+++ b/packages/server/app.ts
@@ -442,7 +442,7 @@ export async function init() {
 export async function shutdown(params: {
   graphqlServer: ApolloServer<GraphQLContext>
 }): Promise<void> {
-  await params.graphqlServer?.stop()
+  await params.graphqlServer.stop()
   await ModulesSetup.shutdown()
 }
 
diff --git a/packages/server/modules/activitystream/tests/activity.spec.js b/packages/server/modules/activitystream/tests/activity.spec.js
index d7760ad88c..ad74e12749 100644
--- a/packages/server/modules/activitystream/tests/activity.spec.js
+++ b/packages/server/modules/activitystream/tests/activity.spec.js
@@ -16,12 +16,10 @@ const { db } = require('@/db/knex')
 
 const getUserActivity = getUserActivityFactory({ db })
 
+let server
 let sendRequest
 
 describe('Activity @activity', () => {
-  let server
-  let app
-
   const userIz = {
     name: 'Izzy Lyseggen',
     email: 'izzybizzi@speckle.systems',
@@ -78,8 +76,19 @@ describe('Activity @activity', () => {
   }
 
   before(async () => {
-    ;({ server, app } = await beforeEachContext())
-    ;({ sendRequest } = await initializeTestServer(server, app))
+    const {
+      server: tmpServer,
+      app,
+      graphqlServer,
+      readinessCheck
+    } = await beforeEachContext()
+    server = tmpServer
+    ;({ sendRequest } = await initializeTestServer({
+      server,
+      app,
+      graphqlServer,
+      readinessCheck
+    }))
 
     const normalScopesList = [
       Scopes.Streams.Read,
diff --git a/packages/server/modules/auth/tests/apps.graphql.spec.js b/packages/server/modules/auth/tests/apps.graphql.spec.js
index 87f0eb973e..def16db34d 100644
--- a/packages/server/modules/auth/tests/apps.graphql.spec.js
+++ b/packages/server/modules/auth/tests/apps.graphql.spec.js
@@ -26,7 +26,6 @@ const {
 
 let sendRequest
 let server
-let app
 
 const createAuthorizationCode = createAuthorizationCodeFactory({ db })
 const createAppTokenFromAccessCode = createAppTokenFromAccessCodeFactory({
@@ -45,8 +44,19 @@ describe('GraphQL @apps-api', () => {
   let testToken2
 
   before(async () => {
-    ;({ app, server } = await beforeEachContext())
-    ;({ sendRequest } = await initializeTestServer(server, app))
+    const {
+      app,
+      server: tmpServer,
+      graphqlServer,
+      readinessCheck
+    } = await beforeEachContext()
+    server = tmpServer
+    ;({ sendRequest } = await initializeTestServer({
+      server,
+      app,
+      graphqlServer,
+      readinessCheck
+    }))
     testUser = {
       name: 'Dimitrie Stefanescu',
       email: 'didimitrie@gmail.com',
diff --git a/packages/server/modules/auth/tests/auth.spec.js b/packages/server/modules/auth/tests/auth.spec.js
index ab6c8a0b7b..db4eda0e6b 100644
--- a/packages/server/modules/auth/tests/auth.spec.js
+++ b/packages/server/modules/auth/tests/auth.spec.js
@@ -43,8 +43,20 @@ describe('Auth @auth', () => {
     }
 
     before(async () => {
-      ;({ app, server } = await beforeEachContext())
-      ;({ sendRequest } = await initializeTestServer(server, app))
+      const {
+        app: tmpApp,
+        server: tmpServer,
+        graphqlServer,
+        readinessCheck
+      } = await beforeEachContext()
+      server = tmpServer
+      app = tmpApp
+      ;({ sendRequest } = await initializeTestServer({
+        server,
+        app,
+        graphqlServer,
+        readinessCheck
+      }))
 
       // Register a user for testing login flows
       await createUser(me).then((id) => (me.id = id))
diff --git a/packages/server/modules/core/tests/graph.spec.js b/packages/server/modules/core/tests/graph.spec.js
index d198b9eba5..a7e2431be5 100644
--- a/packages/server/modules/core/tests/graph.spec.js
+++ b/packages/server/modules/core/tests/graph.spec.js
@@ -41,8 +41,20 @@ describe('GraphQL API Core @core-api', () => {
 
   // set up app & two basic users to ping pong permissions around
   before(async () => {
-    ;({ app, server } = await beforeEachContext())
-    ;({ sendRequest } = await initializeTestServer(server, app))
+    const {
+      app: tmpApp,
+      server: tmpServer,
+      graphqlServer,
+      readinessCheck
+    } = await beforeEachContext()
+    server = tmpServer
+    app = tmpApp
+    ;({ sendRequest } = await initializeTestServer({
+      server,
+      app,
+      graphqlServer,
+      readinessCheck
+    }))
 
     userA.id = await createUser(userA)
     userA.token = `Bearer ${await createPersonalAccessToken(
diff --git a/packages/server/modules/fileuploads/tests/fileuploads.integration.spec.ts b/packages/server/modules/fileuploads/tests/fileuploads.integration.spec.ts
index 5ff3caa6c9..fc8ee0dbf7 100644
--- a/packages/server/modules/fileuploads/tests/fileuploads.integration.spec.ts
+++ b/packages/server/modules/fileuploads/tests/fileuploads.integration.spec.ts
@@ -32,8 +32,20 @@ describe('FileUploads @fileuploads', () => {
   let serverAddress: string
 
   before(async () => {
-    ;({ app, server } = await beforeEachContext())
-    ;({ serverAddress, sendRequest } = await initializeTestServer(server, app))
+    const {
+      app: tmpApp,
+      server: tmpServer,
+      graphqlServer,
+      readinessCheck
+    } = await beforeEachContext()
+    server = tmpServer
+    app = tmpApp
+    ;({ serverAddress, sendRequest } = await initializeTestServer({
+      server,
+      app,
+      graphqlServer,
+      readinessCheck
+    }))
 
     //TODO does mocha have a nicer way of temporarily swapping an environment variable, like vitest?
     existingCanonicalUrl = process.env['CANONICAL_URL'] || ''
diff --git a/packages/server/modules/stats/tests/stats.spec.ts b/packages/server/modules/stats/tests/stats.spec.ts
index d06040bd3a..5fb7d37e49 100644
--- a/packages/server/modules/stats/tests/stats.spec.ts
+++ b/packages/server/modules/stats/tests/stats.spec.ts
@@ -21,7 +21,6 @@ import {
 } from '@/modules/stats/repositories/index'
 import { Scopes } from '@speckle/shared'
 import { Server } from 'node:http'
-import { Express } from 'express'
 import { db } from '@/db/knex'
 
 const params = { numUsers: 25, numStreams: 30, numObjects: 100, numCommits: 100 }
@@ -92,8 +91,7 @@ describe('Server stats services @stats-services', function () {
 
 describe('Server stats api @stats-api', function () {
   let server: Server,
-    sendRequest: Awaited<ReturnType<typeof initializeTestServer>>['sendRequest'],
-    app: Express
+    sendRequest: Awaited<ReturnType<typeof initializeTestServer>>['sendRequest']
 
   const adminUser = {
     name: 'Dimitrie',
@@ -130,8 +128,19 @@ describe('Server stats api @stats-api', function () {
 
   before(async function () {
     this.timeout(15000)
-    ;({ app, server } = await beforeEachContext())
-    ;({ sendRequest } = await initializeTestServer(server, app))
+    const {
+      app,
+      server: tmpServer,
+      graphqlServer,
+      readinessCheck
+    } = await beforeEachContext()
+    server = tmpServer
+    ;({ sendRequest } = await initializeTestServer({
+      server,
+      app,
+      graphqlServer,
+      readinessCheck
+    }))
 
     adminUser.id = await createUser(adminUser)
     adminUser.goodToken = `Bearer ${await createPersonalAccessToken(
diff --git a/packages/server/modules/webhooks/tests/webhooks.spec.js b/packages/server/modules/webhooks/tests/webhooks.spec.js
index ee4fb75e75..7dd62fb70f 100644
--- a/packages/server/modules/webhooks/tests/webhooks.spec.js
+++ b/packages/server/modules/webhooks/tests/webhooks.spec.js
@@ -41,7 +41,7 @@ const getStreamWebhooks = getStreamWebhooksFactory({ db })
 
 describe('Webhooks @webhooks', () => {
   const getWebhook = getWebhookByIdFactory({ db })
-  let server, sendRequest, app
+  let server, sendRequest
 
   const userOne = {
     name: 'User',
@@ -65,8 +65,19 @@ describe('Webhooks @webhooks', () => {
   }
 
   before(async () => {
-    ;({ app, server } = await beforeEachContext())
-    ;({ sendRequest } = await initializeTestServer(server, app))
+    const {
+      app,
+      server: tmpServer,
+      graphqlServer,
+      readinessCheck
+    } = await beforeEachContext()
+    server = tmpServer
+    ;({ sendRequest } = await initializeTestServer({
+      server,
+      app,
+      graphqlServer,
+      readinessCheck
+    }))
 
     userOne.id = await createUser(userOne)
     streamOne.ownerId = userOne.id
diff --git a/packages/server/test/hooks.js b/packages/server/test/hooks.js
index a4fb6beb5e..bc05f84f56 100644
--- a/packages/server/test/hooks.js
+++ b/packages/server/test/hooks.js
@@ -46,12 +46,8 @@ exports.truncateTables = async (tableNames) => {
   await knex.raw(`truncate table ${tableNames.join(',')} cascade`)
 }
 
-/**
- * @param {import('http').Server} server
- * @param {import('express').Express} app
- */
-const initializeTestServer = async (server, app) => {
-  await startHttp({ server, app, customPortOverride: 0 })
+const initializeTestServer = async ({ server, app, graphqlServer, readinessCheck }) => {
+  await startHttp({ server, app, graphqlServer, readinessCheck, customPortOverride: 0 })
 
   await once(app, 'appStarted')
   const port = server.address().port

From 4f28dc01aaf3b468d9aeffd611d20eb8446a4ca2 Mon Sep 17 00:00:00 2001
From: Iain Sproat <68657+iainsproat@users.noreply.github.com>
Date: Tue, 5 Nov 2024 12:34:11 +0000
Subject: [PATCH 10/14] Fix broken merge

---
 packages/server/healthchecks/health.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/server/healthchecks/health.ts b/packages/server/healthchecks/health.ts
index 673737da7f..7a1429c7e3 100644
--- a/packages/server/healthchecks/health.ts
+++ b/packages/server/healthchecks/health.ts
@@ -2,7 +2,7 @@ import { createRedisClient } from '@/modules/shared/redis/redis'
 import { getRedisUrl, postgresMaxConnections } from '@/modules/shared/helpers/envHelper'
 import type { Redis } from 'ioredis'
 import { numberOfFreeConnections } from '@/modules/shared/helpers/dbHelper'
-
+import { db } from '@/db/knex'
 import type { Knex } from 'knex'
 import { getServerInfoFactory } from '@/modules/core/repositories/server'
 import { BaseError } from '@/modules/shared/errors'

From 6d716b1dece5a7ba6adf901678a2a0e840b44a70 Mon Sep 17 00:00:00 2001
From: Iain Sproat <68657+iainsproat@users.noreply.github.com>
Date: Tue, 5 Nov 2024 12:48:55 +0000
Subject: [PATCH 11/14] fix broken merge

---
 .../modules/fileuploads/tests/fileuploads.integration.spec.ts   | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/server/modules/fileuploads/tests/fileuploads.integration.spec.ts b/packages/server/modules/fileuploads/tests/fileuploads.integration.spec.ts
index 9842e220b8..0680193ed5 100644
--- a/packages/server/modules/fileuploads/tests/fileuploads.integration.spec.ts
+++ b/packages/server/modules/fileuploads/tests/fileuploads.integration.spec.ts
@@ -163,7 +163,7 @@ describe('FileUploads @fileuploads', () => {
     } = await beforeEachContext()
     server = tmpServer
     app = tmpApp
-    ;({ serverAddress, sendRequest } = await initializeTestServer({
+    ;({ serverAddress, serverPort, sendRequest } = await initializeTestServer({
       server,
       app,
       graphqlServer,

From 04582eebf1e576fd4d8c4a8312d29747e6891192 Mon Sep 17 00:00:00 2001
From: Iain Sproat <68657+iainsproat@users.noreply.github.com>
Date: Tue, 12 Nov 2024 10:24:05 +0000
Subject: [PATCH 12/14] incorporate review comments

---
 .../activitystream/tests/activity.spec.js      | 16 +++-------------
 .../modules/auth/tests/apps.graphql.spec.js    | 16 +++-------------
 .../server/modules/auth/tests/auth.spec.js     | 18 ++++--------------
 .../server/modules/core/tests/graph.spec.js    | 18 ++++--------------
 .../tests/fileuploads.integration.spec.ts      | 18 ++++--------------
 .../server/modules/stats/tests/stats.spec.ts   | 16 +++-------------
 .../modules/webhooks/tests/webhooks.spec.js    | 16 +++-------------
 7 files changed, 24 insertions(+), 94 deletions(-)

diff --git a/packages/server/modules/activitystream/tests/activity.spec.js b/packages/server/modules/activitystream/tests/activity.spec.js
index 273ff63b02..890365eb1f 100644
--- a/packages/server/modules/activitystream/tests/activity.spec.js
+++ b/packages/server/modules/activitystream/tests/activity.spec.js
@@ -185,19 +185,9 @@ describe('Activity @activity', () => {
   }
 
   before(async () => {
-    const {
-      server: tmpServer,
-      app,
-      graphqlServer,
-      readinessCheck
-    } = await beforeEachContext()
-    server = tmpServer
-    ;({ sendRequest } = await initializeTestServer({
-      server,
-      app,
-      graphqlServer,
-      readinessCheck
-    }))
+    const ctx = await beforeEachContext()
+    server = ctx.server
+    ;({ sendRequest } = await initializeTestServer(ctx))
 
     const normalScopesList = [
       Scopes.Streams.Read,
diff --git a/packages/server/modules/auth/tests/apps.graphql.spec.js b/packages/server/modules/auth/tests/apps.graphql.spec.js
index fea5055c5c..6ca2252308 100644
--- a/packages/server/modules/auth/tests/apps.graphql.spec.js
+++ b/packages/server/modules/auth/tests/apps.graphql.spec.js
@@ -127,19 +127,9 @@ describe('GraphQL @apps-api', () => {
   let testToken2
 
   before(async () => {
-    const {
-      app,
-      server: tmpServer,
-      graphqlServer,
-      readinessCheck
-    } = await beforeEachContext()
-    server = tmpServer
-    ;({ sendRequest } = await initializeTestServer({
-      server,
-      app,
-      graphqlServer,
-      readinessCheck
-    }))
+    const ctx = await beforeEachContext()
+    server = ctx.server
+    ;({ sendRequest } = await initializeTestServer(ctx))
     testUser = {
       name: 'Dimitrie Stefanescu',
       email: 'didimitrie@gmail.com',
diff --git a/packages/server/modules/auth/tests/auth.spec.js b/packages/server/modules/auth/tests/auth.spec.js
index a8929a3d23..b97d53c5fb 100644
--- a/packages/server/modules/auth/tests/auth.spec.js
+++ b/packages/server/modules/auth/tests/auth.spec.js
@@ -161,20 +161,10 @@ describe('Auth @auth', () => {
     }
 
     before(async () => {
-      const {
-        app: tmpApp,
-        server: tmpServer,
-        graphqlServer,
-        readinessCheck
-      } = await beforeEachContext()
-      server = tmpServer
-      app = tmpApp
-      ;({ sendRequest } = await initializeTestServer({
-        server,
-        app,
-        graphqlServer,
-        readinessCheck
-      }))
+      const ctx = await beforeEachContext()
+      server = ctx.server
+      app = ctx.app
+      ;({ sendRequest } = await initializeTestServer(ctx))
 
       // Register a user for testing login flows
       await createUser(me).then((id) => (me.id = id))
diff --git a/packages/server/modules/core/tests/graph.spec.js b/packages/server/modules/core/tests/graph.spec.js
index f65a4b8bd8..c0a00cb617 100644
--- a/packages/server/modules/core/tests/graph.spec.js
+++ b/packages/server/modules/core/tests/graph.spec.js
@@ -172,20 +172,10 @@ describe('GraphQL API Core @core-api', () => {
 
   // set up app & two basic users to ping pong permissions around
   before(async () => {
-    const {
-      app: tmpApp,
-      server: tmpServer,
-      graphqlServer,
-      readinessCheck
-    } = await beforeEachContext()
-    server = tmpServer
-    app = tmpApp
-    ;({ sendRequest } = await initializeTestServer({
-      server,
-      app,
-      graphqlServer,
-      readinessCheck
-    }))
+    const ctx = await beforeEachContext()
+    server = ctx.server
+    app = ctx.app
+    ;({ sendRequest } = await initializeTestServer(ctx))
 
     userA.id = await createUser(userA)
     userA.token = `Bearer ${await createPersonalAccessToken(
diff --git a/packages/server/modules/fileuploads/tests/fileuploads.integration.spec.ts b/packages/server/modules/fileuploads/tests/fileuploads.integration.spec.ts
index b0df75c029..6026724490 100644
--- a/packages/server/modules/fileuploads/tests/fileuploads.integration.spec.ts
+++ b/packages/server/modules/fileuploads/tests/fileuploads.integration.spec.ts
@@ -147,20 +147,10 @@ describe('FileUploads @fileuploads', () => {
   let serverPort: string
 
   before(async () => {
-    const {
-      app: tmpApp,
-      server: tmpServer,
-      graphqlServer,
-      readinessCheck
-    } = await beforeEachContext()
-    server = tmpServer
-    app = tmpApp
-    ;({ serverAddress, serverPort, sendRequest } = await initializeTestServer({
-      server,
-      app,
-      graphqlServer,
-      readinessCheck
-    }))
+    const ctx = await beforeEachContext()
+    server = ctx.server
+    app = ctx.app
+    ;({ serverAddress, serverPort, sendRequest } = await initializeTestServer(ctx))
 
     //TODO does mocha have a nicer way of temporarily swapping an environment variable, like vitest?
     existingCanonicalUrl = process.env['CANONICAL_URL'] || ''
diff --git a/packages/server/modules/stats/tests/stats.spec.ts b/packages/server/modules/stats/tests/stats.spec.ts
index 186f351387..07ea82ebfe 100644
--- a/packages/server/modules/stats/tests/stats.spec.ts
+++ b/packages/server/modules/stats/tests/stats.spec.ts
@@ -289,19 +289,9 @@ describe('Server stats api @stats-api', function () {
 
   before(async function () {
     this.timeout(15000)
-    const {
-      app,
-      server: tmpServer,
-      graphqlServer,
-      readinessCheck
-    } = await beforeEachContext()
-    server = tmpServer
-    ;({ sendRequest } = await initializeTestServer({
-      server,
-      app,
-      graphqlServer,
-      readinessCheck
-    }))
+    const ctx = await beforeEachContext()
+    server = ctx.server
+    ;({ sendRequest } = await initializeTestServer(ctx))
 
     adminUser.id = await createUser(adminUser)
     adminUser.goodToken = `Bearer ${await createPersonalAccessToken(
diff --git a/packages/server/modules/webhooks/tests/webhooks.spec.js b/packages/server/modules/webhooks/tests/webhooks.spec.js
index 499ed1709e..6bc57fee09 100644
--- a/packages/server/modules/webhooks/tests/webhooks.spec.js
+++ b/packages/server/modules/webhooks/tests/webhooks.spec.js
@@ -192,19 +192,9 @@ describe('Webhooks @webhooks', () => {
   }
 
   before(async () => {
-    const {
-      app,
-      server: tmpServer,
-      graphqlServer,
-      readinessCheck
-    } = await beforeEachContext()
-    server = tmpServer
-    ;({ sendRequest } = await initializeTestServer({
-      server,
-      app,
-      graphqlServer,
-      readinessCheck
-    }))
+    const ctx = await beforeEachContext()
+    server = ctx.server
+    ;({ sendRequest } = await initializeTestServer(ctx))
 
     userOne.id = await createUser(userOne)
     streamOne.ownerId = userOne.id

From 8a8c4b86f214400efcccbc3e238dac26542692f3 Mon Sep 17 00:00:00 2001
From: Iain Sproat <68657+iainsproat@users.noreply.github.com>
Date: Fri, 15 Nov 2024 16:27:31 +0000
Subject: [PATCH 13/14] fix invalid merge

---
 packages/server/test/hooks.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/server/test/hooks.ts b/packages/server/test/hooks.ts
index b5e27881b8..b725ef3b75 100644
--- a/packages/server/test/hooks.ts
+++ b/packages/server/test/hooks.ts
@@ -225,7 +225,7 @@ export const initializeTestServer = async (params: {
   readinessCheck: ReadinessHandler
   customPortOverride?: number
 }) => {
-  await startHttp({ ...params, customPortOverride: 0 })
+  await startHttp({ ...params, customPortOverride: params.customPortOverride ?? 0 })
   const { server, app } = params
 
   await once(app, 'appStarted')

From 60e7b2a531a314225702fcbf9717c90c9cd01f04 Mon Sep 17 00:00:00 2001
From: Iain Sproat <68657+iainsproat@users.noreply.github.com>
Date: Fri, 15 Nov 2024 17:01:54 +0000
Subject: [PATCH 14/14] fix readinesscheck not being passed as parameter

---
 packages/server/test/hooks.ts | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/packages/server/test/hooks.ts b/packages/server/test/hooks.ts
index b725ef3b75..121f055155 100644
--- a/packages/server/test/hooks.ts
+++ b/packages/server/test/hooks.ts
@@ -283,8 +283,7 @@ export const mochaHooks: mocha.RootHookObject = {
 }
 
 export const buildApp = async () => {
-  const { app, graphqlServer, server } = await init()
-  return { app, graphqlServer, server }
+  return await init()
 }
 
 export const beforeEachContext = async () => {