From 48ef03d8f814832067d1323224b7b1fe973ab554 Mon Sep 17 00:00:00 2001 From: Liangjun Song <146005915+adai26@users.noreply.github.com> Date: Mon, 14 Oct 2024 15:57:13 +0800 Subject: [PATCH] Merge pull request #21011 from overleaf/ls-serve-static-wrapper add a wrapper for serve static to handle premature close error GitOrigin-RevId: 8128702d9340a893624061d07bf0da15ea457f43 --- .../src/infrastructure/ServeStaticWrapper.mjs | 34 ++++++++++ .../web/app/src/infrastructure/Server.mjs | 12 ++-- .../ServeStaticWrapperTests.mjs | 62 +++++++++++++++++++ 3 files changed, 104 insertions(+), 4 deletions(-) create mode 100644 services/web/app/src/infrastructure/ServeStaticWrapper.mjs create mode 100644 services/web/test/unit/src/infrastructure/ServeStaticWrapperTests.mjs diff --git a/services/web/app/src/infrastructure/ServeStaticWrapper.mjs b/services/web/app/src/infrastructure/ServeStaticWrapper.mjs new file mode 100644 index 0000000000..fba667e25c --- /dev/null +++ b/services/web/app/src/infrastructure/ServeStaticWrapper.mjs @@ -0,0 +1,34 @@ +import express from 'express' +import { plainTextResponse } from './Response.js' + +/* + This wrapper is implemented specifically to handle "Premature Close" errors. + These errors occur when the client cancels a request while static assets are being loaded. + This issue is beyond our control, it can result in unnecessary log noise. + Therefore, this wrapper is added to handle such errors. +*/ +function serveStaticWrapper(root, options) { + const serveStatic = express.static(root, options) + return (req, res, next) => { + serveStatic(req, res, error => { + if (!error) { + return next() + } + + if (error.code !== 'ERR_STREAM_PREMATURE_CLOSE') { + return next(error) + } + + req.logger.addFields({ err: error }) + req.logger.setLevel('debug') + if (res.headersSent) { + res.end() + } else { + res.status(400) + plainTextResponse(res, 'Premature close') + } + }) + } +} + +export default serveStaticWrapper diff --git a/services/web/app/src/infrastructure/Server.mjs b/services/web/app/src/infrastructure/Server.mjs index 66b522d80f..8f275e443f 100644 --- a/services/web/app/src/infrastructure/Server.mjs +++ b/services/web/app/src/infrastructure/Server.mjs @@ -37,6 +37,7 @@ import noCache from 'nocache' import os from 'os' import http from 'http' import { fileURLToPath } from 'url' +import serveStaticWrapper from './ServeStaticWrapper.mjs' const sessionsRedisClient = UserSessionsRedis.client() @@ -120,10 +121,13 @@ if (Settings.exposeHostname) { } webRouter.use( - express.static(fileURLToPath(new URL('../../../public', import.meta.url)), { - maxAge: STATIC_CACHE_AGE, - setHeaders: csp.removeCSPHeaders, - }) + serveStaticWrapper( + fileURLToPath(new URL('../../../public', import.meta.url)), + { + maxAge: STATIC_CACHE_AGE, + setHeaders: csp.removeCSPHeaders, + } + ) ) app.set('views', fileURLToPath(new URL('../../views', import.meta.url))) diff --git a/services/web/test/unit/src/infrastructure/ServeStaticWrapperTests.mjs b/services/web/test/unit/src/infrastructure/ServeStaticWrapperTests.mjs new file mode 100644 index 0000000000..01fe5d7a0d --- /dev/null +++ b/services/web/test/unit/src/infrastructure/ServeStaticWrapperTests.mjs @@ -0,0 +1,62 @@ +import { strict as esmock } from 'esmock' +import { expect } from 'chai' +import Path from 'node:path' +import { fileURLToPath } from 'node:url' +import sinon from 'sinon' +import MockResponse from '../helpers/MockResponse.js' +import MockRequest from '../helpers/MockRequest.js' + +const __dirname = fileURLToPath(new URL('.', import.meta.url)) +const modulePath = Path.join( + __dirname, + '../../../../app/src/infrastructure/ServeStaticWrapper' +) + +describe('ServeStaticWrapperTests', function () { + let error = null + + beforeEach(async function () { + this.req = new MockRequest() + this.res = new MockResponse() + this.express = { + static: () => (req, res, next) => { + if (error) { + next(error) + } else { + next() + } + }, + } + this.serveStaticWrapper = await esmock(modulePath, { + express: this.express, + }) + }) + + this.afterEach(() => { + error = null + }) + + it('Premature close error thrown', async function () { + error = new Error() + error.code = 'ERR_STREAM_PREMATURE_CLOSE' + const middleware = this.serveStaticWrapper('test_folder', {}) + const next = sinon.stub() + middleware(this.req, this.res, next) + expect(next.called).to.be.false + }) + + it('No error thrown', async function () { + const middleware = this.serveStaticWrapper('test_folder', {}) + const next = sinon.stub() + middleware(this.req, this.res, next) + expect(next).to.be.calledWith() + }) + + it('Other error thrown', async function () { + error = new Error() + const middleware = this.serveStaticWrapper('test_folder', {}) + const next = sinon.stub() + middleware(this.req, this.res, next) + expect(next).to.be.calledWith(error) + }) +})