(core) change handling of server access prior to full configuration

Summary:
Recently, the server became more strict about not responding to
requests before being fully configured. This is a problem when
a doc worker is trying to check whether it has become available
from a load balancer.

This change gives access to health checks prior to configuration
being complete. Otherwise, app endpoints accessed before full
configuration return a 503.

A flag is added to /status to allow checking explicitly for
the server being ready and configured.

Test Plan: manual

Reviewers: dsagal

Reviewed By: dsagal

Subscribers: dsagal

Differential Revision: https://phab.getgrist.com/D4103
This commit is contained in:
Paul Fitzpatrick 2023-10-31 17:24:48 -04:00
parent cc6265eebf
commit 4fb1df567b
2 changed files with 17 additions and 18 deletions

View File

@ -173,8 +173,6 @@ export class FlexServer implements GristServer {
private _getLogoutRedirectUrl: (req: express.Request, nextUrl: URL) => Promise<string>;
private _sendAppPage: (req: express.Request, resp: express.Response, options: ISendAppPageOptions) => Promise<void>;
private _getLoginSystem?: () => Promise<GristLoginSystem>;
// Called by ready() to allow requests to be served.
private _ready: () => void;
// Set once ready() is called
private _isReady: boolean = false;
@ -183,20 +181,6 @@ export class FlexServer implements GristServer {
this.app = express();
this.app.set('port', port);
// Before doing anything, we pause any request handling to wait
// for the server being entirely ready. The specific reason to do
// so is because, if we are serving plugins, and using an
// OS-assigned port to do so, we won't know the URL to use for
// plugins until quite late. But it seems a nice thing to
// guarantee in general.
const readyPromise = new Promise(resolve => {
this._ready = () => resolve(undefined);
});
this.app.use(async (_req, _res, next) => {
await readyPromise;
next();
});
this.appRoot = getAppRoot();
this.host = process.env.GRIST_HOST || "localhost";
log.info(`== Grist version is ${version.version} (commit ${version.gitcommit})`);
@ -452,6 +436,7 @@ export class FlexServer implements GristServer {
// /status/hooks allows the tests to wait for them to be ready.
// If db=1 query parameter is included, status will include the status of DB connection.
// If redis=1 query parameter is included, status will include the status of the Redis connection.
// If ready=1 query parameter is included, status will include whether the server is fully ready.
this.app.get('/status(/hooks)?', async (req, res) => {
const checks = new Map<string, Promise<boolean>|boolean>();
const timeout = optIntegerParam(req.query.timeout, 'timeout') || 10_000;
@ -473,6 +458,9 @@ export class FlexServer implements GristServer {
if (isParameterOn(req.query.redis)) {
checks.set('redis', asyncCheck(this._docWorkerMap.getRedisClient()?.pingAsync()));
}
if (isParameterOn(req.query.ready)) {
checks.set('ready', this._isReady);
}
let extra = '';
let ok = true;
// If we had any extra check, collect their status to report them.
@ -493,6 +481,18 @@ export class FlexServer implements GristServer {
});
}
public denyRequestsIfNotReady() {
this.app.use((_req, res, next) => {
if (!this._isReady) {
// If ready() hasn't been called yet, don't continue, and
// give a clear error. This is to avoid exposing the service
// in a partially configured form.
return res.status(503).json({error: 'Service unavailable during start up'});
}
next();
});
}
public testAddRouter() {
if (this._check('router')) { return; }
this.app.get('/test/router', (req, res) => {
@ -1568,9 +1568,7 @@ export class FlexServer implements GristServer {
}
public ready() {
if (this._isReady) { return; }
this._isReady = true;
this._ready();
}
public checkOptionCombinations() {

View File

@ -104,6 +104,7 @@ export async function main(port: number, serverTypes: ServerType[],
}
server.addHealthCheck();
server.denyRequestsIfNotReady();
if (includeHome || includeStatic || includeApp) {
server.setDirectory();