Refactoring backend codebase to use central error handling and logging #152
Conversation
Added:
- Added Midleware to capture and handle all errors
- will catch error from `throw ...` and `next(...)`
- Added RequestError base error class to build consistent error objects
- Added common errors like `UnauthorizedRequestError`, `BadRequestError`
- Added consistent Logging solution using `winston`
- Supports Loki Transporter using `winston-loki`
- Outputing Legal disclaimer to console when `TELEMETRY_ENABLED=true`
Changed:
- Changed console.log to getLogger() favor of using consistent logging
Refactored middlewares to use RequestError rather than using
`try {...}catch (err){...}`. With this change it's possible to manage
all error details within one place.
Added:
- Added Sentry.captureException to Error Handler
Error handler is a middleware that captures errors from throw and next(...), so in order to be able to handle it needs to come after all of the routing and other middlewares. Previously it was before routuing logic and wasn't working, now it works as intended. Also added check for LogLevel to Sentry for not sending false-positive errors like `BadRequestError`, `UnauthorizedRequestError` and etc.
|
Just left a few minor comments above to sort out. Otherwise, great work with improving the logging for Infisical! Looking forward to merging this 😁 |
Added: - New error types such as `IntegrationNotFoundError`, `WorkspaceNotFoundError`, `AccountNotFoundError' and more. Refactored: - Refactored most of the middlewares and very little number of helper functions to use RequestError - Deleted unused imports Changed: - Some of the error types in middlewares changed to more related error types. - Environment variable of 'VERBOSE_ERROR_OUTPUT' changed to more reliable validation method in `config/index.ts` as per @dangtony98 requested.
|
At this point, I think we can start merging these changes to take small steps forward. We still have a lot of work to do to completely refactor all legacy errors to the new RequestError. If I were to do it by myself, it would take much longer than necessary. That's why I think we can merge this pull request and encourage existing and new contributors to use getLogger() for logging, Even though I tested in Web GUI and HTTP Request, please give it a try and give me some feedback. I will make corrections as soon as possible of any given. @dangtony98 @maidul98 |
In order to catch Promise rejections inside and outside of middlewares as well as in route logic we need to patch `Router#handle` and add .catch() to the function. With that addition it is possible to catch rejections and handle inside `requestErrorHandler` middleware.
There was a problem hiding this comment.
Looks good to me!
There're definitely a lot of parts that need to be re-classified/renamed (e.g. there may need to be a BotNotFound Error) and ironed out but I think this is a great start for us to start re-working the backend and getting more contributors involved in that process :)
Great stuff.
|
| GitGuardian id | Secret | Commit | Filename | |
|---|---|---|---|---|
| - | Generic High Entropy Secret | a12ae35 | .env.example | View secret |
| - | Generic High Entropy Secret | a12ae35 | .env.example | View secret |
| - | Generic High Entropy Secret | a12ae35 | .env.example | View secret |
| - | Generic High Entropy Secret | a12ae35 | .env.example | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secrets safely. Learn here the best practices.
- Revoke and rotate these secrets.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
Our GitHub checks need improvements? Share your feedbacks!
Added:
[21.12.2022]
throw ...andnext(...)VERBOSE_ERROR_OUTPUTenvironment variable to controlstacktraceof error mesages, with this variable it's easier to locate error in developmen environment.UnauthorizedRequestError,BadRequestErrorwinstonwinston-lokiTELEMETRY_ENABLED=true[22.12.2022]
[24.12.2022]
IntegrationNotFoundError,WorkspaceNotFoundError, `AccountNotFoundError' and more.Changed:
[21.12.2022]
[24.12.2022]
config/index.tsas per @dangtony98 requested.Screenshosts:
New error messages for unrouted requests (stacktrace can be omitted using
VERBOSE_ERROR_OUTPUT=false)