diff --git a/packages/server/app.ts b/packages/server/app.ts index 0a54fd0554..e5a7592b65 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, @@ -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 - // 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 { +export async function shutdown(params: { + graphqlServer: ApolloServer +}): Promise { + 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 + 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) + } } }) diff --git a/packages/server/bin/ts-www b/packages/server/bin/ts-www index cbddd719df..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, 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...') diff --git a/packages/server/bin/www b/packages/server/bin/www index 3309271a43..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, 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...') diff --git a/packages/server/healthchecks/index.ts b/packages/server/healthchecks/index.ts index 2c66d34801..cae174b59e 100644 --- a/packages/server/healthchecks/index.ts +++ b/packages/server/healthchecks/index.ts @@ -7,6 +7,7 @@ import { knexFreeDbConnectionSamplerFactory, isRedisAlive, isPostgresAlive, + ReadinessHandler, FreeConnectionsCalculator } from '@/healthchecks/health' import { Application } from 'express' @@ -14,7 +15,7 @@ import { Application } from 'express' export const initFactory: () => ( app: Application, isInitial: boolean -) => Promise = () => { +) => 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 }) - }) + }) + } } } diff --git a/packages/server/modules/activitystream/tests/activity.spec.js b/packages/server/modules/activitystream/tests/activity.spec.js index 7f5d24c970..890365eb1f 100644 --- a/packages/server/modules/activitystream/tests/activity.spec.js +++ b/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, diff --git a/packages/server/modules/auth/tests/apps.graphql.spec.js b/packages/server/modules/auth/tests/apps.graphql.spec.js index 1f68119805..6ca2252308 100644 --- a/packages/server/modules/auth/tests/apps.graphql.spec.js +++ b/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', diff --git a/packages/server/modules/auth/tests/auth.spec.js b/packages/server/modules/auth/tests/auth.spec.js index 56a2e2928b..b97d53c5fb 100644 --- a/packages/server/modules/auth/tests/auth.spec.js +++ b/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)) diff --git a/packages/server/modules/core/tests/graph.spec.js b/packages/server/modules/core/tests/graph.spec.js index 2c894c6fdd..c0a00cb617 100644 --- a/packages/server/modules/core/tests/graph.spec.js +++ b/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( 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/fileuploads/tests/fileuploads.integration.spec.ts b/packages/server/modules/fileuploads/tests/fileuploads.integration.spec.ts index 1f72eaf0b2..d4f30f9063 100644 --- a/packages/server/modules/fileuploads/tests/fileuploads.integration.spec.ts +++ b/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'] || '' diff --git a/packages/server/modules/shared/helpers/envHelper.ts b/packages/server/modules/shared/helpers/envHelper.ts index dec954a07e..654e9ca04d 100644 --- a/packages/server/modules/shared/helpers/envHelper.ts +++ b/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') +} diff --git a/packages/server/modules/stats/tests/stats.spec.ts b/packages/server/modules/stats/tests/stats.spec.ts index 99f1991490..07ea82ebfe 100644 --- a/packages/server/modules/stats/tests/stats.spec.ts +++ b/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>['sendRequest'], - app: Express + sendRequest: Awaited>['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( diff --git a/packages/server/modules/webhooks/tests/webhooks.spec.js b/packages/server/modules/webhooks/tests/webhooks.spec.js index d389175c3d..6bc57fee09 100644 --- a/packages/server/modules/webhooks/tests/webhooks.spec.js +++ b/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 diff --git a/packages/server/test/hooks.ts b/packages/server/test/hooks.ts index 495ab17d02..121f055155 100644 --- a/packages/server/test/hooks.ts +++ b/packages/server/test/hooks.ts @@ -37,6 +37,9 @@ import { import { Knex } from 'knex' import { isMultiRegionTestMode } from '@/test/speckle-helpers/regions' import { isMultiRegionEnabled } from '@/modules/multiregion/helpers' +import { GraphQLContext } from '@/modules/shared/helpers/typeHelper' +import { ApolloServer } from '@apollo/server' +import { ReadinessHandler } from '@/healthchecks/health' // why is server config only created once!???? // because its done in a migration, to not override existing configs @@ -215,11 +218,15 @@ export const truncateTables = async (tableNames?: string[]) => { } } -export const initializeTestServer = async ( - server: http.Server, +export const initializeTestServer = async (params: { + server: http.Server app: express.Express -) => { - await startHttp(server, app, 0) + graphqlServer: ApolloServer + readinessCheck: ReadinessHandler + customPortOverride?: number +}) => { + await startHttp({ ...params, customPortOverride: params.customPortOverride ?? 0 }) + const { server, app } = params await once(app, 'appStarted') const port = (server.address() as net.AddressInfo).port + '' @@ -246,6 +253,8 @@ export const initializeTestServer = async ( } } +let graphqlServer: ApolloServer + export const mochaHooks: mocha.RootHookObject = { beforeAll: async () => { if (isMultiRegionTestMode()) { @@ -262,20 +271,19 @@ export const mochaHooks: mocha.RootHookObject = { await setupMultiregionMode() // Init app - await init() + ;({ graphqlServer } = await init()) }, afterAll: async () => { logger.info('running after all') await inEachDb(async (db) => { await unlockFactory({ db })() }) - await shutdown() + await shutdown({ graphqlServer }) } } export const buildApp = async () => { - const { app, graphqlServer, server } = await init() - return { app, graphqlServer, server } + return await init() } export const beforeEachContext = async () => {