Add logic to catch clause #1183
Conversation
src/api/web/endpoint/package.js
Outdated
| @@ -45,6 +45,7 @@ function addPackageWebApi(route: Router, storage: IStorageHandler, auth: IAuth) | |||
| permissions.push(pkg); | |||
| } | |||
| } catch (err) { | |||
| console.error(err) | |||
There was a problem hiding this comment.
Problem is the eslint don't allow console.error on code. Here we should use the logger object.
/home/circleci/verdaccio/src/api/web/endpoint/package.js
48:13 error Unexpected console statement no-console
48:31 error Missing semicolon semi
Instead I would
import logger from '../../../lib/logger';
// below
logger.logger.error({name: pkg.name, error }, 'permission process for @{name} has failed: @{error}');
There was a problem hiding this comment.
Ok, will change it in about 1 - 2 hours.
There was a problem hiding this comment.
Do we run the eslint checks or how can I make sure to catch such things like you mention?
There was a problem hiding this comment.
If you remove the console.error(err) all should be fine. But you can run locally yarn lint to check all lint pass. https://github.com/verdaccio/verdaccio/wiki/Build-Source-Code#scripts-1
There was a problem hiding this comment.
Ah, CI is red because of this which is good =). I guess I either oversaw it or it took some time after my changes.
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Type:
bug
The following has been addressed in the PR:
Description:
A
catchclause that only rethrows the caught exception has the same effect as omitting the catch altogether and letting it bubble up automatically, but with more code and the additional detriment of leaving maintainers scratching their heads.Such clauses should either be eliminated or populated with the appropriate logic.