fix: prevent express errors when headers have already been sent

jankapunkt · jankapunkt · commit 8bb68bb3438d · 2024-08-07T12:03:22.000+02:00
diff --git a/package.json b/package.json
@@ -25,7 +25,7 @@
     "test": "meteor npm run setup-test && meteor npm run test:once",
     "test:once": "cd someapp && BABEL_ENV=COVERAGE TEST_BROWSER_DRIVER=puppeteer COVERAGE_VERBOSE=1 COVERAGE=1 COVERAGE_OUT_LCOVONLY=1 COVERAGE_APP_FOLDER=$(pwd)/ meteor test-packages --once --driver-package meteortesting:mocha ./packages/meteor-coverage",
     "test:watch": "cd someapp && BABEL_ENV=COVERAGE TEST_WATCH=1 COVERAGE_VERBOSE=1 COVERAGE=1 COVERAGE_APP_FOLDER=$(pwd)/ meteor test-packages --driver-package meteortesting:mocha ./packages/meteor-coverage",
-    "test:headless": "cd someapp && BABEL_ENV=COVERAGE TEST_BROWSER_DRIVER=puppeteer COVERAGE_VERBOSE=1 COVERAGE=1 COVERAGE_OUT_LCOVONLY=1 COVERAGE_APP_FOLDER=$(pwd)/ meteor test-packages --driver-package meteortesting:mocha ./packages/meteor-coverage",
+    "test:headless": "cd someapp && BABEL_ENV=COVERAGE TEST_BROWSER_DRIVER=puppeteer COVERAGE=1 COVERAGE_APP_FOLDER=$(pwd)/ meteor test-packages --driver-package meteortesting:mocha ./packages/meteor-coverage",
     "start": "meteor npm run lint:fix & meteor npm run test:watch",
     "lint": "eslint .",
     "lint:fix": "eslint --fix .",
diff --git a/server/handlers.js b/server/handlers.js
@@ -4,6 +4,7 @@ import ReportService from './report/report-service';
 import fs from 'node:fs';
 import path from 'node:path';
 import Log from './context/log';
+import { Response } from './services/response';
 
 showCoverage = function (params, req, res, next) {
   let options = {
@@ -24,20 +25,20 @@ getAsset = function (params, req, res, next) {
         fs.readFile(assetsDir + '/vendor/' + filename, function (err, fileContent) {
           /* istanbul ignore else */
           if (err) {
-            console.error(err);
+            Log.error(err);
             return next();
           }
-          res.json(fileContent);
+          Response.send({ res, message: fileContent });
         });
       });
     } else {
       fs.readFile(assetsDir + '/' + filename, function (err, fileContent) {
         /* istanbul ignore else */
         if (err) {
-          console.error(err);
+          Log.error(err);
           return next();
         }
-        res.json(fileContent);
+        Response.send({ res, message: fileContent });
       });
     }
   });
@@ -47,8 +48,11 @@ addClientCoverage = function (params, req, res, next) {
   var body = req.body;
   /* istanbul ignore else */
   if (!body) {
-    res.status(400);
-    return res.end();
+    return Response.send({
+      res,
+      status: 400,
+      end: true
+    });
   }
 
   var clientCoverage;
@@ -60,10 +64,16 @@ addClientCoverage = function (params, req, res, next) {
   }
   if (clientCoverage) {
     Core.mergeCoverageWith(clientCoverage);
-    res.json({ type: 'success' });
+    Response.send({
+      res,
+      json: { type: 'success' }
+    });
   } else {
-    res.status(400);
-    res.send('Nothing has been imported');
+    Response.send({
+      res,
+      status: 400,
+      message: 'Nothing has been imported'
+    });
   }
 };
 
@@ -76,17 +86,23 @@ exportFile = function (params, req, res, next) {
     reportService.generateReport(res, type, {});
   } catch (e) {
     Log.error('Failed to export', e, e.stack);
-    res.status(400);
-    res.send('Nothing has been export');
+    Response.send({
+      res,
+      status: 400,
+      message: 'Nothing has been export'
+    });
   }
 };
 importCoverage = function (params, req, res, next) {
   try {
     Core.importCoverage(res);
   } catch (e) {
     Log.error('Failed to import', e, e.stack);
-    res.status(400);
-    res.send('No file has been import');
+    Response.send({
+      res,
+      status: 400,
+      message: 'No file has been import'
+    });
   }
 };
 
diff --git a/server/report/report-coverage.js b/server/report/report-coverage.js
@@ -1,4 +1,5 @@
 import fs from 'node:fs';
