Skip to content

Conversation

Muatasim-Aswad
Copy link
Contributor

@Muatasim-Aswad Muatasim-Aswad commented Aug 1, 2025

Note

This PR is an RFC to get an earlier feedback. The reason for this is the size and the scope of the refactoring. It can be closed later.

Intro

  • Major part of the linter errors in the server controllers are caused by inputs, e.g. req.x | res.x, and by try catch blocks. As a solution the main changes are made.
  • Since the main changes will result in a major refactoring, I see that including another refactorings within the same scope is possible with less effort. These additional changes are supposed to increase readability and code consistency.

Main Changes

  • Zod is introduced as a replacement for the existing native validations.
  • All encountered unnecessary try catch blocks were removed, as express current version handles rejected promises automatically.

Additional Changes

  • HttpErrors are introduced for common failure responses, and the global error handler is expanded to handle them.
  • Request handlers are made more consistent in terms of naming and the code order where possible.
  • Library types are used where possible instead of the mirroring types. e.g. RequestHandler.
  • Constructors param shortcuts are used where possible.
  • Helper methods/classes are created where the same functionality is used multiple times.

Notes about Zod Schemas and Their Types

  • The created schemas only mirrors the existing models' types and validation functions, except for obvious characteristics, e.g. age > 0.
  • Currently the schemas coexist with the models' types. This creates multiple sources for the truth about models. While, generally speaking, schemas are a bit less readable than TS declarations, zod is flexible in manipulating the schemas and its inferred types can totally replace the model type declarations. An example is shown in the code.

@Copilot Copilot AI review requested due to automatic review settings August 1, 2025 15:46
@Muatasim-Aswad Muatasim-Aswad requested a review from stasel as a code owner August 1, 2025 15:46
@Muatasim-Aswad Muatasim-Aswad added the BE Backend ticket label Aug 1, 2025
@HackYourFutures HackYourFutures temporarily deployed to dojo-example-gvuwn0pdxserd3l3y August 1, 2025 15:46 Inactive
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors backend error handling and input validation by introducing Zod for schema validation and HttpErrors for standardized error responses. The changes remove try-catch blocks and replace native validation functions with Zod schemas.

  • Introduces HttpError classes for standardized HTTP error responses
  • Replaces native validation functions with Zod schemas for type safety
  • Removes unnecessary try-catch blocks and centralizes error handling
  • Creates reusable helper classes to reduce code duplication

Reviewed Changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 10 comments.

Show a summary per file
File Description
server/src/utils/httpErrors.ts Defines HttpError base class and specific error types with Zod integration
server/src/types.ts Adds global type definitions for Express locals and utility types
server/src/models/ResponseError.ts Removes legacy ResponseError class
server/src/models/Interaction.ts Replaces validation function with Zod schema
server/src/models/EmploymentHistory.ts Replaces validation function with Zod schema
server/src/middlewares/errorHandlerMiddleware.ts Implements centralized error handling middleware
server/src/index.ts Updates application setup to use new error handling
server/src/controllers/Trainee/TraineeHelper.ts Creates helper class for common trainee operations
server/src/controllers/Trainee/InteractionController.ts Refactors to use Zod validation and HttpErrors
server/src/controllers/Trainee/EmploymentHistoryController.ts Refactors to use Zod validation and HttpErrors
server/eslint.config.js Updates ESLint configuration

export type InteractionWithReporter = Interaction & { reporter: DisplayUser };

// For database storage
// ! reporterID is already part of the interface Interaction?
Copy link

Copilot AI Aug 1, 2025

Choose a reason for hiding this comment

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

This comment indicates confusion about the data structure. The comment should be resolved or removed, and if there's genuinely duplicate reporterID fields, the type definition should be clarified.

Suggested change
// ! reporterID is already part of the interface Interaction?

Copilot uses AI. Check for mistakes.

@Muatasim-Aswad Muatasim-Aswad changed the title [example] refactor(BE): fix linter errors in server 2, introduce zod & http errors [example | RFC] refactor(BE): fix linter errors in server 2, introduce zod & http errors Aug 2, 2025
@HackYourFutures HackYourFutures temporarily deployed to dojo-example-dublugyoaynpkwye0 August 2, 2025 08:26 Inactive
@HackYourFutures HackYourFutures temporarily deployed to dojo-example-uy4skol6iev13hor9 August 2, 2025 08:53 Inactive
@HackYourFutures HackYourFutures temporarily deployed to dojo-example-iv3sboh8pohnnwgo4 August 2, 2025 09:55 Inactive
@HackYourFutures HackYourFutures temporarily deployed to dojo-example-4f4ftiyfdq8dmglow August 2, 2025 10:17 Inactive
@HackYourFutures HackYourFutures had a problem deploying to dojo-example-dt25vzks4wiyyqpti August 2, 2025 10:21 Failure
@HackYourFutures HackYourFutures temporarily deployed to dojo-example-dt25vzks4wiyyqpti August 2, 2025 10:22 Inactive
@HackYourFutures HackYourFutures temporarily deployed to dojo-example-5qzrnjqqidilqp54m August 4, 2025 07:00 Inactive
@HackYourFutures HackYourFutures temporarily deployed to dojo-example-m48fcapdhdnhn9xth August 4, 2025 07:05 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

BE Backend ticket

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants