Feature/clerk id sync #8
Conversation
WalkthroughAdds a new Next.js App Router POST route at Changes
Sequence DiagramsequenceDiagram
actor Clerk
participant Handler as Clerk Webhook Handler
participant Svix as Svix Webhook (verify)
participant DB as Supabase (supabase_admin)
Clerk->>Handler: POST /api/webhooks/clerk (svix headers + body)
rect rgb(235,245,255)
Note over Handler: Validation
Handler->>Handler: Check CLERK_WEBHOOK_SECRET
Handler->>Handler: Ensure svix headers present
end
rect rgb(245,255,235)
Note over Handler,Svix: Signature verification
Handler->>Svix: verify(body, headers)
Svix-->>Handler: valid / invalid
end
alt valid
Handler->>Handler: parse event.type & data
alt user.created or user.updated
Handler->>DB: upsert user (clerk_id, email, names)
DB-->>Handler: success / error
else user.deleted
Handler->>DB: delete where clerk_id
DB-->>Handler: success / warning
end
Handler-->>Clerk: 200 OK (Webhook processed)
else invalid or missing
Handler-->>Clerk: 400 Bad Request / 500 if env missing
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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: 3
🧹 Nitpick comments (2)
app/api/webhooks/clerk/route.ts (2)
44-70: Consider extracting event handlers into separate functions.The user creation/update logic is well-structured and handles errors appropriately. For improved testability and maintainability, consider extracting this into a separate async function (e.g.,
handleUserCreatedOrUpdated).Example refactor:
async function handleUserCreatedOrUpdated(userId: string, eventData: any) { const { email_addresses, first_name, last_name } = eventData; const email = email_addresses[0]?.email_address; if (!userId || !email) { return new NextResponse('Error: Missing clerk id or email', { status: 400 }); } const { error: upsertError } = await supabase_admin.from('Users').upsert( { clerk_id: userId, email: email, first_name: first_name, last_name: last_name, }, { onConflict: 'clerk_id', } ); if (upsertError) { console.error('Supabase upsert error:', upsertError.message); return new NextResponse('Error processing webhook', { status: 500 }); } console.log('User was created or updated in Supabase.'); return null; // Success }Then in the main handler:
if (eventType === 'user.created' || eventType === 'user.updated') { const response = await handleUserCreatedOrUpdated(id, wh_event.data); if (response) return response; }
23-23: Remove self-evident comment.The comment "new svix instance" is self-explanatory from the code itself.
Apply this diff:
- // new svix instance const wh = new Webhook(WEBHOOK_SECRET);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
app/api/webhooks/clerk/route.ts(1 hunks)package.json(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
app/api/webhooks/clerk/route.ts (1)
lib/supabase_admin.ts (1)
supabase_admin(13-15)
🔇 Additional comments (6)
app/api/webhooks/clerk/route.ts (5)
12-19: LGTM: Proper svix header extraction and validation.The header extraction and validation correctly implements the svix webhook verification requirements. Returning 400 for missing headers is appropriate.
21-37: LGTM: Robust signature verification implementation.The webhook signature verification correctly follows svix best practices: reading body as text, verifying with proper headers, and returning 400 for invalid signatures. Error logging aids debugging without blocking the response.
39-42: LGTM: Event data extraction is correct.Extracting the clerk_id and event type from the verified webhook event is appropriate. The logging at this stage only includes the event type, which is good.
6-82: Overall implementation follows webhook best practices.The webhook handler correctly implements:
- Signature verification using svix
- Idempotent database operations (upsert/delete)
- Appropriate HTTP status codes for different error conditions
- Event-specific handling logic
This provides a solid foundation for syncing Clerk user lifecycle events to Supabase.
60-60: Verify the unique constraint on clerk_id in your Supabase account.The upsert operation in
app/api/webhooks/clerk/route.ts(line 52-60) usesonConflict: 'clerk_id', which requires a unique constraint on theclerk_idcolumn. The codebase doesn't version-control Supabase schema definitions—no SQL migrations or schema files are present in the repository. You must verify this constraint exists directly in your Supabase dashboard:
- Navigate to the Users table in Supabase
- Check the
clerk_idcolumn has a unique constraint definedIf the constraint is missing, the upsert will fail at runtime.
package.json (1)
30-30: svix version 1.81.0 is current and secure—no changes needed.The latest version of svix is 1.81.0, which matches what's specified in package.json. GitHub's security vulnerability database shows no known vulnerabilities for the svix package. The caret constraint (^1.81.0) appropriately allows for future patch and minor updates.
app/api/webhooks/clerk/route.ts
Outdated
| return new NextResponse('Error processing webhook', { status: 500 }); | ||
| } | ||
|
|
||
| console.log(`User ${id} was created or updated in Supabase.`); |
There was a problem hiding this comment.
Remove user identifiers from logs to comply with privacy requirements.
Logging clerk_id (user identifier) creates compliance and privacy risks. Per security best practices, avoid logging user identifiers like emails, user IDs, etc., as they can constitute PII under GDPR/CCPA.
Apply this diff to remove identifiers from logs:
- console.log(`User ${id} was created or updated in Supabase.`);
+ console.log('User was created or updated in Supabase.');- console.log(`User ${id} was deleted from Supabase.`);
+ console.log('User was deleted from Supabase.');If you need to track specific user operations for debugging, consider using structured logging with appropriate PII redaction or filtering at the log aggregation layer.
Also applies to: 78-78
🤖 Prompt for AI Agents
In app/api/webhooks/clerk/route.ts around lines 69 and 78, the code logs the
Clerk user identifier (e.g., `id`) which can expose PII; remove the identifier
from the console logs and replace with a generic message or a non-PII debug
token. Update both log statements to avoid printing any user IDs/emails — e.g.,
log "A user was created or updated in Supabase" and "A user was deleted in
Supabase" or use structured logging that omits/redacts PII and emits only
non-identifying context; do not log the `id` value directly.
| if (deleteError) { | ||
| console.warn(`Supabase delete error (user ${id}):`, deleteError.message); | ||
| } |
There was a problem hiding this comment.
Consider the implications of continuing execution after delete errors.
Delete errors are logged as warnings but don't prevent the webhook from returning a 200 success response. While this approach provides idempotency (subsequent deletes won't cause retries), it could mask genuine database issues.
Consider whether you want to:
- Keep current behavior for idempotency (acceptable for deleted user scenarios)
- Return an error response for actual database failures
If keeping current behavior, add a comment explaining the rationale:
if (deleteError) {
- console.warn(`Supabase delete error (user ${id}):`, deleteError.message);
+ // Log as warning rather than returning error to maintain idempotency
+ // (subsequent deletes of already-deleted users should not cause retries)
+ console.warn('Supabase delete error:', deleteError.message);
}📝 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 (deleteError) { | |
| console.warn(`Supabase delete error (user ${id}):`, deleteError.message); | |
| } | |
| if (deleteError) { | |
| // Log as warning rather than returning error to maintain idempotency | |
| // (subsequent deletes of already-deleted users should not cause retries) | |
| console.warn('Supabase delete error:', deleteError.message); | |
| } |
🤖 Prompt for AI Agents
In app/api/webhooks/clerk/route.ts around lines 74-76, the code currently logs
Supabase delete errors as warnings but still returns a 200 success response;
decide and implement one of two fixes: (A) if you want idempotency and to
intentionally suppress retries for deleted users, add a concise comment above
the warn explaining that database delete failures are intentionally non-fatal to
avoid webhook retries for already-deleted users and note when this should be
revisited (e.g., persistent failures), or (B) if you want to surface real DB
problems, change the flow to return an error response when deleteError is truthy
(respond with a 5xx and include error details in the response/log) so callers
know the webhook failed; apply only one of these and ensure the added comment or
changed response is clear and short.
Pull Request: Synced clerk user updates with supabase db
Type of Change
Description
Now all user interactions through clerk(sign up, change information, delete) will also be reflected in our Users table
How to Test / Verification Steps
Screenshots / GIFs (if applicable)
Summary by CodeRabbit