Skip to content

fix: use NestJS HTTP exceptions in vouching service#84

Open
Doiae9 wants to merge 1 commit into
StepFi-app:mainfrom
Doiae9:fix/nestjs-http-exceptions
Open

fix: use NestJS HTTP exceptions in vouching service#84
Doiae9 wants to merge 1 commit into
StepFi-app:mainfrom
Doiae9:fix/nestjs-http-exceptions

Conversation

@Doiae9

@Doiae9 Doiae9 commented Jul 2, 2026

Copy link
Copy Markdown

Fixes #68

  • Replaced all new Error(...) instances with appropriate NestJS HTTP exceptions (InternalServerErrorException, BadRequestException, NotFoundException) in vouching.service.ts.
  • Verified and applied the same fix in vendors.service.ts at line 63.

@Doiae9
Doiae9 requested a review from EmeditWeb as a code owner July 2, 2026 21:53

@EmeditWeb EmeditWeb left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Review verdict: ✅ APPROVE with minor corrections

Correct intent and matches the code standard (surface NestJS HTTP exceptions instead of bare throw new Error(...), which would otherwise all collapse to generic 500s). The InternalServerErrorException swaps in vendors/vouching are right.

Minor corrections before merge:

  1. Indentation is broken in the new vouching.service.ts block (the if (error) { ... } if (!data) { ... } section is mis-indented vs the surrounding 4-space code). npm run lint (eslint --fix) should catch it — please run it.
  2. Non-English comments: // Es un fallo del sistema/BD and // Es un fallo por datos inválidos del cliente — please translate to English (or remove) to match the rest of the codebase.
  3. Mis-classified exception: the !data case throws BadRequestException('Invalid vouch request data provided.'). A successful insert that returns no row is a server-side condition, not client bad input — this should be InternalServerErrorException (500), otherwise clients get a misleading 400.
  4. Trailing whitespace on the new import line.
  5. Resolve conflicts at src/modules/vouching/vouching.service.ts

Small stuff — approve once cleaned up.

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.

fix: use NestJS HTTP exceptions in vouching service

2 participants