chore(api): perf section 1 — compression, SNS fetch timeouts, pagination#201
Merged
Conversation
H1 — Add response compression (gzip/deflate) after Helmet in main.ts.
1 KB threshold; respects Cache-Control: no-transform. JSON bodies don't
mix secrets + user input, so BREACH-safe.
H2 — 5 s AbortSignal.timeout on both SNS fetches:
- sns-signature-validator.service.ts cert fetch
- ses-events.controller.ts SubscribeURL confirm fetch
Prevents an AWS edge blip from pinning a request slot indefinitely.
H3 — 60 s app-level request timeout middleware (matches ALB idle timeout).
Returns clean 503 instead of letting the ALB 504.
H4 — CORS preflight maxAge: 86_400 (24 h, browser ceiling) +
optionsSuccessStatus: 204.
DB1+DB2 — Pagination on 6 unbounded findMany endpoints:
- GET /admin/settings/countries
- GET /admin/settings/categories
- GET /admin/settings/trending
- GET /admin/payouts/export (ExportPaginationDto — 1000 default, 5000 max)
- GET /offers (PaginationDto — 50 default, 200 max)
- GET /auth/sessions (PaginationDto)
Reuses existing admin PaginationDto (page/limit/max=100) for the 3 admin
settings lists. New common/dto/pagination.dto.ts adds PaginationDto (50/200)
+ ExportPaginationDto (1000/5000) for non-admin routes and the payouts
export. Response shape stays as bare arrays — no contract break.
DB3 — bookings.service.ts:1100 voucher fetch tightened from
include: { coupon: true } to a select scoped to the 8 fields actually used
(id, code, status, validTo, minOrderAmount, discountType, discountValue,
maxDiscount). Type-check is the load-bearing guard against missed fields.
DB5 — LoyaltyLedger @@index([bookingId]) was already present in
schema.prisma:666; no migration needed.
Tests: 701 unit tests pass. Updated auth.controller.spec.ts callsites
for the new getSessions(query) arg. Added regression test asserting the
cert fetch carries an AbortSignal (so the 5 s timeout can't be silently
dropped in a future refactor).
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR adds pagination to multiple API endpoints (admin, auth, catalog) via new reusable DTOs, introduces HTTP compression and request timeout middleware, and hardens external fetch operations with 5-second timeouts to prevent indefinite hangs during transient failures. ChangesPagination Feature
Server Infrastructure
Resilience & Optimization
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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: 2
🧹 Nitpick comments (2)
apps/api/src/common/dto/pagination.dto.ts (1)
16-49: ⚡ Quick winConsider bounding
pageto defang offset-scan attacks.
limitis capped at 200 / 5 000, butpageis unbounded. A client sending?page=999999999&limit=200forces Prisma to issue anOFFSET 199_999_999_800against the underlying table — Postgres still has to walk and discard those rows before returning the (empty) slice, which is essentially a free DoS knob on every paginated endpoint that adopts these DTOs. Sincebookings.service.ts:getMyBookingsalready uses aMath.min(..., 1000)page cap, mirroring that here keeps the policy consistent across the codebase.♻️ Suggested fix — cap `page` on both DTOs
export class PaginationDto { `@IsOptional`() `@Type`(() => Number) `@IsInt`() `@Min`(1) + `@Max`(10_000) page?: number = 1; ... export class ExportPaginationDto { `@IsOptional`() `@Type`(() => Number) `@IsInt`() `@Min`(1) + `@Max`(10_000) page?: number = 1;🤖 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 `@apps/api/src/common/dto/pagination.dto.ts` around lines 16 - 49, Add an upper bound to the page field on both PaginationDto and ExportPaginationDto to prevent huge OFFSET scans; specifically, annotate the page property with `@Max`(1000) (matching the cap used in bookings.service.ts:getMyBookings) so page stays within a safe range while preserving the existing defaults and other decorators.apps/api/test/unit/auth.controller.spec.ts (1)
211-228: ⚡ Quick winAdd explicit assertions for default
take/skipin the sessions query.The test now exercises the new signature, but it still doesn’t verify that pagination is actually forwarded to Prisma. Adding this assertion will lock in the PR’s functional intent.
Proposed test assertion update
const r = await ctrl.getSessions({ id: 'u1', role: 'VENDOR' } as any, req, {}); + expect(prisma._client.refreshToken.findMany).toHaveBeenCalledWith( + expect.objectContaining({ + take: 50, + skip: 0, + }), + ); + expect(authSvc.getTokenHash).toHaveBeenCalledWith('rtoken'); expect(r).toHaveLength(2);🤖 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 `@apps/api/test/unit/auth.controller.spec.ts` around lines 211 - 228, Add an assertion that the sessions query forwards the controller's default pagination to Prisma: after calling ctrl.getSessions, assert that prisma._client.refreshToken.findMany was invoked with an argument containing the default take and skip values (e.g., take: 10, skip: 0) so the test locks in pagination behavior; use an object-containing matcher when checking the call to refreshToken.findMany to avoid coupling to unrelated query fields and reference ctrl.getSessions and prisma._client.refreshToken.findMany in the assertion.
🤖 Prompt for all review comments with 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.
Inline comments:
In `@apps/api/src/admin/admin.service.ts`:
- Around line 1389-1393: The paginated Prisma queries (e.g., the
this.prisma.client.country.findMany call in admin.service.ts) use a non-unique
single-column orderBy (nameEn) which can produce unstable paging; update those
orderBy clauses to include a deterministic secondary key by adding id as a
tie-breaker (e.g., orderBy: [{ nameEn: 'asc' }, { id: 'asc' }]). Apply the same
change to the other similar findMany queries referenced in this review (the
calls around the other spots noted) so all paginated queries use a stable
composite ordering.
In `@apps/api/src/main.ts`:
- Around line 298-310: The middleware registering a per-request timeout via
req.setTimeout is leaving a persistent 'timeout' listener on keep-alive sockets;
change the middleware so you install an explicit timeout handler (e.g. const
onTimeout = () => { if (!res.headersSent) res.status(503).json({ message:
'Request timeout' }); req.destroy(); }), register it with req.setTimeout or
req.on('timeout', onTimeout) on entry, and then remove that listener on response
completion (res.on('finish', ...) and res.on('close', ...):
req.removeListener('timeout', onTimeout')) so the timeout callback is cleared
when a request completes; keep the same 60_000ms value and the existing behavior
inside onTimeout.
---
Nitpick comments:
In `@apps/api/src/common/dto/pagination.dto.ts`:
- Around line 16-49: Add an upper bound to the page field on both PaginationDto
and ExportPaginationDto to prevent huge OFFSET scans; specifically, annotate the
page property with `@Max`(1000) (matching the cap used in
bookings.service.ts:getMyBookings) so page stays within a safe range while
preserving the existing defaults and other decorators.
In `@apps/api/test/unit/auth.controller.spec.ts`:
- Around line 211-228: Add an assertion that the sessions query forwards the
controller's default pagination to Prisma: after calling ctrl.getSessions,
assert that prisma._client.refreshToken.findMany was invoked with an argument
containing the default take and skip values (e.g., take: 10, skip: 0) so the
test locks in pagination behavior; use an object-containing matcher when
checking the call to refreshToken.findMany to avoid coupling to unrelated query
fields and reference ctrl.getSessions and prisma._client.refreshToken.findMany
in the assertion.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 707ac90f-2ab9-4ac5-95cc-b2e93c9a73b7
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json,!package-lock.json,!**/package-lock.json
📒 Files selected for processing (12)
apps/api/package.jsonapps/api/src/admin/admin.controller.tsapps/api/src/admin/admin.service.tsapps/api/src/auth/auth.controller.tsapps/api/src/bookings/bookings.service.tsapps/api/src/catalog/offers.controller.tsapps/api/src/common/dto/pagination.dto.tsapps/api/src/email/ses-events.controller.tsapps/api/src/email/sns-signature-validator.service.tsapps/api/src/main.tsapps/api/test/unit/auth.controller.spec.tsapps/api/test/unit/sns-signature-validator.spec.ts
Two follow-ups on the perf section-1 PR. Finding 1 (Major) — request-timeout middleware was destroying healthy keep-alive sockets 60s after the last response. req.setTimeout sets the timeout on the underlying socket and the 'timeout' listener does not clear when the response finishes, so an idle-but-healthy keep-alive socket behind the ALB was forced to redo TCP+TLS handshake on every new request. Each request was also stacking a fresh 'timeout' listener on the same socket. Clear timeout + remove the listener on res 'finish' and 'close' so the protection only fires on actually-hung requests. Finding 2 (Minor) — add id as a secondary sort key on all 6 paginated findMany calls. Single-column orderBy on a non-unique field (nameEn, createdAt, lastUsedAt) lets Postgres return tied rows in arbitrary order — page boundaries can duplicate or skip rows. Composite ordering makes page boundaries deterministic across requests. Endpoints updated: - admin countries: [nameEn asc, id asc] - admin categories: [nameEn asc, id asc] - admin trending events: [createdAt desc, id desc] - admin payouts export: [createdAt desc, id desc] - /offers: [createdAt desc, id desc] - /auth/sessions: [lastUsedAt desc, id desc] Tests: 701/701 unit pass, type-check clean, lint clean.
10 tasks
This was referenced May 15, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Section 1 of the recent perf/reliability audit — six items in one PR. All low-risk, no DB migration, no frontend changes required (default-50 pagination keeps existing list shapes intact).
apps/api/src/main.tssns-signature-validator.service.ts:208ses-events.controller.ts:231main.ts(after CORS)maxAge: 86_400+optionsSuccessStatus: 204main.ts:267include: { coupon: true }→ explicitselectof 8 used fieldsbookings.service.ts:1100Pagination endpoints
GET /admin/settings/countriesPaginationDto(admin)GET /admin/settings/categoriesPaginationDto(admin)GET /admin/settings/trendingPaginationDto(admin)GET /admin/payouts/exportExportPaginationDto(new, common)GET /offersPaginationDto(new, common)GET /auth/sessionsPaginationDto(new, common)Response shape stays a bare array — no contract break. Clients that need more pass
?limit=…&page=….Items not included
@@index([bookingId])— already present atschema.prisma:666; audit was stale.Test plan
npx tsc --noEmitcleannpx eslinton all 9 changed source files — no new warningsnpx jest test/unit— 701 tests pass, 33 suitessns-signature-validator.spec.tspins theAbortSignalarg into the certfetch()call so the 5 s timeout can't be silently droppedauth.controller.spec.tscallsites for the newgetSessions(query)argRisk
Low. Runtime-only changes. No schema migration, no SSM, no IAM. Pagination default-50 cap means clients calling the 6 affected endpoints get at most 50 items by default instead of "all" — every current list is well under 50 rows. Rollback:
git revert.Summary by CodeRabbit
Release Notes
New Features
Performance
Bug Fixes