refactor: deepen architecture — consolidate response, wrap repo boile…#8
Conversation
…rplate, make email send explicit, reduce DI friction
There was a problem hiding this comment.
Pull request overview
This PR refactors the server architecture by consolidating HTTP response utilities into a single module, introducing a shared DB error wrapper to reduce repository boilerplate, and simplifying DI/router wiring by passing the Awilix cradle directly. It also makes email sending in registration explicit and surfaces whether the verification email was sent.
Changes:
- Consolidate response formatting/handlers/schemas into
src/server/infrastructure/http/response/response.tsand update imports accordingly. - Introduce
withDbError()and refactor Postgres repositories to use it instead of per-method try/catch. - Adjust router dependency shapes to
*UseCasenames and update integration tests; extend register flow to returnverificationEmailSent.
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/use-cases/auth/register.test.ts | Asserts the new verificationEmailSent field in register result. |
| tests/integration/auth/auth.routes.test.ts | Updates router dependency keys to *UseCase. |
| tests/integration/admin/admin.routes.test.ts | Updates router dependency keys to *UseCase. |
| src/server/infrastructure/persistence/verification-token.pg.repository.ts | Refactors DB error handling to withDbError(). |
| src/server/infrastructure/persistence/user.pg.repository.ts | Refactors DB error handling to withDbError() and maps unique violations. |
| src/server/infrastructure/persistence/token.pg.repository.ts | Refactors DB error handling to withDbError(). |
| src/server/infrastructure/persistence/role.pg.repository.ts | Refactors DB error handling to withDbError(). |
| src/server/infrastructure/persistence/password-reset-token.pg.repository.ts | Refactors DB error handling to withDbError(). |
| src/server/infrastructure/http/shared/create-router.ts | Updates formatError import to the consolidated response module. |
| src/server/infrastructure/http/response/response.types.ts | Removes now-consolidated response types file. |
| src/server/infrastructure/http/response/response.schemas.ts | Removes now-consolidated response schemas file. |
| src/server/infrastructure/http/response/response.handler.ts | Removes now-consolidated success handler file. |
| src/server/infrastructure/http/response/response.formatter.ts | Removes now-consolidated formatter file. |
| src/server/infrastructure/http/response/response.ts | New single module for response types, formatters, handlers, and OpenAPI schemas. |
| src/server/infrastructure/http/middleware/rate-limiter.ts | Updates formatError import to the consolidated response module. |
| src/server/infrastructure/http/middleware/error-handler.ts | Updates formatError import to the consolidated response module. |
| src/server/infrastructure/http/auth/index.ts | Simplifies DI wiring by passing container.cradle directly to router/middleware. |
| src/server/infrastructure/http/auth/auth.schemas.ts | Adds registerResponseSchema including verificationEmailSent. |
| src/server/infrastructure/http/auth/auth.routes.ts | Updates deps naming to *UseCase, uses consolidated response module, and updates register response schema. |
| src/server/infrastructure/http/admin/index.ts | Simplifies DI wiring by passing container.cradle directly to router/middleware. |
| src/server/infrastructure/http/admin/admin.routes.ts | Updates deps naming to *UseCase and uses consolidated response module. |
| src/server/infrastructure/db/with-db-error.ts | Introduces shared DB error wrapper and DB error type guard. |
| src/server/index.ts | Removes no-longer-needed destructured DI items. |
| src/server/core/use-cases/auth/register.ts | Awaits verification email send and returns verificationEmailSent in the result. |
Comments suppressed due to low confidence (8)
src/server/infrastructure/persistence/token.pg.repository.ts:37
- Same return-type issue as above:
delete()promisesvoidbut the callback passed towithDbErrorreturns the Drizzle delete result. Make the callbackasyncandawaitthe query (returningvoid) or otherwise discard the result.
return withDbError("delete refresh token", () =>
this.db.delete(refreshTokens).where(eq(refreshTokens.token, token)),
);
src/server/infrastructure/persistence/token.pg.repository.ts:43
- Same return-type issue:
deleteAllForUser()promisesvoidbut thewithDbErrorcallback returns the Drizzle delete result. Ensure the callback returnsPromise<void>by awaiting and discarding the query result.
return withDbError("delete all refresh tokens for user", () =>
this.db.delete(refreshTokens).where(eq(refreshTokens.userId, userId)),
);
src/server/infrastructure/persistence/password-reset-token.pg.repository.ts:37
- Return type mismatch risk:
delete()returnsPromise<void>but thewithDbErrorcallback returns the Drizzle delete result. Make the callbackasyncandawaitthe query so it resolves tovoid.
return withDbError("delete password reset token", () =>
this.db.delete(passwordResetTokens).where(eq(passwordResetTokens.token, token)),
);
src/server/infrastructure/persistence/password-reset-token.pg.repository.ts:43
- Return type mismatch risk:
deleteAllForUser()returnsPromise<void>but thewithDbErrorcallback returns the Drizzle delete result. Ensure the callback resolves tovoidby awaiting and discarding the result.
return withDbError("delete all password reset tokens for user", () =>
this.db.delete(passwordResetTokens).where(eq(passwordResetTokens.userId, userId)),
);
src/server/infrastructure/persistence/user.pg.repository.ts:62
- Same
Promise<void>vs query-result return mismatch:updatePassword()promisesvoidbut returns whatever the Drizzle update resolves to viawithDbError. Await/discard the query result inside the callback so the method resolves tovoid.
return withDbError("update password", () =>
this.db.update(users).set({ passwordHash }).where(eq(users.id, userId)),
);
src/server/infrastructure/persistence/user.pg.repository.ts:86
- Same issue:
delete()promisesvoidbut the callback passed towithDbErrorreturns the Drizzle delete result. Make the callbackasyncandawaitthe delete (or discard the result) so the method resolves tovoid.
return withDbError("delete user", () =>
this.db.delete(users).where(eq(users.id, userId)),
);
src/server/infrastructure/persistence/role.pg.repository.ts:74
- Return type mismatch risk:
removeRoleFromUser()returnsPromise<void>but the callback passed towithDbErrorreturns the delete query result. Use anasynccallback that awaits the delete and returnsvoid(or discard the result).
return withDbError("remove role from user", () =>
this.db.delete(userRoles).where(and(eq(userRoles.userId, userId), eq(userRoles.roleId, roleId))),
);
src/server/infrastructure/persistence/verification-token.pg.repository.ts:45
- Same
Promise<void>vs query-result mismatch:deleteAllForUser()promisesvoidbut returns the delete query result viawithDbError. Await/discard the result inside anasynccallback so the method resolves tovoid.
return withDbError("delete all verification tokens for user", () =>
this.db.delete(verificationTokens).where(eq(verificationTokens.userId, userId)),
);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import { AppError, ErrorCode } from "../../core/errors"; | ||
|
|
||
| export function isDbError(err: unknown): err is { code: string; message: string } { | ||
| return typeof err === "object" && err !== null && "code" in err; |
| import { logger } from "../logger"; | ||
| import { eq, sql } from "drizzle-orm"; | ||
| import { DB } from "../db"; | ||
| import { withDbError, isDbError } from "../db/with-db-error"; |
| return withDbError("save refresh token", () => | ||
| this.db.insert(refreshTokens).values({ | ||
| id: crypto.randomUUID(), | ||
| userId, | ||
| token, |
| return withDbError("save password reset token", () => | ||
| this.db.insert(passwordResetTokens).values({ | ||
| id: crypto.randomUUID(), | ||
| userId, | ||
| token, | ||
| expiresAt, |
| return withDbError("verify user", () => | ||
| this.db.update(users).set({ isVerified: true }).where(eq(users.id, userId)), | ||
| ); |
| return withDbError("assign role to user", () => | ||
| this.db.insert(userRoles).values({ userId, roleId }).onConflictDoNothing(), | ||
| ); |
| return withDbError("delete verification token", () => | ||
| this.db.delete(verificationTokens).where(eq(verificationTokens.token, token)), | ||
| ); |
…rplate, make email send explicit, reduce DI friction