chore(server): graceful shutdown

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,
@@ -43,7 +48,8 @@ import {
   isApolloMonitoringEnabled,
   enableMixpanel,
   getPort,
-  getBindAddress
+  getBindAddress,
+  shutdownTimeoutSeconds
 } from '@/modules/shared/helpers/envHelper'
 import * as ModulesSetup from '@/modules'
 import { GraphQLContext, Optional } from '@/modules/shared/helpers/typeHelper'
@@ -68,11 +74,10 @@ import { loggingPlugin } from '@/modules/core/graph/plugins/logging'
 import { shouldLogAsInfoLevel } from '@/logging/graphqlError'
 import { getUserFactory } from '@/modules/core/repositories/users'
 import { initFactory as healthchecksInitFactory } from '@/healthchecks'
+import type { ReadinessHandler } from '@/healthchecks/health'
 
 const GRAPHQL_PATH = '/graphql'
 
-let graphqlServer: ApolloServer<GraphQLContext>
-
 // eslint-disable-next-line @typescript-eslint/no-explicit-any
 type SubscriptionResponse = { errors?: GraphQLError[]; data?: any }
 
@@ -334,7 +339,10 @@ export async function buildApolloServer(options?: {
     persistedQueries: false,
     csrfPrevention: true,
     formatError: buildErrorFormatter({ includeStacktraceInErrorResponses }),
-    includeStacktraceInErrorResponses
+    includeStacktraceInErrorResponses,
+    status400ForVariableCoercionErrors: true,
+    stopOnTerminationSignals: false, // handled by terminus and shutdown function
+    logger: graphqlLogger
   })
   await server.start()
 
