Skip to content

fix: use .maybeSingle() in reviews route to prevent 500 on not-found#445

Closed
tankgxy wants to merge 1 commit into
profullstack:masterfrom
tankgxy:fix/reviews-route-maybe-single
Closed

fix: use .maybeSingle() in reviews route to prevent 500 on not-found#445
tankgxy wants to merge 1 commit into
profullstack:masterfrom
tankgxy:fix/reviews-route-maybe-single

Conversation

@tankgxy

@tankgxy tankgxy commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

Fixed 6 .single() calls in the reviews route that would throw 500 errors when no rows were found. Changed all to .maybeSingle() so the null-check logic that follows actually works correctly.

@greptile-apps

greptile-apps Bot commented Jun 13, 2026

Copy link
Copy Markdown

Greptile Summary

This PR fixes the five lookup queries in POST /api/reviews that previously called .single() and would throw a 500 when no row was found — the .maybeSingle() change makes the existing null-check guards work as intended. However, the INSERT+SELECT query also had .single() replaced with .maybeSingle(), which introduces a new crash path.

  • Five lookup queries (gig, current-user application, reviewee application, existing-review check, reviewee profile): .maybeSingle() is correct; null-check logic already follows each one.
  • INSERT+SELECT: changing to .maybeSingle() means both createError and review can be null simultaneously (e.g., RLS allows INSERT but blocks SELECT), causing the request to crash with a TypeError on review.id — and all subsequent property accesses — rather than returning a safe error response.

Confidence Score: 3/5

The five lookup fixes are safe and correct, but the INSERT+SELECT change introduces a new crash path that would produce an unhandled TypeError in production whenever the inserted row cannot be read back.

The INSERT+SELECT now silently returns null for review without any guard, and the code immediately accesses review.id, review.reviewer, and review.reviewee in multiple places — all of which throw at runtime. This is a regression on the happy path of review creation, not a theoretical edge case, and should be resolved before merging.

src/app/api/reviews/route.ts — specifically the INSERT+SELECT block around lines 218–260

Important Files Changed

Filename Overview
src/app/api/reviews/route.ts Six .single() calls changed to .maybeSingle(); fix is correct for the five lookup queries, but the INSERT+SELECT case introduces a potential null-reference crash if review is null after a successful insert.

Sequence Diagram

sequenceDiagram
    participant Client
    participant POST /api/reviews
    participant Supabase

    Client->>POST /api/reviews: POST { gig_id, reviewee_id, rating, comment }
    POST /api/reviews->>Supabase: SELECT gig (maybeSingle)
    Supabase-->>POST /api/reviews: gig | null
    alt gig is null
        POST /api/reviews-->>Client: 404 Gig not found
    end
    POST /api/reviews->>Supabase: SELECT application for current user (maybeSingle)
    Supabase-->>POST /api/reviews: application | null
    POST /api/reviews->>Supabase: SELECT reviewee application (maybeSingle)
    Supabase-->>POST /api/reviews: revieweeApplication | null
    alt neither poster nor accepted applicant
        POST /api/reviews-->>Client: 403 Forbidden
    end
    POST /api/reviews->>Supabase: SELECT existing review (maybeSingle)
    Supabase-->>POST /api/reviews: existingReview | null
    alt existingReview found
        POST /api/reviews-->>Client: 400 Already reviewed
    end
    POST /api/reviews->>Supabase: INSERT review + SELECT (maybeSingle)
    Supabase-->>POST /api/reviews: review | null (+ createError | null)
    alt createError
        POST /api/reviews-->>Client: 400 Supabase error
    end
    Note over POST /api/reviews: Missing null check — review.id crashes if review is null
    POST /api/reviews->>Supabase: SELECT reviewee profile (maybeSingle)
    POST /api/reviews-->>Client: 201 Created
Loading

Comments Outside Diff (1)

  1. src/app/api/reviews/route.ts, line 246-249 (link)

    P1 Add a null check on review immediately after the error guard. If createError is null but review is also null (e.g., RLS allows INSERT but blocks SELECT), every subsequent access to review.id, review.reviewer, and review.reviewee will throw a TypeError at runtime.

    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!

Reviews (1): Last reviewed commit: "fix: use .maybeSingle() in reviews route..." | Re-trigger Greptile

Comment on lines 244 to 259
@@ -254,7 +254,7 @@ export async function POST(request: NextRequest) {
.from("profiles")
.select("did")
.eq("id", reviewee_id)
.single();
.maybeSingle();
if (userDid) {
onReviewCreated(userDid, review.id, revieweeProfile?.did || undefined);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Null review after insert causes crash

Switching the INSERT+SELECT from .single() to .maybeSingle() means both data and error can be null when Supabase allows the INSERT but the follow-up SELECT is blocked (e.g., a row-level-security policy that permits writes but not reads). The if (createError) guard on line 246 passes cleanly, and the code then crashes with TypeError: Cannot read properties of null (reading 'id') at every subsequent reference: review.id (lines 259, 265, 270, 275, 280, 284, 294, 297), review.reviewer, and review.reviewee. A missing-row null check needs to be added after the error guard, or .single() should be retained for this INSERT case since a successful insert always returns exactly one row.

@ralyodio ralyodio closed this Jun 14, 2026
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.

2 participants