fix(reviews): validate limit/offset/gig_id, log errors, expose validation issues (matches #387-#390)

[jhosepm352-design]

Bug Fix: /api/reviews input validation

Continuation of #387 (bounties), #388 (gigs), #389 (notifications), #390 (activity). This PR applies the same input validation pattern to /api/reviews.

Bugs Found

Bug 1: ?limit=abc returns 200
parseInt("abc", 10) returns NaN. Falls back to default 20 silently.

Bug 2: ?limit=99999 returns 200
No upper bound validation, silently capped at 50.

Bug 3: ?limit=0 returns 200
Should be 400 - limit must be >= 1.

Bug 4: ?offset=-1 returns 200
Should be 400 - offset must be >= 0.

Bug 5: ?gig_id=not-a-uuid returns 200
No UUID validation. Supabase query is sent unfiltered, returns empty array, but client receives 200 OK with no error feedback.

Bug 6: POST validation returns only first issue

{ error: validationResult.error.issues[0].message }

Returns only the first Zod issue, masking all other validation problems. Should return full issues array.

Bug 7: Generic 500 errors with no logging
The catch block returns "An unexpected error occurred" with no console.error.

Changes

1. Validate limit: must be positive integer (>= 1), returns 400 otherwise.
2. Validate offset: must be non-negative integer (>= 0), returns 400 otherwise.
3. Validate gig_id as UUID v4 with regex (returns 400 if invalid).
4. Cap at MAX_REVIEW_LIMIT (50) and MAX_REVIEW_OFFSET (100,000).
5. POST validation returns full issues array (not just first).
6. Added console.error logging for both Supabase errors and unexpected errors.

Testing

curl 'https://ugig.net/api/reviews?limit=abc'
# 400: {"error":"Invalid limit. Must be a positive integer (>= 1)."}

curl 'https://ugig.net/api/reviews?offset=-1'
# 400: {"error":"Invalid offset. Must be a non-negative integer (>= 0)."}

curl 'https://ugig.net/api/reviews?gig_id=not-a-uuid'
# 400: {"error":"Invalid gig_id. Must be a valid UUID."}

Related

• Continuation of fix(bounties): validate inputs, return pagination, log errors, expose updated_at/questions #387, fix(gigs): validate status, page, limit query params (matches /api/bounties fix in #387) #388, fix(notifications): validate limit/offset query params (matches #387, #388) #389, fix(activity): validate limit/offset query params (matches #387, #388, #389) #390
• Same pattern, same author
• File changed: 1 (src/app/api/reviews/route.ts)
• 8457 -> ~9700 bytes (estimated)

Looking forward to your review.

[greptile-apps]

Greptile Summary

This PR adds input validation for limit, offset, and gig_id query parameters in GET /api/reviews, exposes the full Zod issues array for POST validation failures, and adds console.error logging to Supabase error paths. GET /api/bounties receives the same treatment plus a new pagination metadata envelope in the response.

• reviews/route.ts: Inline validation replaces parsePaginationParam for limit/offset, UUID regex guards gig_id, full issues array is now returned on POST schema failures, and console.error is added to the GET catch block — but the POST catch block was missed and still swallows unexpected errors silently.
• bounties/route.ts: Status, limit, and page validation added, pagination metadata (page, limit, total, total_pages) added to GET response, and error logging added to both catch blocks; the POST validation response now includes issues but still uses the first issue's message as the top-level error, inconsistent with how reviews was fixed.

Confidence Score: 3/5

Safe to merge for the validation improvements, but the POST handler's catch block in reviews/route.ts was not updated and still silently swallows unexpected errors with no logging — exactly the problem this PR set out to fix.

The GET path in reviews is correctly hardened and the Supabase error paths now log. However, the POST handler's catch {} block — which covers a large surface including reputation hooks, email sending, webhook dispatch, and notification inserts — still has no error binding and no console.error. Any crash in that path produces a silent 500 in production. The unused parsePaginationParam import is also a build-time concern if the project has strict no-unused-imports linting.

src/app/api/reviews/route.ts — the POST handler catch block needs the same logging treatment applied to the GET handler

Important Files Changed

Filename	Overview
src/app/api/reviews/route.ts	Adds inline validation for limit/offset/gig_id query params and exposes full Zod issues array on POST; GET catch logging added but POST catch block still uses bare catch {} with no logging, and the parsePaginationParam import is now unused.
src/app/api/bounties/route.ts	Adds status/limit/page validation, pagination metadata in response, and error logging; POST validation response inconsistently still exposes only the first Zod issue message as error while also adding the full issues array.

Sequence Diagram

sequenceDiagram
    participant Client
    participant GET /api/reviews
    participant POST /api/reviews
    participant Supabase

    Client->>GET /api/reviews: ?gig_id=X&limit=Y&offset=Z
    GET /api/reviews->>GET /api/reviews: Validate gig_id (UUID regex)
    GET /api/reviews->>GET /api/reviews: Validate limit (>= 1, integer, cap 50)
    GET /api/reviews->>GET /api/reviews: Validate offset (>= 0, integer, cap 100k)
    alt Invalid param
        GET /api/reviews-->>Client: 400 { error: "..." }
    else Valid
        GET /api/reviews->>Supabase: SELECT reviews WHERE gig_id=X LIMIT Y OFFSET Z
        Supabase-->>GET /api/reviews: data | error
        alt Supabase error
            GET /api/reviews->>GET /api/reviews: console.error (logged)
            GET /api/reviews-->>Client: 400 { error: message }
        else Success
            GET /api/reviews-->>Client: 200 { data, pagination }
        end
    end

    Client->>POST /api/reviews: { gig_id, reviewee_id, rating, comment }
    POST /api/reviews->>POST /api/reviews: Zod schema validation
    alt Invalid body
        POST /api/reviews-->>Client: 400 { error: "Invalid review payload", issues: [...] }
    else Valid
        POST /api/reviews->>Supabase: INSERT review
        Supabase-->>POST /api/reviews: data | error
        alt Supabase error
            POST /api/reviews->>POST /api/reviews: console.error (logged)
            POST /api/reviews-->>Client: 400 { error: message }
        else Unexpected exception
            POST /api/reviews->>POST /api/reviews: catch{} NOT logged
            POST /api/reviews-->>Client: 500 { error: "An unexpected error occurred" }
        else Success
            POST /api/reviews-->>Client: 201 { data: review }
        end
    end

Loading

Comments Outside Diff (2)

1. src/app/api/reviews/route.ts, line 329-334 (link)

   POST catch block still missing error logging

   The GET handler's catch was updated to bind err and call console.error, but the POST handler's catch (line 329) still uses a bare } catch { with no variable binding and no logging. Any unexpected exception thrown during review creation — JSON parse failures, reputation hook crashes, email errors, webhook dispatch errors — will be silently swallowed and returned as a generic 500 with zero diagnostic output in production logs. The PR explicitly targets this pattern as a bug to fix, but the fix was only partially applied.

2. src/app/api/reviews/route.ts, line 9 (link)

   Unused import left behind

   parsePaginationParam was imported from @/lib/api-pagination but is no longer referenced now that limit/offset validation is inlined. Depending on the project's TypeScript/ESLint config this can fail the build or generate a lint error.

_{Reviews (1): Last reviewed commit: "fix(reviews): validate limit/offset/gig_..." | Re-trigger Greptile}

[greptile-apps]

Bounties POST still exposes only the first Zod issue as error

The reviews POST was updated to use a generic "Invalid review payload" string so the issues array is the authoritative source of failures. The bounties POST was only half-updated — it now includes issues in the response body, but the top-level error field still shows issues[0].message. This makes the error contract inconsistent across the two endpoints and means clients who read only error still miss all but the first validation failure.

Suggested change

	{
	error: parsed.error.issues[0].message,
	issues: parsed.error.issues,
	},
	{
	error: "Invalid bounty payload",
	issues: parsed.error.issues,
	},

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!