Repository: compiler-explorer/compiler-explorer
Pull request #7145 (2024-11-28 15:57:51)

Add controller for healthcheck endpoint
Follow up to building separate express routers for each domain. Now handles the healthcheck in a separate controller


Patch:
diff --git a/app.ts b/app.ts
index 2b8054a81..12c70e5df 100755
--- a/app.ts
+++ b/app.ts
@@ -58,10 +58,10 @@ import {RemoteExecutionQuery} from './lib/execution/execution-query.js';
 import {initHostSpecialties} from './lib/execution/execution-triple.js';
 import {startExecutionWorkerThread} from './lib/execution/sqs-execution-queue.js';
 import {AssemblyDocumentationController} from './lib/handlers/api/assembly-documentation-controller.js';
+import {HealthcheckController} from './lib/handlers/api/healthcheck-controller.js';
 import {SiteTemplateController} from './lib/handlers/api/site-template-controller.js';
 import {SourceController} from './lib/handlers/api/source-controller.js';
 import {CompileHandler} from './lib/handlers/compile.js';
-import * as healthCheck from './lib/handlers/health-check.js';
 import {NoScriptHandler} from './lib/handlers/noscript.js';
 import {RouteAPI, ShortLinkMetaData} from './lib/handlers/route-api.js';
 import {languages as allLanguages} from './lib/languages.js';
