Merge pull request #11702 from overleaf/em-headers-sent

Don't send an error response when headers have already been sent

GitOrigin-RevId: a9832022a4f47562d31134d34bfd8c52e1eb4d7e

services/web/app/src/Features/Errors/ErrorController.js

@@ -23,6 +23,7 @@ module.exports = ErrorController = {
   },
 
   handleError(error, req, res, next) {
+    const shouldSendErrorResponse = !res.headersSent
     const user = SessionManager.getSessionUser(req.session)
     // log errors related to SAML flow
     if (req.session && req.session.saml) {
@@ -33,43 +34,66 @@ module.exports = ErrorController = {
         { err: error, url: req.url, method: req.method, user },
         'invalid csrf'
       )
-      res.sendStatus(403)
+      if (shouldSendErrorResponse) {
+        res.sendStatus(403)
+      }
     } else if (error instanceof Errors.NotFoundError) {
       logger.warn({ err: error, url: req.url }, 'not found error')
-      ErrorController.notFound(req, res)
+      if (shouldSendErrorResponse) {
+        ErrorController.notFound(req, res)
+      }
     } else if (
       error instanceof URIError &&
       error.message.match(/^Failed to decode param/)
     ) {
       logger.warn({ err: error, url: req.url }, 'Express URIError')
-      res.status(400)
-      res.render('general/500', { title: 'Invalid Error' })
+      if (shouldSendErrorResponse) {
+        res.status(400)
+        res.render('general/500', { title: 'Invalid Error' })
+      }
     } else if (error instanceof Errors.ForbiddenError) {
       logger.error({ err: error }, 'forbidden error')
-      ErrorController.forbidden(req, res)
+      if (shouldSendErrorResponse) {
+        ErrorController.forbidden(req, res)
+      }
     } else if (error instanceof Errors.TooManyRequestsError) {
       logger.warn({ err: error, url: req.url }, 'too many requests error')
-      res.sendStatus(429)
+      if (shouldSendErrorResponse) {
+        res.sendStatus(429)
+      }
     } else if (error instanceof Errors.InvalidError) {
       logger.warn({ err: error, url: req.url }, 'invalid error')
-      res.status(400)
-      plainTextResponse(res, error.message)
+      if (shouldSendErrorResponse) {
+        res.status(400)
+        plainTextResponse(res, error.message)
+      }
     } else if (error instanceof Errors.InvalidNameError) {
       logger.warn({ err: error, url: req.url }, 'invalid name error')
-      res.status(400)
-      plainTextResponse(res, error.message)
+      if (shouldSendErrorResponse) {
+        res.status(400)
+        plainTextResponse(res, error.message)
+      }
     } else if (error instanceof Errors.SAMLSessionDataMissing) {
       logger.warn(
         { err: error, url: req.url },
         'missing SAML session data error'
       )
-      HttpErrorHandler.badRequest(req, res, error.message)
+      if (shouldSendErrorResponse) {
+        HttpErrorHandler.badRequest(req, res, error.message)
+      }
     } else {
       logger.error(
         { err: error, url: req.url, method: req.method, user },
         'error passed to top level next middleware'
       )
-      ErrorController.serverError(req, res)
+      if (shouldSendErrorResponse) {
+        ErrorController.serverError(req, res)
+      }
+    }
+    if (!shouldSendErrorResponse) {
+      // Pass the error to the default Express error handler, which will close
+      // the connection.
+      next(error)
     }
   },