+import { Response } from '../services/response';
 
 export default class {
   constructor(res, options) {
@@ -15,9 +16,15 @@ export default class {
     fs.writeFile(reportPath, coverageReport, function (err) {
       /* istanbul ignore else */
       if (err) {
-        instance.res.json({ type: 'failed', message: 'failed to write report file: ' + reportPath });
+        Response.send({
+          res: instance.res,
+          json: { type: 'failed', message: 'failed to write report file: ' + reportPath }
+        });
       } else {
-        instance.res.json({ type: 'success' });
+        Response.send({
+          res: instance.res,
+          json: { type: 'success' }
+        });
       }
     });
   }
diff --git a/server/report/report-generic.js b/server/report/report-generic.js
@@ -3,6 +3,7 @@ import Core from './../services/core';
 import ReportCommon from './report-common';
 import Conf from '../context/conf';
 import Log from '../context/log';
+import { Response } from '../services/response';
 
 const ReportImpl = Npm.require('istanbul-reports');
   
@@ -22,26 +23,33 @@ export default class {
 
   generate() {
     const coverage = Core.getCoverageObject();
-    let childs = CoverageData.getLcovonlyReport(coverage);
+    let children = CoverageData.getLcovonlyReport(coverage);
     this.report.onStart(null, this.context);
     /* istanbul ignore else */
-    if (childs.length === 0) {
-      this.res.set('Content-type', 'text/plain');
-      this.res.status(500);
-      return this.res.json({'type':'No coverage to export'});
+    if (children.length === 0) {
+      return Response.send({
+        res: this.res,
+        status: 500,
+        type: 'application/json',
+        json: {'type':'No coverage to export'}
+      });
     }
 
-    this.writeFile(childs);
-    this.res.json({ type: 'success' });
+    this.writeFile(children);
+    Response.send({
+      res: this.res,
+      type: 'application/json',
+      json: { type: 'success' }
+    });
   }
 
   writeFile(children) {
     for (let i = 0; i < children.length; i++) {
       // Remove the COVERAGE_APP_FOLDER from the filepath
       try {
         children[i].fileCoverage.data.path = children[i].fileCoverage.data.path.replace(Conf.COVERAGE_APP_FOLDER, '');
-      } catch {
-        // eslint-disable no-empty
+      } catch (e) {
+        Log.error(e);
       }
       this.report.onDetail(children[i]);
     }
diff --git a/server/report/report-html.js b/server/report/report-html.js
@@ -4,6 +4,8 @@ import fs from 'node:fs';
 import path from 'node:path';
 import ReportCommon from './report-common';
 import Log from './../context/log';
+import { Response } from '../services/response';
+
 const Report = Npm.require('istanbul-lib-report'),
   ReportImpl = Npm.require('istanbul-reports');
 
@@ -54,11 +56,17 @@ export default class {
 
     /* istanbul ignore else */
     if (!(coverage && Object.keys(coverage).length > 0)) {
-      this.res.status(500);
-      return this.res.json({
-        type:'failed',
-        message: 'No coverage information have been collected'
+      return Response.send({
+        res: this.res,
+        status: 500,
+        type: 'application/json',
+        json: {
+          type:'failed',
+          status: 500,
+          message: 'No coverage information have been collected'
+        }
       });
+
     }
     let root = CoverageData.getTreeReport(coverage);
     let filepath = path.join(folderPath, 'index.html');
@@ -77,7 +85,11 @@ export default class {
       report.onDetail(fileReport, reportCtx);
     });
 
-    this.res.json({ type: 'success' });
+    Response.send({
+      res: this.res,
+      type: 'application/json',
+      json: { type: 'success' }
+    });
   }
 
   copyStatic() {
diff --git a/server/report/report-http.js b/server/report/report-http.js
@@ -1,6 +1,7 @@
 import CoverageData from '../services/coverage-data';
 import Conf from '../context/conf';
 import Core from '../services/core';
+import { Response } from '../services/response';
 // If we change Npm.require('istanbul-reports') into import a from 'istanbul-reports'
 // the __dirname change and the  istanbul dependency fails
 // See istanbul-reports
@@ -43,8 +44,11 @@ export default class {
     var coverage = Core.getCoverageObject();
     /* istanbul ignore else */
     if (!(coverage && Object.keys(coverage).length > 0)) {
-      this.res.set('Content-type', 'text/plain');
-      return this.res.send('No coverage information has been collected');
+      return Response.send({
+        res: this.res,
+        type: 'text/plain',
+        message: 'No coverage information has been collected'
+      });
     }
     this.res.set('Content-type', 'text/html');
     this.alterFS(this.res);
@@ -79,7 +83,9 @@ export default class {
   // istanbul-reports expect to save HTML report to the file system and not over network
   alterFS(res) {
     res.close = function () {
-      this.end();
+      if (!res.headersSent) {
+        this.end();
+      }
     };
   }
 
diff --git a/server/report/report-json-summary.js b/server/report/report-json-summary.js
@@ -2,6 +2,9 @@ import Conf from './../context/conf';
 import CoverageData from './../services/coverage-data';
 import Core from './../services/core';
 import ReportCommon from './report-common';
+import Log from '../context/log';
+import { Response } from '../services/response';
+
 const ReportImpl = Npm.require('istanbul-reports');
 
 export default class {
@@ -18,28 +21,35 @@ export default class {
 
   generate() {
     const coverage = Core.getCoverageObject();
-    let childs = CoverageData.getLcovonlyReport(coverage);
+    let children = CoverageData.getLcovonlyReport(coverage);
     this.report.onStart(null, this.context);
-    /* istanbul ignore else */
-    this.res.set('Content-type', 'application/json');
 
-    if (childs.length === 0) {
-      this.res.status(500);
-      return this.res.json({'type':'No coverage to export'});
-    }
-    this.writeFile(childs);
-    this.res.json({ type: 'success' });
+    if (children.length === 0) {
+      return Response.send({
+        res: this.res,
+        status: 500,
+        type: 'application/json',
+        json: {'type':'No coverage to export'}
+      });
+    } 
+    this.writeFile(children);
+    Response.send({
+      res: this.res,
+      type: 'application/json',
+      json: { type: 'success' }
+    });
+    
   }
 
-  writeFile (childs) {
-    for (let i = 0; i < childs.length; i++) {
+  writeFile (children) {
+    for (let i = 0; i < children.length; i++) {
       // Remove the COVERAGE_APP_FOLDER from the filepath
       try {
-        childs[i].fileCoverage.data.path = childs[i].fileCoverage.data.path.replace(Conf.COVERAGE_APP_FOLDER, '');
-      } catch {
-        // eslint-disable no-empty
+        children[i].fileCoverage.data.path = children[i].fileCoverage.data.path.replace(Conf.COVERAGE_APP_FOLDER, '');
+      } catch (e) {
+        Log.error(e);
       }
-      this.report.onDetail(childs[i]);
+      this.report.onDetail(children[i]);
     }
     ///Todo: not working
     //this.report.onSummary(childs);
diff --git a/server/report/report-remap.js b/server/report/report-remap.js
@@ -3,6 +3,8 @@ import ReportCommon from './report-common';
 import IstanbulGenericReporter from './report-generic';
 import path from 'node:path';
 import Log from '../context/log';
+import { Response } from '../services/response';
+
 const remapIstanbul = Npm.require('remap-istanbul');
 const MemoryStore = Npm.require('istanbul/lib/store/memory');
 
@@ -59,11 +61,15 @@ export default class {
     this.remapWrapper(this.pathJSON, reports, this.options)
       .catch(e => {
         Log.error(e);
-        res.status(500);
-        res.send(e.message);
+        Response.send({
+          res,
+          status: 500,
+          type: 'text/plain',
+          message: e.message
+        });
       })
       .finally(() => {
-      // Restore previous working directory
+        // Restore previous working directory
         process.chdir(cwd);
       });
   }
@@ -82,7 +88,7 @@ export default class {
     }
 
     let p = Object.keys(reports).map((reportType) => {
-      let reportOptions = Object.assign({}, this.options, {verbose: reportType !== 'html'});
+      let reportOptions = Object.assign({}, options, {verbose: reportType !== 'html'});
       return remapIstanbul.writeReport(collector, reportType, reportOptions, reports[reportType], sourceStore);
     });
 
diff --git a/server/report/report-service.js b/server/report/report-service.js
@@ -9,7 +9,7 @@ import ReportCoverage from './report-coverage';
 import ReportRemap from './report-remap';
 import TextSummary from './report-text-summary';
 import path from 'node:path';
-import fs from 'node:fs';
+import { Response } from '../services/response';
 
 export default class {
   generateReport(res, type, options) {
@@ -88,15 +88,21 @@ export default class {
       }
       default:
         Log.error('Failed to export - this type is not implemented yet');
-        res.status(400);
-        res.json({ type: 'This type [\' + type + \'] is not supported' });
+        Response.send({
+          res,
+          status: 400,
+          json: { type: 'This type [\' + type + \'] is not supported' }
+        });
         return;
       }
     } catch (e) {
       Log.error('ReportService failed while creating report type [', type, ']');
-      console.error(e, e.stack);
-      res.status(400);
-      res.json({ type: 'error','message':'Unexpected error'});
+      Log.error(e);
+      Response.send({
+        res,
+        status: 400,
+        json: { type: 'error','message': `Unexpected error. ${e.message}`}
+      });
     }
   }
   addFileToOptions(options, filename) {
diff --git a/server/report/report-teamcity.js b/server/report/report-teamcity.js
diff --git a/server/report/report-text-summary.js b/server/report/report-text-summary.js
diff --git a/server/services/core.js b/server/services/core.js
diff --git a/server/services/response.js b/server/services/response.js
