Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CHE-167] BE Refactor Error Handling #150

Merged
merged 55 commits into from
Jun 20, 2024

Conversation

seantokuzo
Copy link
Contributor

Description

**This PR requires a rebuild of the development image. codehammers/ch-dev-dep-v3 does not currently include express-async-errors dependency

What it do:

  • Creates reusable custom error classes for throwing common errors
  • Adds express-async-errors package for easy error catching in async middlewares
  • Refactors the global error handler to handle custom errors
  • Refactors catch all route handler to use custom NotFoundError error class
  • Separates express app config logic from server startup logic
  • Adds environment variable checks before server startup
  • Adds necessary development env vars to docker-compose-dev.yml
  • Makes connectDB more extensible by accepting connection string as arg
  • Refactors auth protect middleware to use new error handling
  • Adds optional user property to express's Request interface to handle type errors in protected routes when trying to access req.user and deletes CustomRequest interface and usage of it
  • Refactors authController at endpoint /api/auth/validate-session to use refactored protect middleware
  • Simplifies usage of protect middleware in all routers
  • Simplifies imports of routers from /routes folder
  • Updates any tests affected by refactored error handling
  • Apparently updates github workflow to use latest dev image which doesn't matter because the workflow builds the image itself

Jira Task

JIRA TICKET

Testing Instructions

To test you will need to build the codehammers/ch-dev-dep-v3 image locally to include the express-async-errors dependency:

  • run npm run docker-remove-all
  • run docker build -f Dockerfile-dev -t codehammers/ch-dev-dep-v3 .
  • run npm run docker-build-check to make sure linting and unit tests pass
  • add - 3000:3000 to the ports array in docker-compose-dev.yml to expose the server for manual testing
  • run npm run docker-dev
  • navigate to a non-existent route, eg. http://localhost:3000/non-existent-route
  • should see the response [{ "message": "Not Found" }] in the browser
  • should see the ❌ NotFoundError in your terminal with the error's stack trace pointing to app.ts

All Team Members

  • I added a descriptive title to this PR.
  • I filled out the Description, Jira Task, and Testing Instructions sections above.
  • I added or updated [Jest unit tests]for any changes to components, server-side controllers, etc.
  • I ran npm run docker-build-check in my local environment to check that this PR passes all linting and unit tests.
  • I did a quick check to make sure my code changes follow the recommended style guide.

seantokuzo and others added 30 commits June 3, 2024 20:00
…to CHE-169/subtask/Add-Express-Async-Errors-Dep
…v-Into-Story-Branch

[Che-167b] Pull Dev into Story Branch
…-Async-Errors-Dep

[CHE-169] Add Express Async Errors Dep
…obal-Error-Handler

[CHE-170] Refactor Global Error Handler
…tch-All-Route-Handler

[CHE-171] Refactor Catch All Route Handler
seantokuzo and others added 21 commits June 12, 2024 12:03
…rver-Startup-and-DB-Connection

[CHE-172] Refactor Server Startup and DB Connection
Pull dev branch changes into CHE-167 Story branch
…th-Middleware

[CHE-173] Refactor Auth Middleware
Copy link
Contributor

@brok3turtl3 brok3turtl3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Glad to see this all come together. 🎉 Nice work!

Im not sure the applications router flow got hooked up. As well there is some odd image behavior at the moment. Possibly fixed in your database seeder ticket?

Copy link
Contributor

@brok3turtl3 brok3turtl3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a comment regarding applications router and images.

Copy link
Contributor

@brok3turtl3 brok3turtl3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @seantokuzo ! I noticed
Screenshot 2024-06-20 at 7 41 42 AM
this code has been removed during your re-org. We will need it reintroduced as it is crucial to the production environment. Our AWS deployment pings this route as part of its monitoring system.

@brok3turtl3 brok3turtl3 merged commit d08fba4 into dev Jun 20, 2024
1 check passed
@brok3turtl3 brok3turtl3 deleted the CHE-167/story/BE-Refactor-Error-Handling branch August 10, 2024 20:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants