fix: reuse middleware reporting layer (#4114)

* chore: clean up

* chore: clean up

.github/workflows/ci-lint.yml

@@ -0,0 +1,22 @@
+name: CI Lint
+
+on: [push, pull_request]
+
+jobs:
+  ci-lint:
+    name: Node Lint
+
+    runs-on: ubuntu-latest
+
+    steps:
+    - uses: actions/checkout@c85c95e3d7251135ab7dc9ce3241c5835cc595a9 # v3.5.3
+    - name: Use Node 
+      uses: actions/setup-node@7c12f8017d5436eb855f1ed4399f037a36fbd9e8 # v2.5.2
+      with:
+        node-version-file: '.nvmrc'
+    - name: Install
+      run: yarn install --immutable
+    - name: Format
+      run: yarn format:check
+    - name: Lint
+      run: yarn lint

.github/workflows/ci.yml

@@ -21,10 +21,6 @@ jobs:
         node-version: ${{ matrix.node_version }}
     - name: Install
       run: yarn install --immutable
-    - name: Format
-      run: yarn format:check
-    - name: Lint
-      run: yarn lint        
     - name: Build
       run: yarn build
     - name: Types

.github/workflows/smok-test-module-v12.yml

@@ -19,7 +19,7 @@ jobs:
       - name: login    
         run: npx npm-cli-login -u test -p 1234 -e test@domain.test -r http://localhost:4873                 
       - name: Build
-        run: yarn build
+        run: yarn && yarn build
       - name: Types
         run: yarn code:types           
       - name: Bump up package

.github/workflows/smok-test-module.yml

@@ -19,7 +19,7 @@ jobs:
       - name: login    
         run: npx npm-cli-login -u test -p 1234 -e test@domain.test -r http://localhost:4873                 
       - name: Build
-        run: yarn build
+        run: yarn && yarn build
       - name: Types
         run: yarn code:types           
       - name: Bump up package

package.json

@@ -19,20 +19,20 @@
     "url": "https://opencollective.com/verdaccio"
   },
   "dependencies": {
-    "@verdaccio/config": "7.0.0-next.3",
-    "@verdaccio/core": "7.0.0-next.3",
+    "@verdaccio/config": "7.0.0-next.4",
+    "@verdaccio/core": "7.0.0-next.4",
     "@verdaccio/local-storage": "10.3.3",
-    "@verdaccio/logger-7": "7.0.0-next.3",
-    "@verdaccio/middleware": "7.0.0-next.3",
+    "@verdaccio/logger-7": "7.0.0-next.4",
+    "@verdaccio/middleware": "7.0.0-next.4",
     "@verdaccio/search": "7.0.0-next.2",
-    "@verdaccio/signature": "7.0.0-next.1",
+    "@verdaccio/signature": "7.0.0-next.2",
     "@verdaccio/streams": "10.2.1",
-    "@verdaccio/tarball": "12.0.0-next.3",
-    "@verdaccio/ui-theme": "7.0.0-next.3",
-    "@verdaccio/url": "12.0.0-next.3",
-    "@verdaccio/utils": "7.0.0-next.3",
+    "@verdaccio/tarball": "12.0.0-next.4",
+    "@verdaccio/ui-theme": "7.0.0-next.4",
+    "@verdaccio/url": "12.0.0-next.4",
+    "@verdaccio/utils": "7.0.0-next.4",
     "JSONStream": "1.3.5",
-    "async": "3.2.4",
+    "async": "3.2.5",
     "clipanion": "3.2.1",
     "compression": "1.7.4",
     "cookies": "0.8.0",
@@ -55,8 +55,8 @@
     "request": "2.88.2",
     "semver": "7.5.4",
     "validator": "13.11.0",
-    "verdaccio-audit": "12.0.0-next.3",
-    "verdaccio-htpasswd": "12.0.0-next.3"
+    "verdaccio-audit": "12.0.0-next.4",
+    "verdaccio-htpasswd": "12.0.0-next.4"
   },
   "devDependencies": {
     "@babel/cli": "7.23.0",
@@ -126,7 +126,7 @@
     "pinst": "2.1.6",
     "prettier": "3.0.3",
     "rimraf": "3.0.2",
-    "selfsigned": "2.1.1",
+    "selfsigned": "2.4.1",
     "standard-version": "9.5.0",
     "supertest": "6.3.3",
     "ts-node": "10.9.1",
@@ -160,7 +160,7 @@
     "test:all": "yarn run test && yarn run test:functional",
     "pre:ci": "yarn run lint",
     "lint:ts": "yarn run type-check",
-    "lint": "eslint \"**/*.{js,jsx,ts}\" --max-warnings 145 -c ./eslintrc.js",
+    "lint": "eslint \"**/*.{js,jsx,ts}\" --max-warnings 145 -c ./eslintrc.cjs",
     "lint:lockfile": "lockfile-lint --path yarn.lock --type yarn --validate-https --allowed-hosts verdaccio npm yarn",
     "start": "yarn babel-node --extensions \".ts,.tsx\" src/lib/cli --inspect",
     "start:brk": "yarn babel-node --extensions \".ts,.tsx\" src/lib/cli --inspect-brk",
@@ -178,7 +178,8 @@
   "preferGlobal": true,
   "license": "MIT",
   "resolutions": {
-    "@types/serve-static": "1.13.10"
+    "@types/serve-static": "1.13.10",
+    "@types/express-serve-static-core": "4.17.41"
   },
   "collective": {
     "type": "opencollective",

src/api/index.ts

@@ -5,7 +5,7 @@ import _ from 'lodash';
 
 import { getUserAgent } from '@verdaccio/config';
 import { pluginUtils } from '@verdaccio/core';
-import { final } from '@verdaccio/middleware';
+import { errorReportingMiddleware, final, handleError } from '@verdaccio/middleware';
 import { log } from '@verdaccio/middleware';
 import { SearchMemoryIndexer } from '@verdaccio/search';
 import { Config as IConfig } from '@verdaccio/types';
@@ -20,7 +20,7 @@ import { ErrorCode } from '../lib/utils';
 import { $NextFunctionVer, $RequestExtend, $ResponseExtend } from '../types';
 import hookDebug from './debug';
 import apiEndpoint from './endpoint';
-import { errorReportingMiddleware, handleError, serveFavicon } from './middleware';
+import { serveFavicon } from './middleware';
 import webMiddleware from './web';
 
 const { version } = require('../../package.json');
@@ -60,7 +60,7 @@ const defineAPI = async function (config: IConfig, storage: Storage): Promise<ex
 
   // // Router setup
   app.use(log(logger));
-  app.use(errorReportingMiddleware);
+  app.use(errorReportingMiddleware(logger));
   if (config.user_agent) {
     app.use(function (_req: $RequestExtend, res: $ResponseExtend, next: $NextFunctionVer): void {
       res.setHeader('X-Powered-By', getUserAgent(config.user_agent));
@@ -118,7 +118,7 @@ const defineAPI = async function (config: IConfig, storage: Storage): Promise<ex
   app.get('/*', function (_, __, next: $NextFunctionVer) {
     next(ErrorCode.getNotFound(API_ERROR.FILE_NOT_FOUND));
   });
-  app.use(handleError);
+  app.use(handleError(logger));
   app.use(final);
 
   return app;

src/api/middleware.ts

@@ -1,17 +1,15 @@
 import buildDebug from 'debug';
 import fs from 'fs';
-import { HttpError } from 'http-errors';
 import _ from 'lodash';
 import path from 'path';
 import validator from 'validator';
 
-import { Config, Package } from '@verdaccio/types';
+import { Config } from '@verdaccio/types';
 
-import { API_ERROR, HTTP_STATUS } from '../lib/constants';
-import { logger } from '../lib/logger';
+import { HTTP_STATUS } from '../lib/constants';
 import { $NextFunctionVer, $RequestExtend, $ResponseExtend } from '../types';
 
-const debug = buildDebug('verdaccio');
+const debug = buildDebug('verdaccio:middleware:favicon');
 
 export function serveFavicon(config: Config) {
   return function (req: $RequestExtend, res: $ResponseExtend, next: $NextFunctionVer) {
@@ -57,67 +55,3 @@ export function serveFavicon(config: Config) {
     }
   };
 }
-
-export function handleError(
-  err: HttpError,
-  req: $RequestExtend,
-  res: $ResponseExtend,
-  next: $NextFunctionVer
-) {
-  debug('error handler init');
-  if (_.isError(err)) {
-    debug('is native error');
-    if (err.code === 'ECONNABORT' && res.statusCode === HTTP_STATUS.NOT_MODIFIED) {
-      return next();
-    }
-    if (_.isFunction(res.locals.report_error) === false) {
-      debug('is locals error report ref');
-      // in case of very early error this middleware may not be loaded before error is generated
-      // fixing that
-      errorReportingMiddleware(req, res, _.noop);
-    }
-    debug('set locals error report ref');
-    res.locals.report_error(err);
-  } else {
-    // Fall to Middleware.final
-    debug('no error to report, jump next layer');
-    return next(err);
-  }
-}
-
-export interface MiddlewareError {
-  error: string;
-}
-
-export type FinalBody = Package | MiddlewareError | string;
-
-// Middleware
-export function errorReportingMiddleware(
-  req: $RequestExtend,
-  res: $ResponseExtend,
-  next: $NextFunctionVer
-): void {
-  res.locals.report_error =
-    res.locals.report_error ||
-    function (err: any): void {
-      if (err.status && err.status >= HTTP_STATUS.BAD_REQUEST && err.status < 600) {
-        if (!res.headersSent) {
-          res.status(err.status);
-          next({ error: err.message || API_ERROR.UNKNOWN_ERROR });
-        }
-      } else {
-        logger.error({ err: err }, 'unexpected error: @{!err.message}\n@{err.stack}');
-        if (!res.status || !res.send) {
-          logger.error('this is an error in express.js, please report this');
-          res.destroy();
-        } else if (!res.headersSent) {
-          res.status(HTTP_STATUS.INTERNAL_ERROR);
-          next({ error: API_ERROR.INTERNAL_SERVER_ERROR });
-        } else {
-          // socket should be already closed
-        }
-      }
-    };
-
-  next();
-}

test/helpers/initializeServer.ts

@@ -4,12 +4,12 @@ import os from 'os';
 import path from 'path';
 
 import { errorUtils } from '@verdaccio/core';
-import { final } from '@verdaccio/middleware';
+import { errorReportingMiddleware, final, handleError } from '@verdaccio/middleware';
 import { generateRandomHexString } from '@verdaccio/utils';
 
-import { errorReportingMiddleware, handleError } from '../../src/api/middleware';
 import Auth from '../../src/lib/auth';
 import Config from '../../src/lib/config';
+import { logger } from '../../src/lib/logger';
 
 const debug = buildDebug('verdaccio:tools:helpers:server');
 
@@ -34,7 +34,7 @@ export async function initializeServer(
   // TODO: this might not be need it, used in apiEndpoints
   app.use(express.json({ strict: false, limit: '100mb' }));
   // @ts-ignore
-  app.use(errorReportingMiddleware);
+  app.use(errorReportingMiddleware(logger));
   for (let route of routesMiddleware) {
     if (route.async) {
       const middleware = await route.routes(config, auth, storage);
@@ -50,7 +50,7 @@ export async function initializeServer(
   });
 
   // @ts-ignore
-  app.use(handleError);
+  app.use(handleError(logger));
   // @ts-ignore
   app.use(final);