@@ -403,14 +411,14 @@ export async function init() {
   await ModulesSetup.init(app)
 
   // Initialize healthchecks
-  await healthchecksInitFactory()(app, true)
+  const healthchecks = await healthchecksInitFactory()(app, true)
 
   // Init HTTP server & subscription server
   const server = http.createServer(app)
   const subscriptionServer = buildApolloSubscriptionServer(server)
 
   // Initialize graphql server
-  graphqlServer = await buildApolloServer({
+  const graphqlServer = await buildApolloServer({
     subscriptionServer
   })
   app.use(
@@ -433,10 +441,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(): Promise<void> {
+export async function shutdown(params: {
+  graphqlServer: ApolloServer<GraphQLContext>
+}): Promise<void> {
+  await params.graphqlServer.stop()
   await ModulesSetup.shutdown()
 }
 
@@ -463,11 +480,14 @@ 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>
+  readinessCheck: ReadinessHandler
   customPortOverride?: number
-) {
+}) {
+  const { server, app, graphqlServer, readinessCheck, customPortOverride } = params
   let bindAddress = getBindAddress() // defaults to 127.0.0.1
   let port = getPort() // defaults to 3000
 
@@ -492,16 +512,30 @@ export async function startHttp(
   // large timeout to allow large downloads on slow connections to finish
   createTerminus(server, {
     signals: ['SIGTERM', 'SIGINT'],
-    timeout: 5 * 60 * 1000,
+    timeout: shutdownTimeoutSeconds() * 1000,
     beforeShutdown: async () => {
       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': 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) => {
+      if (err) {
+        shutdownLogger.error({ err }, message)
+      } else {
+        shutdownLogger.info(message)
+      }
     }
   })
 

packages/server/bin/ts-www

@@ -11,7 +11,9 @@ const { logger } = require('../logging/logging')
 const { init, startHttp } = require('../app')
 
 init()
-  .then(({ app, server }) => startHttp(server, 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...')
 

packages/server/bin/www

@@ -5,7 +5,9 @@ const { logger } = require('../dist/logging/logging')
 const { init, startHttp } = require('../dist/app')
 
 init()
-  .then(({ app, server }) => startHttp(server, 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...')
 

packages/server/healthchecks/index.ts

@@ -7,14 +7,15 @@ import {
   knexFreeDbConnectionSamplerFactory,
   isRedisAlive,
   isPostgresAlive,
+  ReadinessHandler,
   FreeConnectionsCalculator
 } from '@/healthchecks/health'
 import { Application } from 'express'
 
 export const initFactory: () => (
   app: Application,
   isInitial: boolean
-) => Promise<void> = () => {
+) => Promise<{ isReady: ReadinessHandler }> = () => {
   let knexFreeDbConnectionSamplerLiveness: FreeConnectionsCalculator & {
     start: () => void
   }
@@ -43,19 +44,17 @@ export const initFactory: () => (
       isPostgresAlive,
       freeConnectionsCalculator: knexFreeDbConnectionSamplerLiveness
     })
-
     app.get('/liveness', async (req, res) => {
       const result = await livenessHandler()
       res.status(200).json({ status: 'ok', ...result })
     })
 
-    app.get('/readiness', async (req, res) => {
-      const result = await handleReadinessFactory({
+    return {
+      isReady: handleReadinessFactory({
         isRedisAlive,
         isPostgresAlive,
         freeConnectionsCalculator: knexFreeDbConnectionSamplerReadiness
-      })()
-      res.status(200).json({ status: 'ok', ...result })
-    })
+      })
+    }
   }
 }

packages/server/modules/activitystream/tests/activity.spec.js

@@ -125,12 +125,10 @@ const createObject = createObjectFactory({
   storeClosuresIfNotFound: storeClosuresIfNotFoundFactory({ db })
 })
 
+let server
 let sendRequest
 
 describe('Activity @activity', () => {
-  let server
-  let app
-
   const userIz = {
     name: 'Izzy Lyseggen',
     email: 'izzybizzi@speckle.systems',
@@ -187,8 +185,9 @@ describe('Activity @activity', () => {
   }
 
   before(async () => {
-    ;({ server, app } = await beforeEachContext())
-    ;({ sendRequest } = await initializeTestServer(server, app))
+    const ctx = await beforeEachContext()
+    server = ctx.server
+    ;({ sendRequest } = await initializeTestServer(ctx))
 
     const normalScopesList = [
       Scopes.Streams.Read,

packages/server/modules/auth/tests/apps.graphql.spec.js

@@ -64,7 +64,6 @@ const { getServerInfoFactory } = require('@/modules/core/repositories/server')
 
 let sendRequest
 let server
-let app
 
 const createAppToken = createAppTokenFactory({
   storeApiToken: storeApiTokenFactory({ db }),
@@ -128,8 +127,9 @@ describe('GraphQL @apps-api', () => {
   let testToken2
 
   before(async () => {
-    ;({ app, server } = await beforeEachContext())
-    ;({ sendRequest } = await initializeTestServer(server, app))
+    const ctx = await beforeEachContext()
+    server = ctx.server
+    ;({ sendRequest } = await initializeTestServer(ctx))
     testUser = {
       name: 'Dimitrie Stefanescu',
       email: 'didimitrie@gmail.com',

packages/server/modules/auth/tests/auth.spec.js

@@ -161,8 +161,10 @@ describe('Auth @auth', () => {
     }
 
     before(async () => {
-      ;({ app, server } = await beforeEachContext())
-      ;({ sendRequest } = await initializeTestServer(server, app))
+      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))

packages/server/modules/core/tests/graph.spec.js

@@ -172,8 +172,10 @@ 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 ctx = await beforeEachContext()
+    server = ctx.server
+    app = ctx.app
+    ;({ sendRequest } = await initializeTestServer(ctx))
 
     userA.id = await createUser(userA)
     userA.token = `Bearer ${await createPersonalAccessToken(

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')
   })
 })

packages/server/modules/fileuploads/tests/fileuploads.integration.spec.ts

@@ -147,11 +147,10 @@ describe('FileUploads @fileuploads', () => {
   let serverPort: string
 
   before(async () => {
-    ;({ app, server } = await beforeEachContext())
-    ;({ serverAddress, serverPort, sendRequest } = await initializeTestServer(
-      server,
-      app
-    ))
+    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'] || ''

packages/server/modules/shared/helpers/envHelper.ts

@@ -430,3 +430,7 @@ export function getMultiRegionConfigPath(options?: Partial<{ unsafe: boolean }>)
 
 export const shouldRunTestsInMultiregionMode = () =>
   getBooleanFromEnv('RUN_TESTS_IN_MULTIREGION_MODE')
+
+export function shutdownTimeoutSeconds() {
+  return getIntFromEnv('SHUTDOWN_TIMEOUT_SECONDS', '300')
+}

packages/server/modules/stats/tests/stats.spec.ts

@@ -15,7 +15,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'
 import {
   createCommitByBranchIdFactory,
@@ -253,8 +252,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',
@@ -291,8 +289,9 @@ describe('Server stats api @stats-api', function () {
 
   before(async function () {
     this.timeout(15000)
-    ;({ app, server } = await beforeEachContext())
-    ;({ sendRequest } = await initializeTestServer(server, app))
+    const ctx = await beforeEachContext()
+    server = ctx.server
+    ;({ sendRequest } = await initializeTestServer(ctx))
 
     adminUser.id = await createUser(adminUser)
     adminUser.goodToken = `Bearer ${await createPersonalAccessToken(

packages/server/modules/webhooks/tests/webhooks.spec.js

@@ -168,7 +168,7 @@ const createPersonalAccessToken = createPersonalAccessTokenFactory({
 
 describe('Webhooks @webhooks', () => {
   const getWebhook = getWebhookByIdFactory({ db })
-  let server, sendRequest, app
+  let server, sendRequest
 
   const userOne = {
     name: 'User',
@@ -192,8 +192,9 @@ describe('Webhooks @webhooks', () => {
   }
 
   before(async () => {
-    ;({ app, server } = await beforeEachContext())
-    ;({ sendRequest } = await initializeTestServer(server, app))
+    const ctx = await beforeEachContext()
+    server = ctx.server
+    ;({ sendRequest } = await initializeTestServer(ctx))
 
     userOne.id = await createUser(userOne)
     streamOne.ownerId = userOne.id