@@ -558,9 +558,18 @@ async function main() {
     const storageHandler = new storageType(httpRoot, compilerProps, awsProps);
     const compilerFinder = new CompilerFinder(compileHandler, compilerProps, awsProps, defArgs, clientOptionsHandler);
 
+    const isExecutionWorker = ceProps<boolean>('execqueue.is_worker', false);
+    const healthCheckFilePath = ceProps('healthCheckFilePath', null) as string | null;
+
     const siteTemplateController = new SiteTemplateController();
     const sourceController = new SourceController(sources);
     const assemblyDocumentationController = new AssemblyDocumentationController();
+    const healthCheckController = new HealthcheckController(
+        compilationQueue,
+        healthCheckFilePath,
+        compileHandler,
+        isExecutionWorker,
+    );
 
     logger.info('=======================================');
     if (gitReleaseName) logger.info(`  git release ${gitReleaseName}`);
@@ -569,8 +578,6 @@ async function main() {
     let initialCompilers: CompilerInfo[];
     let prevCompilers: CompilerInfo[];
 
-    const isExecutionWorker = ceProps<boolean>('execqueue.is_worker', false);
-
     if (opts.prediscovered) {
         const prediscoveredCompilersJson = await fs.readFile(opts.prediscovered, 'utf8');
         initialCompilers = JSON.parse(prediscoveredCompilersJson);
@@ -611,8 +618,6 @@ async function main() {
         process.exit(0);
     }
 
-    const healthCheckFilePath = ceProps('healthCheckFilePath', false) as string | false;
-
     // Exported to allow compilers to refer to other existing compilers.
     global.handler_config = {
         compileHandler,
@@ -683,12 +688,6 @@ async function main() {
                 }
             }),
         )
-        // Handle healthchecks at the root, as they're not expected from the outside world
-        .use(
-            '/healthcheck',
-            new healthCheck.HealthCheckHandler(compilationQueue, healthCheckFilePath, compileHandler, isExecutionWorker)
-                .handle,
-        )
         .use(httpRoot, router)
         .use((req, res, next) => {
             next({status: 404, message: `page "${req.path}" could not be found`});
@@ -875,6 +874,7 @@ async function main() {
         .use(siteTemplateController.createRouter())
         .use(sourceController.createRouter())
         .use(assemblyDocumentationController.createRouter())
+        .use(healthCheckController.createRouter())
         .get('/g/:id', oldGoogleUrlHandler)
         // Deprecated old route for this -- TODO remove in late 2021
         .post('/shortener', routeApi.apiHandler.shortener.handle.bind(routeApi.apiHandler.shortener));
diff --git a/lib/handlers/api/healthcheck-controller.ts b/lib/handlers/api/healthcheck-controller.ts
new file mode 100644
index 000000000..b45e642cb
--- /dev/null
+++ b/lib/handlers/api/healthcheck-controller.ts
@@ -0,0 +1,87 @@
+// Copyright (c) 2024, Compiler Explorer Authors
+// All rights reserved.
+//
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are met:
+//
+//     * Redistributions of source code must retain the above copyright notice,
+//       this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above copyright
+//       notice, this list of conditions and the following disclaimer in the
+//       documentation and/or other materials provided with the distribution.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+// AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+// IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+// ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE
+// LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+// CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+// SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+// INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+// CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+// ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+// POSSIBILITY OF SUCH DAMAGE.
+
+import fs from 'node:fs/promises';
+
+import express from 'express';
+
+import {CompilationQueue} from '../../compilation-queue.js';
+import {logger} from '../../logger.js';
+import {SentryCapture} from '../../sentry.js';
+import {ICompileHandler} from '../compile.interfaces.js';
+
+import {HttpController} from './controller.interfaces.js';
+
+export class HealthcheckController implements HttpController {
+    public constructor(
+        private readonly compilationQueue: CompilationQueue,
+        private readonly healthCheckFilePath: string | null,
+        private readonly compileHandler: ICompileHandler,
+        private readonly isExecutionWorker: boolean,
+    ) {}
+
+    createRouter(): express.Router {
+        const router = express.Router();
+        // TODO: Consider if this could be `GET` only. It doesn't quite make sense to do anything but GET the
+        //  healthcheck
+        router.use('/healthcheck', this.healthcheck.bind(this));
+        return router;
+    }
+
+    /**
+     * Handle request to `/healthcheck` endpoint
+     * @param req
+     * @param res
+     */
+    public async healthcheck(req: express.Request, res: express.Response) {
+        // Enqueue an empty job to ensure the queue is running. This check simply tests that the queue is being
+        // processed by _something_.
+        await this.compilationQueue.enqueue(async () => {}, {highPriority: true});
+
+        // If this is a worker, we don't require that the server has languages configured.
+        if (!this.isExecutionWorker && !this.compileHandler.hasLanguages()) {
+            logger.error(`*** HEALTH CHECK FAILURE: no languages/compilers detected`);
+            return res.status(500).send();
+        }
+
+        // If we have a healthcheck file, we require that it exists and it is non-empty. The /efs/.health file contents
+        // are not important, but the file acts as a health check for EFS mounts.
+        if (this.healthCheckFilePath !== null) {
+            try {
+                const content = await fs.readFile(this.healthCheckFilePath);
+                if (content.length === 0) {
+                    throw new Error('File is empty');
+                }
+                res.set('Content-Type', 'text/html');
+                return res.send(content);
+            } catch (e) {
+                logger.error(`*** HEALTH CHECK FAILURE: while reading file '${this.healthCheckFilePath}' got ${e}`);
+                SentryCapture(e, 'Health check');
+                return res.status(500).send();
+            }
+        }
+
+        return res.send('Everything is awesome');
+    }
+}
diff --git a/lib/handlers/health-check.ts b/lib/handlers/health-check.ts
deleted file mode 100644
index 95071a90f..000000000
--- a/lib/handlers/health-check.ts
+++ /dev/null
@@ -1,81 +0,0 @@
-// Copyright (c) 2017, Compiler Explorer Authors
-// All rights reserved.
-//
-// Redistribution and use in source and binary forms, with or without
-// modification, are permitted provided that the following conditions are met:
-//
-//     * Redistributions of source code must retain the above copyright notice,
-//       this list of conditions and the following disclaimer.
-//     * Redistributions in binary form must reproduce the above copyright
-//       notice, this list of conditions and the following disclaimer in the
-//       documentation and/or other materials provided with the distribution.
-//
-// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
-// AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
-// IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
-// ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE
-// LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
-// CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
-// SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
-// INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
-// CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
-// ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
-// POSSIBILITY OF SUCH DAMAGE.
-
-import express from 'express';
-import fs from 'fs-extra';
-
-import {CompilationQueue} from '../compilation-queue.js';
-import {logger} from '../logger.js';
-import {SentryCapture} from '../sentry.js';
-
-import {ICompileHandler} from './compile.interfaces.js';
-
-export class HealthCheckHandler {
-    public readonly handle: (req: express.Request, res: express.Response) => Promise<void>;
-
-    constructor(
-        private readonly compilationQueue: CompilationQueue,
-        private readonly filePath: string | false,
-        private readonly compileHandler: ICompileHandler,
-        private readonly isExecutionWorker: boolean,
-    ) {
-        this.handle = this._handle.bind(this);
-    }
-
-    async _handle(req: express.Request, res: express.Response) {
-        /* wait on an empty job to pass through the compilation queue
-         * to ensure the health check will timeout if it is deadlocked
-         * we set the priority to super high here and rely on the fact
-         * that timed out jobs will auto-cancel. this health check then
-         * is just testing that _something_ is still processing jobs in
-         * the async queue.
-         *
-         * we perform the remainder of the health check outside of the
-         * job to minimize the duration that we hold an execution slot
-         */
-        await this.compilationQueue.enqueue(async () => {}, {highPriority: true});
-
-        if (!this.isExecutionWorker && !this.compileHandler.hasLanguages()) {
-            logger.error(`*** HEALTH CHECK FAILURE: no languages/compilers detected`);
-            res.status(500).end();
-            return;
-        }
-
-        if (!this.filePath) {
-            res.send('Everything is awesome');
-            return;
-        }
-
-        try {
-            const content = await fs.readFile(this.filePath);
-            if (content.length === 0) throw new Error('File is empty');
-            res.set('Content-Type', 'text/html');
-            res.send(content);
-        } catch (e) {
-            logger.error(`*** HEALTH CHECK FAILURE: while reading file '${this.filePath}' got ${e}`);
-            SentryCapture(e, 'Health check');
-            res.status(500).end();
-        }
-    }
-}


Test patch:
diff --git a/test/handlers/health-check-tests.ts b/test/handlers/health-check-tests.ts
index 426de43bd..7aa7185ae 100644
--- a/test/handlers/health-check-tests.ts
+++ b/test/handlers/health-check-tests.ts
@@ -28,11 +28,11 @@ import request from 'supertest';
 import {afterAll, beforeAll, beforeEach, describe, expect, it} from 'vitest';
 
 import {CompilationQueue} from '../../lib/compilation-queue.js';
-import {HealthCheckHandler} from '../../lib/handlers/health-check.js';
+import {HealthcheckController} from '../../lib/handlers/api/healthcheck-controller.js';
 
 describe('Health checks', () => {
-    let app;
-    let compilationQueue;
+    let app: express.Express;
+    let compilationQueue: CompilationQueue;
 
     beforeEach(() => {
         const compileHandlerMock = {
@@ -40,26 +40,28 @@ describe('Health checks', () => {
         };
         compilationQueue = new CompilationQueue(1, 0, 0);
         app = express();
-        app.use('/hc', new HealthCheckHandler(compilationQueue, '', compileHandlerMock, false).handle);
+        const controller = new HealthcheckController(compilationQueue, null, compileHandlerMock, false);
+        app.use(controller.createRouter());
     });
 
     it('should respond with OK', async () => {
-        await request(app).get('/hc').expect(200, 'Everything is awesome');
+        await request(app).get('/healthcheck').expect(200, 'Everything is awesome');
     });
 
     it('should use compilation queue', async () => {
         let count = 0;
+        // @ts-expect-error: bypass the private _queue property
         compilationQueue._queue.on('active', () => {
             count++;
         });
-        await request(app).get('/hc');
+        await request(app).get('/healthcheck');
         expect(count).toEqual(1);
     });
 });
 
 describe('Health checks without lang/comp', () => {
-    let app;
-    let compilationQueue;
+    let app: express.Express;
+    let compilationQueue: CompilationQueue;
 
     beforeEach(() => {
         const compileHandlerMock = {
@@ -67,17 +69,18 @@ describe('Health checks without lang/comp', () => {
         };
         compilationQueue = new CompilationQueue(1, 0, 0);
         app = express();
-        app.use('/hc', new HealthCheckHandler(compilationQueue, '', compileHandlerMock, false).handle);
+        const controller = new HealthcheckController(compilationQueue, null, compileHandlerMock, false);
+        app.use(controller.createRouter());
     });
 
     it('should respond with error', async () => {
-        await request(app).get('/hc').expect(500);
+        await request(app).get('/healthcheck').expect(500);
     });
 });
 
 describe('Health checks without lang/comp but in execution worker mode', () => {
-    let app;
-    let compilationQueue;
+    let app: express.Express;
+    let compilationQueue: CompilationQueue;
 
     beforeEach(() => {
         const compileHandlerMock = {
@@ -85,16 +88,18 @@ describe('Health checks without lang/comp but in execution worker mode', () => {
         };
         compilationQueue = new CompilationQueue(1, 0, 0);
         app = express();
-        app.use('/hc', new HealthCheckHandler(compilationQueue, '', compileHandlerMock, true).handle);
+        const controller = new HealthcheckController(compilationQueue, null, compileHandlerMock, true);
+        app.use(controller.createRouter());
     });
 
     it('should respond with ok', async () => {
-        await request(app).get('/hc').expect(200);
+        await request(app).get('/healthcheck').expect(200);
     });
 });
 
 describe('Health checks on disk', () => {
-    let app;
+    let healthyApp: express.Express;
+    let unhealthyApp: express.Express;
 
     beforeAll(() => {
         const compileHandlerMock = {
@@ -102,9 +107,13 @@ describe('Health checks on disk', () => {
         };
         const compilationQueue = new CompilationQueue(1, 0, 0);
 
-        app = express();
-        app.use('/hc', new HealthCheckHandler(compilationQueue, '/fake/.nonexist', compileHandlerMock, false).handle);
-        app.use('/hc2', new HealthCheckHandler(compilationQueue, '/fake/.health', compileHandlerMock, false).handle);
+        healthyApp = express();
+        const hc1 = new HealthcheckController(compilationQueue, '/fake/.health', compileHandlerMock, false);
+        healthyApp.use(hc1.createRouter());
+
+        unhealthyApp = express();
+        const hc2 = new HealthcheckController(compilationQueue, '/fake/.nonexist', compileHandlerMock, false);
+        unhealthyApp.use(hc2.createRouter());
 
         mockfs({
             '/fake': {
@@ -118,10 +127,13 @@ describe('Health checks on disk', () => {
     });
 
     it('should respond with 500 when file not found', async () => {
-        await request(app).get('/hc').expect(500);
+        await request(unhealthyApp).get('/healthcheck').expect(500);
     });
 
     it('should respond with OK and file contents when found', async () => {
-        await request(app).get('/hc2').expect(200, 'Everything is fine');
+        await request(healthyApp)
+            .get('/healthcheck')
+            .expect(200, 'Everything is fine')
+            .expect('content-type', /text\/html/);
     });
 });