fix: added supabase docs #172
Conversation
- Add details parameter to AppException base class - Update all domain exceptions to support optional details payload - Create exceptions index file for clean imports - Standardize exception structure across the application
- Create prisma-error.mapper.ts with comprehensive error code mapping - Map P2002 (unique constraint) to ConflictException - Map P2025 (record not found) to ValidationException - Map P2003 (foreign key failure) to ValidationException - Map P2014 (invalid ID) to ValidationException - Provide prismaGuard() wrapper for async operations - Provide prismaGuardSync() wrapper for sync operations - Ensure no raw Prisma errors leak from repositories
- Refactor errorHandler to be single source of truth
- Implement consistent JSON response: {statusCode, errorCode, message, details?, traceId?}
- Add Winston logging integration with traceId support
- Remove duplicate error handlers from middlewares directory
- Ensure all errors follow standardized format across application
- Replace custom JSON error responses with ValidationException throws - Remove ValidationErrorResponse interface (no longer needed) - Ensure all validation failures surface as domain exceptions - Maintain consistent error handling flow through global handler - Remove ad-hoc error crafting from validation middleware
- Wrap all Prisma operations with prismaGuard() wrapper - Ensure no raw Prisma errors can leak from repository layer - Maintain consistent error handling through domain exceptions - Apply error mapping to all CRUD operations in PrismaUserRepository
… cases - Replace all 'throw new Error(...)' with appropriate domain exceptions - Use ValidationException for user not found scenarios - Use AuthenticationException for token/verification failures - Use ConflictException for already verified users - Ensure consistent error handling across all auth use cases - Remove ad-hoc error handling patterns
- Remove all res.status().json() error crafting from controller - Replace custom error responses with domain exception throws - Update DTO validation to throw ValidationException with details - Ensure controller relies on global error handler for all error responses - Maintain consistent error handling flow through exception bubbling
- Update main index.ts to use shared error handler - Remove dependency on middlewares errorHandler - Ensure consistent error handling across entire application - Complete the error handling refactor pipeline
- Add 'Error Handling & Exceptions' section to README - Document all 5 allowed domain exceptions with status codes - Provide consistent JSON error response format specification - Include implementation rules and best practices - Add code examples showing correct vs incorrect error handling - Ensure developers understand the new error handling contract
- Remove all test files created during refactor - Clean up empty __tests__ directories - Maintain clean project structure without test artifacts - Focus on production-ready error handling implementation
WalkthroughRefactors auth controllers and use-cases to throw domain exceptions, introduces unified error types and Prisma-to-domain error mapping, adds a structured global error handler and updates validation middleware to throw ValidationException; also adds README sections (Error Handling & Supabase) and removes a validation middleware test file. Changes
Sequence Diagram(s)sequenceDiagram
participant C as Controller
participant U as UseCase
participant DB as Repository/Prisma
participant G as Global ErrorHandler
participant CL as Client
CL->>C: HTTP request
C->>U: call execute(...)
U->>DB: repo call (wrapped with prismaGuard)
DB-->>U: success / PrismaClientKnownRequestError
alt Prisma error
U->>U: prismaGuard -> mapPrismaError
U-->>G: throw DomainException (e.g., ConflictException)
else Domain validation error
U-->>G: throw ValidationException
else success
U-->>C: result
C-->>CL: 200 OK
end
Note right of G: Global ErrorHandler logs structured error (incl. traceId) and responds with unified JSON
sequenceDiagram
participant M as Validation Middleware
participant C as Controller
participant G as Global ErrorHandler
participant CL as Client
CL->>M: request with payload
M->>M: validate DTO
alt invalid
M-->>G: throw ValidationException (details: formatted errors)
G-->>CL: 400 JSON { statusCode, errorCode, message, details, traceId? }
else valid
M->>C: next()
C->>CL: business response
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Assessment against linked issues
Out-of-scope changes
Possibly related PRs
Poem
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
README.md (2)
239-248: Clarify env var scope and add server-only key + consistency with env filesDocument which keys are safe for clients vs server, and reference .env.example/.env.test to keep docs aligned. Add optional service role key for backend-only operations.
-## 🔌 Supabase Integration - -This project uses Supabase for external data access and future integrations. - -Update your `.env` file with: +## 🔌 Supabase Integration + +Supabase backs selected services (storage/auth/data). Configure the following environment variables: +• SUPABASE_URL — project URL (public) +• SUPABASE_ANON_KEY — anon key (public, client-safe) +• SUPABASE_SERVICE_ROLE_KEY — service role key (server-only, never expose to clients) [optional] + +Add these to your environment files (.env, .env.example, and .env.test as needed): ```bash SUPABASE_URL=... SUPABASE_ANON_KEY=... +# Server-only (optional; required for privileged operations) +SUPABASE_SERVICE_ROLE_KEY=...--- `250-250`: **Minor Markdown nit: consider removing this extra separator** You already use section separators consistently; this one may be redundant directly before “Module Overview.” ```diff ----
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
README.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[grammar] ~239-~239: There might be a mistake here.
Context: ...eed ``` --- ## 🔌 Supabase Integration This project uses Supabase for external ...
(QB_NEW_EN)
| ## 🔌 Supabase Integration | ||
|
|
||
| This project uses Supabase for external data access and future integrations. | ||
|
|
||
| Update your `.env` file with: | ||
|
|
||
| ```bash | ||
| SUPABASE_URL=... | ||
| SUPABASE_ANON_KEY=... | ||
| ``` |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify variables are used in code and present in sample envs
Ensure the code expects these exact names and that .env.example/.env.test include them.
🏁 Script executed:
#!/bin/bash
# Find references to Supabase env vars across the repo
rg -nP '\bSUPABASE_(URL|ANON_KEY|SERVICE_ROLE_KEY)\b' -C2
# Confirm they exist in sample env files
fd -H '^\.env(\.example|\.test)?$' -a | xargs -I{} sh -c 'echo "---- {}"; cat "{}" | rg -n "SUPABASE_" || true'
# Check docker compose files for propagation
fd -a 'docker-compose*.yml' | xargs -I{} sh -c 'echo "---- {}"; rg -n "SUPABASE_" "{}" || true'Length of output: 1122
Add missing Supabase env vars to sample env files
.env.testis currently missing SUPABASE_URL and SUPABASE_ANON_KEY which are required bysrc/config/supabase.ts; add them with appropriate test placeholders.- Confirm
docker-compose.ymlpropagates these vars to any Supabase‐dependent services (viaenv_fileorenvironment).
🧰 Tools
🪛 LanguageTool
[grammar] ~239-~239: There might be a mistake here.
Context: ...eed ``` --- ## 🔌 Supabase Integration This project uses Supabase for external ...
(QB_NEW_EN)
There was a problem hiding this comment.
Actionable comments posted: 23
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/shared/exceptions/AppException.ts (1)
15-22: Whitelist safe properties in errorHandlerdetails
Insrc/middlewares/errorHandler.ts, the handler already appendstraceId, but it spreadserr.detailsverbatim (...(err.details && { details: err.details })), which can leak raw error metadata (e.g. Prisma internals). Filter or whitelist specificdetailsfields before serialization to avoid exposing sensitive information.src/shared/infrastructure/prisma-error.mapper.ts (1)
75-75: Remove stray token at EOF.Line 75 contains a bare
75, which will break compilation. Delete it.-75
♻️ Duplicate comments (1)
README.md (1)
289-299: Supabase envs: ensure sample envs + CI propagate them.Check .env.example/.env.test and docker-compose for SUPABASE_URL/ANON_KEY (and SERVICE_ROLE_KEY if used server-side).
#!/bin/bash # Where are Supabase vars referenced? rg -nP '\bSUPABASE_(URL|ANON_KEY|SERVICE_ROLE_KEY)\b' -C2 # Do sample env files define them? fd -H '^\.env(\.example|\.test)?$' -a | xargs -I{} sh -c 'echo "---- {}"; rg -n "SUPABASE_" "{}" || true' # Are they wired into docker compose? fd -a 'docker-compose*.yml' | xargs -I{} sh -c 'echo "---- {}"; rg -n "SUPABASE_" "{}" || true'
🧹 Nitpick comments (15)
src/shared/exceptions/AppException.ts (2)
15-22: Don’t drop falsy-but-valid details in toJSON.The truthy check skips details like 0, "", or false. Prefer an undefined check.
errorCode: this.errorCode, - ...(this.details && { details: this.details }), + ...(this.details !== undefined ? { details: this.details } : {}), };
6-13: Harden Error subclassing across TS targets.Set the prototype and guard captureStackTrace for consistent instanceof behavior in Node/Jest.
constructor(message: string, statusCode: number, errorCode: string, details?: unknown) { super(message); + // Maintain correct prototype chain across TS transpilation + Object.setPrototypeOf(this, new.target.prototype); this.statusCode = statusCode; this.errorCode = errorCode; this.details = details; this.name = this.constructor.name; - Error.captureStackTrace(this, this.constructor); + if (typeof (Error as any).captureStackTrace === "function") { + Error.captureStackTrace(this, new.target); + } }readme.md (1)
86-97: Minor editorial: tighten phrasing.Consider hyphenation and brevity: “standardized, domain-specific error-handling system.” Purely stylistic.
src/modules/auth/use-cases/verify-email.usecase.ts (1)
16-19: Attach machine-readable details to aid observability.Include a lightweight reason code; it will flow through details in AppException.
- throw new AuthenticationException("Invalid or expired verification token"); + throw new AuthenticationException("Invalid or expired verification token", { reason: "TOKEN_INVALID" }); @@ - throw new AuthenticationException("Verification token has expired"); + throw new AuthenticationException("Verification token has expired", { reason: "TOKEN_EXPIRED" });Also applies to: 36-37
src/modules/auth/use-cases/send-verification-email.usecase.ts (2)
12-15: Avoid user enumeration on public endpoints.If this route is callable unauthenticated, throwing “User not found” reveals account existence. Consider always returning 202/OK with a generic message and doing nothing when the user is absent, or require auth and keep the exception.
Would you like a feature-flagged variant (public = neutral 202, internal = strict exception)?
2-2: Align exception import strategy across the codebase.This file imports from
../../../../shared/exceptionswhile others (e.g., error handler) use sub-paths. Prefer one barrel import style repo-wide for consistency.src/modules/auth/use-cases/verify-wallet.usecase.ts (1)
2-2: Update imports to include InternalServerException.-import { ValidationException } from "../../../../shared/exceptions"; +import { ValidationException, InternalServerException } from "../../../../shared/exceptions";src/shared/middleware/errorHandler.ts (2)
22-24: Propagate traceId via response header for client correlation.- // Extract traceId from request if available - const traceId = (req as any).traceId; + // Extract traceId from request if available + const traceId = (req as any).traceId; + if (traceId) { + res.setHeader("X-Trace-Id", traceId); + }
4-7: Consider consolidating on a single logger implementation.You have both a custom Logger and a Winston-based logger. Standardizing on one reduces operational complexity.
src/shared/middleware/validation.middleware.ts (1)
13-13: Optional: use plainToInstance for class-transformer ≥ 0.5.
plainToClassis deprecated in recent versions. Switch toplainToInstancewhen available.- const dto = plainToClass(dtoClass, req.body); + const dto = plainToInstance(dtoClass, req.body);(Apply similarly for query/params.)
Also applies to: 48-48, 83-83
src/modules/auth/use-cases/resend-email-verification.usecase.ts (1)
16-18: Add rate limiting and resend TTL.Prevent abuse by enforcing per-user and per-IP throttling and minimum resend interval (e.g., 10–15 min).
src/modules/auth/use-cases/resend-verification-email.usecase.ts (1)
13-15: Track TODO with an issue.Open a task to implement token generation, persistence, and email dispatch with proper idempotency.
src/shared/infrastructure/prisma-error.mapper.ts (1)
48-57: Consider wrapping non-Prisma errors to preserve “domain exceptions only” contract.Today non-Prisma errors are rethrown as-is. If any raw
Errorescapes, it violates the unified error-handling objective. Optionally wrap unknown errors withInternalServerExceptionwhile letting known domain exceptions pass through.Would you like me to update
prismaGuard/prismaGuardSyncto rethrow domain exceptions untouched and wrap others? This requires importingAppExceptionto avoid double-wrapping.Also applies to: 63-73
src/modules/auth/presentation/controllers/Auth.controller.ts (2)
25-25: Remove unused import or use it.
AuthenticationExceptionis imported but unused. Drop it unless you plan to use it.-import { ValidationException, AuthenticationException, InternalServerException } from "../../../../shared/exceptions"; +import { ValidationException, InternalServerException } from "../../../../shared/exceptions";
107-110: Redundant type assertion.After
typeof req.query.token === "string", the value is alreadystring. Drop theas string.- const tokenQuery = - typeof req.query.token === "string" - ? (req.query.token as string) - : undefined; + const tokenQuery = + typeof req.query.token === "string" ? req.query.token : undefined;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
README.md(2 hunks)readme.md(1 hunks)src/modules/auth/presentation/controllers/Auth.controller.ts(3 hunks)src/modules/auth/use-cases/email-verification.usecase.ts(1 hunks)src/modules/auth/use-cases/resend-email-verification.usecase.ts(1 hunks)src/modules/auth/use-cases/resend-verification-email.usecase.ts(1 hunks)src/modules/auth/use-cases/send-verification-email.usecase.ts(1 hunks)src/modules/auth/use-cases/verify-email.usecase.ts(4 hunks)src/modules/auth/use-cases/verify-wallet.usecase.ts(1 hunks)src/modules/auth/use-cases/wallet-format-validation.usecase.ts(1 hunks)src/shared/exceptions/AppException.ts(2 hunks)src/shared/exceptions/DomainExceptions.ts(1 hunks)src/shared/exceptions/index.ts(1 hunks)src/shared/infrastructure/prisma-error.mapper.ts(1 hunks)src/shared/middleware/__tests__/validation.middleware.test.ts(0 hunks)src/shared/middleware/errorHandler.ts(3 hunks)src/shared/middleware/validation.middleware.ts(4 hunks)
💤 Files with no reviewable changes (1)
- src/shared/middleware/tests/validation.middleware.test.ts
🧰 Additional context used
🧬 Code graph analysis (9)
src/shared/infrastructure/prisma-error.mapper.ts (2)
src/utils/logger.ts (1)
error(29-31)src/shared/exceptions/index.ts (3)
ConflictException(7-7)ValidationException(3-3)InternalServerException(8-8)
src/shared/exceptions/DomainExceptions.ts (1)
src/shared/exceptions/AppException.ts (3)
HttpStatus(26-33)ErrorCodes(36-43)AppException(1-23)
src/modules/auth/use-cases/resend-verification-email.usecase.ts (1)
src/modules/user/domain/interfaces/IUserRepository.ts (1)
IUserRepository(3-21)
src/shared/middleware/validation.middleware.ts (1)
src/utils/logger.ts (1)
error(29-31)
src/modules/auth/use-cases/send-verification-email.usecase.ts (1)
src/modules/user/domain/interfaces/IUserRepository.ts (1)
IUserRepository(3-21)
src/shared/middleware/errorHandler.ts (2)
src/config/winston.config.ts (1)
logger(92-92)src/utils/logger.ts (2)
Logger(3-32)error(29-31)
src/modules/auth/presentation/controllers/Auth.controller.ts (3)
src/modules/auth/dto/register.dto.ts (1)
RegisterDto(9-32)src/modules/auth/dto/login.dto.ts (1)
LoginDto(3-10)src/modules/auth/dto/wallet-validation.dto.ts (2)
VerifyWalletDto(17-35)ValidateWalletFormatDto(3-15)
src/modules/auth/use-cases/wallet-format-validation.usecase.ts (1)
src/modules/auth/dto/wallet-validation.dto.ts (1)
ValidateWalletFormatDto(3-15)
src/modules/auth/use-cases/verify-wallet.usecase.ts (1)
src/modules/auth/dto/wallet-validation.dto.ts (1)
VerifyWalletDto(17-35)
🪛 Biome (2.1.2)
src/shared/infrastructure/prisma-error.mapper.ts
[error] 11-11: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🪛 LanguageTool
README.md
[grammar] ~86-~86: There might be a mistake here.
Context: ...(DDD) ## 🚨 Error Handling & Exceptions VolunChain uses a standardized error han...
(QB_NEW_EN)
[grammar] ~92-~92: There might be a mistake here.
Context: ...id input data or DTO validation failures - AuthenticationException (401): Invalid...
(QB_NEW_EN)
[grammar] ~93-~93: There might be a mistake here.
Context: ...ssing tokens, or authentication failures - AuthorizationException (403): Insuffic...
(QB_NEW_EN)
[grammar] ~94-~94: There might be a mistake here.
Context: ...nsufficient permissions or access denied - ConflictException (409): Resource conf...
(QB_NEW_EN)
[grammar] ~95-~95: There might be a mistake here.
Context: ...te emails, unique constraint violations) - InternalServerException (500): Unexpec...
(QB_NEW_EN)
[grammar] ~289-~289: There might be a mistake here.
Context: ...eed ``` --- ## 🔌 Supabase Integration This project uses Supabase for external ...
(QB_NEW_EN)
readme.md
[grammar] ~86-~86: There might be a mistake here.
Context: ...(DDD) ## 🚨 Error Handling & Exceptions VolunChain uses a standardized error han...
(QB_NEW_EN)
[grammar] ~92-~92: There might be a mistake here.
Context: ...id input data or DTO validation failures - AuthenticationException (401): Invalid...
(QB_NEW_EN)
[grammar] ~93-~93: There might be a mistake here.
Context: ...ssing tokens, or authentication failures - AuthorizationException (403): Insuffic...
(QB_NEW_EN)
[grammar] ~94-~94: There might be a mistake here.
Context: ...nsufficient permissions or access denied - ConflictException (409): Resource conf...
(QB_NEW_EN)
[grammar] ~95-~95: There might be a mistake here.
Context: ...te emails, unique constraint violations) - InternalServerException (500): Unexpec...
(QB_NEW_EN)
🔇 Additional comments (6)
src/modules/auth/use-cases/verify-email.usecase.ts (1)
40-46: No changes needed for token clearance.verifyUser()inPrismaUserRepositoryalready setsverificationTokenandverificationTokenExpirestonull, preventing reuse.src/shared/exceptions/index.ts (1)
1-9: Centralized exports look good.Consistent import surface for exceptions.
src/modules/auth/use-cases/email-verification.usecase.ts (1)
16-18: No functional regression: token verification is implemented inVerifyEmailUseCase(verify-email.usecase.ts).EmailVerificationUseCaseis intended for token generation and email sending and still has a TODO for that logic.Likely an incorrect or invalid review comment.
src/shared/infrastructure/prisma-error.mapper.ts (1)
1-1: Import path valid Repo uses @prisma/client v^6.4.1, so importing PrismaClientKnownRequestError from '@prisma/client/runtime/library' is correct.src/shared/exceptions/DomainExceptions.ts (1)
4-6: LGTM — constructors now acceptdetailsand align with AppException.Interfaces are consistent; default message for
InternalServerExceptionis a nice touch.Also applies to: 10-12, 16-18, 22-24, 28-30, 34-36
src/modules/auth/presentation/controllers/Auth.controller.ts (1)
10-10: DTO filename and import casing match exactly Confirmedsrc/modules/auth/dto/resendVerificationDTO.tsexportsResendVerificationDTOand matches the import path../../dto/resendVerificationDTO.
| ### Example Usage | ||
|
|
||
| ```typescript | ||
| // ✅ Correct - Throw domain exceptions | ||
| if (!user) { | ||
| throw new ValidationException("User not found"); | ||
| } | ||
|
|
||
| // ❌ Incorrect - Raw errors or custom responses | ||
| if (!user) { | ||
| throw new Error("User not found"); | ||
| // or | ||
| res.status(400).json({ error: "User not found" }); | ||
| } | ||
| ``` |
There was a problem hiding this comment.
Example should not use ValidationException for missing user.
“User not found” maps to NotFound (404), not Validation (400).
-// ✅ Correct - Throw domain exceptions
-if (!user) {
- throw new ValidationException("User not found");
-}
+// ✅ Correct - Throw domain exceptions
+if (!user) {
+ throw new NotFoundException("User not found");
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ### Example Usage | |
| ```typescript | |
| // ✅ Correct - Throw domain exceptions | |
| if (!user) { | |
| throw new ValidationException("User not found"); | |
| } | |
| // ❌ Incorrect - Raw errors or custom responses | |
| if (!user) { | |
| throw new Error("User not found"); | |
| // or | |
| res.status(400).json({ error: "User not found" }); | |
| } | |
| ``` | |
| // ✅ Correct - Throw domain exceptions | |
| if (!user) { | |
| throw new NotFoundException("User not found"); | |
| } |
🤖 Prompt for AI Agents
In readme.md around lines 120 to 134, the example incorrectly uses
ValidationException for a missing user; change the example to throw the correct
domain NotFound exception (or a NotFoundException) to map to 404, and update the
inline comments to show that missing resources should throw NotFound rather than
Validation; keep the “❌ Incorrect” section showing raw Error or manual
res.status usage as bad practice but replace the ValidationExample with a
NotFound example.
| - **ValidationException (400)**: Invalid input data or DTO validation failures | ||
| - **AuthenticationException (401)**: Invalid credentials, missing tokens, or authentication failures | ||
| - **AuthorizationException (403)**: Insufficient permissions or access denied | ||
| - **ConflictException (409)**: Resource conflicts (e.g., duplicate emails, unique constraint violations) | ||
| - **InternalServerException (500)**: Unexpected system errors or database failures | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Include NotFoundException (404) in the allowed list.
Docs omit 404 while the code exports NotFoundException. Add it for consistency.
- **ConflictException (409)**: Resource conflicts (e.g., duplicate emails, unique constraint violations)
- **InternalServerException (500)**: Unexpected system errors or database failures
+- **NotFoundException (404)**: Requested resource does not existCommittable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 LanguageTool
[grammar] ~92-~92: There might be a mistake here.
Context: ...id input data or DTO validation failures - AuthenticationException (401): Invalid...
(QB_NEW_EN)
[grammar] ~93-~93: There might be a mistake here.
Context: ...ssing tokens, or authentication failures - AuthorizationException (403): Insuffic...
(QB_NEW_EN)
[grammar] ~94-~94: There might be a mistake here.
Context: ...nsufficient permissions or access denied - ConflictException (409): Resource conf...
(QB_NEW_EN)
[grammar] ~95-~95: There might be a mistake here.
Context: ...te emails, unique constraint violations) - InternalServerException (500): Unexpec...
(QB_NEW_EN)
🤖 Prompt for AI Agents
In README.md around lines 92 to 97, the error list is missing the
NotFoundException (404) despite the code exporting it; add a new bullet to the
list such as "- **NotFoundException (404)**: Resource not found or does not
exist" (matching style of other entries) so the docs reflect the actual exported
exceptions and HTTP semantics.
| if (!user) { | ||
| throw new ValidationException("User not found"); | ||
| } | ||
|
|
||
| // ❌ Incorrect - Raw errors or custom responses | ||
| if (!user) { | ||
| throw new Error("User not found"); | ||
| // or | ||
| res.status(400).json({ error: "User not found" }); | ||
| } | ||
| ``` |
There was a problem hiding this comment.
Fix exception type in example.
“User not found” → NotFoundException (404), not ValidationException.
-// ✅ Correct - Throw domain exceptions
-if (!user) {
- throw new ValidationException("User not found");
-}
+// ✅ Correct - Throw domain exceptions
+if (!user) {
+ throw new NotFoundException("User not found");
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (!user) { | |
| throw new ValidationException("User not found"); | |
| } | |
| // ❌ Incorrect - Raw errors or custom responses | |
| if (!user) { | |
| throw new Error("User not found"); | |
| // or | |
| res.status(400).json({ error: "User not found" }); | |
| } | |
| ``` | |
| // ✅ Correct - Throw domain exceptions | |
| if (!user) { | |
| throw new NotFoundException("User not found"); | |
| } | |
| // ❌ Incorrect - Raw errors or custom responses | |
| if (!user) { | |
| throw new Error("User not found"); | |
| // or | |
| res.status(400).json({ error: "User not found" }); | |
| } |
🤖 Prompt for AI Agents
In README.md around lines 124 to 134, the example throws the wrong exception
type for a missing user: it currently shows ValidationException (and contrasts
with raw Error/JSON). Replace the ValidationException with a NotFoundException
(HTTP 404) in the example, and update any surrounding explanatory text to
reflect that a missing resource should map to NotFound/404 rather than a
validation error; keep the rest of the example behavior the same.
| ## 🔌 Supabase Integration | ||
|
|
||
| This project uses Supabase for external data access and future integrations. | ||
|
|
||
| Update your `.env` file with: | ||
|
|
||
| ```bash | ||
| SUPABASE_URL=... | ||
| SUPABASE_ANON_KEY=... | ||
| ``` | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid README/readme duplication (case conflict).
Both README.md and readme.md exist; this breaks clones on case-insensitive FS (macOS/Windows) and confuses links. Keep a single README.md.
🧰 Tools
🪛 LanguageTool
[grammar] ~289-~289: There might be a mistake here.
Context: ...eed ``` --- ## 🔌 Supabase Integration This project uses Supabase for external ...
(QB_NEW_EN)
🤖 Prompt for AI Agents
In README.md around lines 289–299 there is a duplicate lower-case readme
(readme.md) in the repo which causes conflicts on case-insensitive filesystems;
consolidate content into a single README.md, remove or rename the redundant
readme.md from the repository, update any internal links or references to point
to README.md, and commit the change with a proper repo-case fix sequence so the
filename case change is recorded on case-insensitive systems.
| const formattedErrors = errors.map(error => ({ | ||
| property: error.property, | ||
| value: error.value, | ||
| constraints: error.constraints ? Object.values(error.constraints) : [], | ||
| })); | ||
| throw new ValidationException('DTO validation failed', { errors: formattedErrors }); |
There was a problem hiding this comment.
Do not include raw input values in validation error details (PII leak).
value: error.value can expose secrets (e.g., passwords). Remove it from the response details.
- const formattedErrors = errors.map(error => ({
- property: error.property,
- value: error.value,
- constraints: error.constraints ? Object.values(error.constraints) : [],
- }));
+ const formattedErrors = errors.map(error => ({
+ property: error.property,
+ constraints: error.constraints ? Object.values(error.constraints) : [],
+ }));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const formattedErrors = errors.map(error => ({ | |
| property: error.property, | |
| value: error.value, | |
| constraints: error.constraints ? Object.values(error.constraints) : [], | |
| })); | |
| throw new ValidationException('DTO validation failed', { errors: formattedErrors }); | |
| const formattedErrors = errors.map(error => ({ | |
| property: error.property, | |
| constraints: error.constraints ? Object.values(error.constraints) : [], | |
| })); | |
| throw new ValidationException('DTO validation failed', { errors: formattedErrors }); |
🤖 Prompt for AI Agents
In src/modules/auth/presentation/controllers/Auth.controller.ts around lines 51
to 56, the validation error payload currently includes raw input values (value:
error.value) which can leak PII/secrets; remove the value field from
formattedErrors and only include non-sensitive fields (e.g., property and
constraints as an array). Update the formattedErrors mapping to omit
error.value, adjust any ValidationException payload typing if necessary, and
ensure no other code paths log or return raw DTO values in validation errors.
| default: | ||
| // Unknown Prisma error | ||
| throw new InternalServerException('Database error', { | ||
| code: error.code, | ||
| message: error.message | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid leaking raw DB error messages to clients.
error.message may expose internal DB details. Remove it (or gate it behind NODE_ENV). Minimal fix below.
- default:
- // Unknown Prisma error
- throw new InternalServerException('Database error', {
- code: error.code,
- message: error.message
- });
+ default:
+ // Unknown Prisma error
+ throw new InternalServerException('Database error', {
+ code: error.code,
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| default: | |
| // Unknown Prisma error | |
| throw new InternalServerException('Database error', { | |
| code: error.code, | |
| message: error.message | |
| }); | |
| default: | |
| // Unknown Prisma error | |
| throw new InternalServerException('Database error', { | |
| code: error.code, | |
| }); |
🤖 Prompt for AI Agents
In src/shared/infrastructure/prisma-error.mapper.ts around lines 36 to 41, the
default branch currently throws an InternalServerException including raw
error.message which can leak DB internals; remove error.message from the
client-facing payload (leave code or a generic message), or conditionally
include the detailed error only when NODE_ENV !== 'production' (e.g., attach
details to logs or to an internal-only field), and ensure the thrown exception
sent to clients contains no raw DB message.
| message: string; | ||
| details?: unknown; | ||
| traceId?: string; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Ensure 5xx responses use a generic message.
Do not echo arbitrary error.message to clients; log it, but return a safe, generic message (the else-branch currently passes through error.message to InternalServerException).
Please confirm InternalServerException.toJSON() always emits a generic message for 5xx. If not, change the else-branch to construct without passing the original message and rely on logging for specifics.
🌐 Web query:
Verify whether InternalServerException in this repo uses a fixed generic client-facing message, regardless of the constructor message.
💡 Result:
I don’t have the repository—please share the repo (or path/file) for InternalServerException (or paste the class code). I’ll inspect and confirm whether it always returns a fixed client-facing message.
🏁 Script executed:
#!/bin/bash
# Locate InternalServerException class definition and source file
rg -nP "class\s+InternalServerException" -n --type tsLength of output: 160
🏁 Script executed:
#!/bin/bash
# Show InternalServerException definition
rg -nP -C5 "class InternalServerException" -n src/shared/exceptions/DomainExceptions.ts
# Locate AppException class file
rg -nP "class\s+AppException" -l --type ts
# Once found, show AppException and its toJSON implementation
APP_FILE=$(rg -nP "class\s+AppException" -l --type ts)
rg -nP -C5 "class AppException" -n "$APP_FILE"
rg -nP "toJSON" -n "$APP_FILE"Length of output: 1083
🏁 Script executed:
#!/bin/bash
# Show the full toJSON method implementation with context
rg -nP -C10 "public toJSON" -n src/shared/exceptions/AppException.tsLength of output: 687
Ensure errorHandler doesn’t expose raw error messages
In src/shared/middleware/errorHandler.ts (else-branch), avoid doing:
throw new InternalServerException(error.message);instead call
throw new InternalServerException();so toJSON() will emit only the default “Internal server error” message; log the original error.message separately.
🤖 Prompt for AI Agents
In src/shared/middleware/errorHandler.ts around lines 11–13, the else-branch
currently throws an InternalServerException with the raw error.message which
exposes internal details; change it to throw new InternalServerException() (no
message) and separately log the original error.message (and stack) to the server
logger (e.g., logger.error or console.error) so toJSON() only returns the
generic "Internal server error" while the original error is recorded for
debugging.
| // Log the error with context | ||
| logger.error("Unhandled error occurred", { | ||
| errorCode: response.errorCode, | ||
| status: response.statusCode, | ||
| message: response.message, | ||
| traceId, | ||
| path: req.path, | ||
| method: req.method, | ||
| stack: error.stack, | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use WARN for 4xx and ERROR for 5xx; include details in meta.
Reduces noise for expected client errors and preserves context.
- // Log the error with context
- logger.error("Unhandled error occurred", {
- errorCode: response.errorCode,
- status: response.statusCode,
- message: response.message,
- traceId,
- path: req.path,
- method: req.method,
- stack: error.stack,
- });
+ // Log with appropriate severity
+ const meta = {
+ errorCode: response.errorCode,
+ status: response.statusCode,
+ message: response.message,
+ details: response.details,
+ traceId,
+ path: req.path,
+ method: req.method,
+ stack: error.stack,
+ };
+ const logFn = response.statusCode >= 500 ? logger.error.bind(logger) : logger.warn.bind(logger);
+ logFn("Unhandled error occurred", meta);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Log the error with context | |
| logger.error("Unhandled error occurred", { | |
| errorCode: response.errorCode, | |
| status: response.statusCode, | |
| message: response.message, | |
| traceId, | |
| path: req.path, | |
| method: req.method, | |
| stack: error.stack, | |
| }); | |
| // Log with appropriate severity | |
| const meta = { | |
| errorCode: response.errorCode, | |
| status: response.statusCode, | |
| message: response.message, | |
| details: response.details, | |
| traceId, | |
| path: req.path, | |
| method: req.method, | |
| stack: error.stack, | |
| }; | |
| const logFn = | |
| response.statusCode >= 500 | |
| ? logger.error.bind(logger) | |
| : logger.warn.bind(logger); | |
| logFn("Unhandled error occurred", meta); |
🤖 Prompt for AI Agents
In src/shared/middleware/errorHandler.ts around lines 43 to 52, change the
single logger.error call to select the log level based on response.statusCode:
use logger.warn(...) for 4xx status codes (400–499) and logger.error(...) for
5xx status codes (500–599); pass the same metadata object (errorCode, status,
message, traceId, path, method, stack) into the chosen logger so all details
remain in the meta.
| constraints: string[]; | ||
| }>; | ||
| } | ||
| import { ValidationException } from "../exceptions"; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Wrong exception type for unexpected errors + async Express propagation via next().
Unexpected errors should be InternalServerException, and async middleware should call next(error) (unless you use express-async-errors). Current throws risk wrong 4xx classification and unhandled rejections.
-import { ValidationException } from "../exceptions";
+import { ValidationException, InternalServerException } from "../exceptions";
@@
- } catch (error) {
- // Re-throw domain exceptions as-is
- if (error instanceof ValidationException) {
- throw error;
- }
- // Wrap unexpected errors
- throw new ValidationException('Internal server error during validation');
- }
+ } catch (error) {
+ if (error instanceof ValidationException) return next(error);
+ return next(
+ new InternalServerException('Internal server error during DTO validation', { cause: error })
+ );
+ }
@@
- } catch (error) {
- // Re-throw domain exceptions as-is
- if (error instanceof ValidationException) {
- throw error;
- }
- // Wrap unexpected errors
- throw new ValidationException('Internal server error during query validation');
- }
+ } catch (error) {
+ if (error instanceof ValidationException) return next(error);
+ return next(
+ new InternalServerException('Internal server error during query validation', { cause: error })
+ );
+ }
@@
- } catch (error) {
- // Re-throw domain exceptions as-is
- if (error instanceof ValidationException) {
- throw error;
- }
- // Wrap unexpected errors
- throw new ValidationException('Internal server error during parameter validation');
- }
+ } catch (error) {
+ if (error instanceof ValidationException) return next(error);
+ return next(
+ new InternalServerException('Internal server error during parameter validation', { cause: error })
+ );
+ }If you rely on express-async-errors, confirm it’s wired; otherwise this change is required for correct propagation.
Also applies to: 30-36, 65-71, 100-106
🏁 Script executed:
#!/bin/bash
set -e
# Locate the validation middleware file
file=$(fd validation.middleware.ts | grep 'src/shared/middleware' | head -n1)
echo "Located file: $file"
# Show the import and the first middleware function
grep -n 'import.*ValidationException' "$file"
nl -ba "$file" | sed -n '1,200p'Length of output: 573
🏁 Script executed:
#!/bin/bash
set -e
# Show content of validation.middleware.ts with line numbers
cat -n src/shared/middleware/validation.middleware.ts | sed -n '1,200p'
# Search for express-async-errors usage in the codebase
grep -R "express-async-errors" -n src || echo "No express-async-errors import found"Length of output: 4231
🏁 Script executed:
#!/bin/bash
# Search entire repository for express-async-errors import or require
grep -R "express-async-errors" -n . || echo "No express-async-errors usage found"Length of output: 116
🏁 Script executed:
#!/bin/bash
set -e
# Verify InternalServerException is defined
grep -R "class InternalServerException" -n src/shared/exceptions
# Verify ValidationException is defined
grep -R "class ValidationException" -n src/shared/exceptionsLength of output: 343
Use next(error) with InternalServerException for unexpected errors in async middleware
Express isn’t using express-async-errors (no import found), so thrown errors in these async middleware won’t be passed to next(error)—resulting in unhandled promise rejections and 4xx status codes for internal failures. In each validateDto, validateQueryDto, and validateParamsDto catch block (lines 30–36, 65–71, 100–106), do not throw; instead call next(error) for ValidationException and wrap all other errors in InternalServerException via next(...). Ensure you import InternalServerException alongside ValidationException at line 4.
-import { ValidationException } from "../exceptions";
+import { ValidationException, InternalServerException } from "../exceptions";
@@ catch (error) {
- if (error instanceof ValidationException) {
- throw error;
- }
- throw new ValidationException('Internal server error during validation');
+ if (error instanceof ValidationException) return next(error);
+ return next(new InternalServerException(
+ 'Internal server error during DTO validation', { cause: error }
+ ));
}(Apply analogous changes to query and params validation blocks.)
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { ValidationException } from "../exceptions"; | |
| // File: src/shared/middleware/validation.middleware.ts | |
| import { ValidationException, InternalServerException } from "../exceptions"; | |
| export const validateDto = (DtoClass: any) => async (req, res, next) => { | |
| try { | |
| // ... existing validation logic | |
| } catch (error) { | |
| - if (error instanceof ValidationException) { | |
| - throw error; | |
| - } | |
| if (error instanceof ValidationException) return next(error); | |
| return next(new InternalServerException( | |
| 'Internal server error during DTO validation', | |
| { cause: error } | |
| )); | |
| } | |
| }; |
🤖 Prompt for AI Agents
In src/shared/middleware/validation.middleware.ts around line 4 and catch blocks
at ~30–36, ~65–71, and ~100–106: import InternalServerException alongside
ValidationException at line 4, and in each catch block replace throwing with
calls to next(...): if the caught error is a ValidationException call
next(error), otherwise wrap the error in a new InternalServerException and call
next(new InternalServerException(error)); do this for validateDto,
validateQueryDto, and validateParamsDto so unexpected async errors are forwarded
to Express error handling rather than thrown.
| const formattedErrors = errors.map((error: ValidationError) => ({ | ||
| property: error.property, | ||
| value: error.value, | ||
| constraints: error.constraints | ||
| ? Object.values(error.constraints) | ||
| : [], | ||
| })); |
There was a problem hiding this comment.
Do not include raw input values in validation error payloads (PII leak).
Echoing error.value can leak sensitive data (e.g., emails, tokens, passwords) to clients and logs. Return only property and messages.
- const formattedErrors = errors.map((error: ValidationError) => ({
- property: error.property,
- value: error.value,
- constraints: error.constraints
- ? Object.values(error.constraints)
- : [],
- }));
+ const formattedErrors = errors.map((error: ValidationError) => ({
+ property: error.property,
+ constraints: error.constraints ? Object.values(error.constraints) : [],
+ }));Also applies to: 52-58, 87-93
🤖 Prompt for AI Agents
In src/shared/middleware/validation.middleware.ts around lines 17-23 (and
similarly at 52-58 and 87-93), the code currently includes error.value in the
validation error payload which can leak PII; remove any inclusion of error.value
and only return the property name and the list of constraint messages (i.e., map
error.constraints to an array of messages), update all three locations to
produce objects like { property, messages: [...] } (or keep the existing key for
messages if preferred), and ensure no raw input values are logged or sent to the
client in those branches.
1 participant
🚀 Volunchain Pull Request
Mark with an
xall the checkboxes that apply (like[x])📌 Type of Change
📝 Changes description
ValidationException,AuthenticationException,AuthorizationException,ConflictException,InternalServerException).P2002 → ConflictException,P2025 → ValidationException, fallback →InternalServerException).{ "statusCode": 400, "errorCode": "VALIDATION_ERROR", "message": "DTO validation failed", "details": {} }Summary by CodeRabbit
Documentation
Behavior
Tests