Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions server/src/module/attendance/attendance.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,11 @@ export class AttendanceController {
const record = await this.attendanceService.regularize(result.data);
return res.json({ message: "Attendance regularized", record });
} catch (error) {
if (error instanceof Error) {
const msg = error.message;
if (msg.includes("must be after") || msg.includes("must not exceed") || msg.includes("Cannot regularize") || msg.includes("cannot be in the future"))
return res.status(400).json({ message: msg });
}
Comment on lines +94 to +98
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Replace fragile string matching with typed error classes.

The reviewer (Sachinchaurasiya360) correctly identified that substring-based error routing is brittle and hard to maintain. If service error messages change, this controller logic may silently break, returning 500 instead of 400 for validation errors.

Additionally, this approach is inconsistent with other methods in this controller (checkIn and checkOut use exact string matching), making the error-handling strategy harder to understand and maintain.

Introduce a typed validation error class (e.g., AttendanceValidationError) in the service layer. Throw it for all validation failures (future dates, invalid times, etc.), then catch instanceof AttendanceValidationError here and return 400 with the error message. This eliminates string coupling and aligns with the reviewer's suggestion.

🏗️ Proposed refactor to use typed error class

First, define a validation error class in the service file:

// server/src/module/attendance/attendance.service.ts
export class AttendanceValidationError extends Error {
  constructor(message: string) {
    super(message);
    this.name = 'AttendanceValidationError';
  }
}

Then, in the service, replace throw new Error(...) with throw new AttendanceValidationError(...) for all validation failures (future dates, checkIn/checkOut constraints, etc.).

Finally, update the controller to catch the typed error:

     } catch (error) {
-      if (error instanceof Error) {
-        const msg = error.message;
-        if (msg.includes("must be after") || msg.includes("must not exceed") || msg.includes("Cannot regularize") || msg.includes("cannot be in the future"))
-          return res.status(400).json({ message: msg });
-      }
+      if (error instanceof AttendanceValidationError) {
+        return res.status(400).json({ message: error.message });
+      }
       console.error(error);
       return res.status(500).json({ message: "Internal Server Error" });
     }

As per coding guidelines, controller layer must handle request/response, call service, and format errors.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@server/src/module/attendance/attendance.controller.ts` around lines 93 - 97,
Define a typed error class AttendanceValidationError in the attendance service
(e.g., export class AttendanceValidationError extends Error) and update all
service validation code (the places currently throwing new Error for future
dates, invalid times, regularize checks, etc.—e.g., the validation branches used
by checkIn/checkOut and other attendance service methods) to throw
AttendanceValidationError instead of plain Error; then change the controller's
error handling in attendance.controller.ts to check error instanceof
AttendanceValidationError and return res.status(400).json({ message:
error.message }) instead of relying on fragile substring matching.

console.error(error);
return res.status(500).json({ message: "Internal Server Error" });
}
Expand Down
4 changes: 4 additions & 0 deletions server/src/module/attendance/attendance.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,9 +138,13 @@ export class AttendanceService {

const checkIn = new Date(data.checkIn);
const checkOut = new Date(data.checkOut);
const now = new Date();
if (checkIn > now) throw new Error("Check-in time cannot be in the future");
if (checkOut > now) throw new Error("Check-out time cannot be in the future");
if (checkOut <= checkIn) {
throw new Error("Check-out time must be after check-in time");
}

const workHours = (checkOut.getTime() - checkIn.getTime()) / 3600000;

return prisma.attendanceRecord.upsert({
Expand Down
Loading