Conversation
📝 WalkthroughWalkthroughThis pull request migrates the application's data persistence layer from MongoDB to PostgreSQL with Drizzle ORM. It adds Drizzle configuration, database schema definitions, migrations, and replaces MongoDB-based queries in API routes and utility functions with Drizzle-based equivalents. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
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 |
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/app/api/others/send/route.ts (1)
10-10:⚠️ Potential issue | 🟡 Minor
req.json()can throw on malformed request bodies.If the client sends a non-JSON body, this will throw an unhandled error. Consider wrapping in try-catch.
src/app/api/others/stats/route.ts (1)
10-18:⚠️ Potential issue | 🟡 MinorPOST handler lacks runtime validation of
pathand ignoresupdateDataresult.Two issues:
pathcomes directly from user input with no runtime validation. If it's not"api"or"page"(or missing entirely),updateDatasilently falls through tofetchStatsand the handler still returns{ ok: true }.- The return value of
updateDatais discarded, so the caller never knows if the update actually succeeded.Proposed fix
export async function POST(req: NextRequest) { try { const jsonData = await req.json(); const { path } = jsonData; + if (path !== "api" && path !== "page") { + return JsonResponse({ ok: false, error: "Invalid path" }, 400); + } await updateData(path); return JsonResponse({ ok: true }); } catch { return JsonResponse({ ok: false }, 500); } }
🤖 Fix all issues with AI agents
In `@package.json`:
- Around line 38-44: The package.json currently lists "mongoose" in production
dependencies even though the app has migrated to Drizzle ORM; remove "mongoose"
from the regular dependencies and either delete it entirely or move it to
devDependencies if you want to keep the one-off migration script (referenced in
scripts/migrate-data.ts) for future use; update package.json accordingly and run
npm/yarn install to refresh the lockfile so production installs no longer
include mongoose.
In `@scripts/migrate-data.ts`:
- Line 90: The top-level call migrate() can reject before its internal try/catch
(e.g., if mongoose.connect fails); wrap the invocation in a top-level async
wrapper or attach a .catch handler so rejections are handled: call await
migrate() inside an immediately-invoked async function with a surrounding
try/catch (or use migrate().catch(...)) and in the catch log the error (include
exception details) and exit non‑zero; reference the migrate() invocation in
scripts/migrate-data.ts and ensure any process termination happens after
logging.
- Around line 51-58: Remove the stream-of-consciousness comments surrounding the
"Pushing schema to Supabase..." console.log in scripts/migrate-data.ts and
replace them with a single concise comment that states the intended behavior
(e.g., whether the script assumes drizzle-kit push is run beforehand or that the
script will create the table if missing). Specifically, delete the multiple
question-style/brainstorm lines and leave one clear comment describing the
chosen approach so the migration script is readable and production-ready.
- Around line 82-87: The finally block currently calls process.exit(0) and masks
errors; change control flow so failures produce a non-zero exit code: remove or
stop calling process.exit(0) inside the finally block (leave await
mongoose.connection.close()), and instead set process.exitCode = 1 or call
process.exit(1) inside the catch(error) branch after logging the error, and call
process.exit(0) (or set process.exitCode = 0) only on success after the try
completes; reference the catch(error) block, the finally block that awaits
mongoose.connection.close(), and the process.exit calls to locate and update the
logic.
In `@src/app/api/others/stats/route.ts`:
- Around line 23-33: The code uses result[0] from the query into data without
guarding for an empty result set, so when no row with id=1 exists you silently
return a response missing stats fields; update the handler to check the query
result (e.g., inspect result.length or result[0]) after calling
db.select().from(stats) and before spreading into data, and if no row is found
return an appropriate response (e.g., JsonResponse({ ok: false, error: "stats
not found" }, 404) or populate defaults) instead of spreading undefined; ensure
you still remove id (data.id) only when data exists and keep using JsonResponse
for the final reply.
In `@src/db/schema.ts`:
- Around line 4-12: The integer columns on the stats table are currently
nullable (producing number | null) but downstream code (e.g., getStatsData)
expects plain numbers; update the schema by adding .notNull() to each integer
column on the exported stats table (fields: api, page, total, past24, past24api,
past24page) while keeping their .default(0), and then generate/apply a migration
(or alter SQL) to add NOT NULL constraints in the DB so TypeScript types and
runtime DB constraints match.
In `@src/server/sendAPImessage.ts`:
- Around line 6-18: sendAPImessage currently issues an HTTP POST back to your
own app, introducing latency, LOCAL branching and hardcoded URLs; replace the
fetch-based implementation by making a direct server-side call to sendToAuthor
(import sendToAuthor from your server sendMessage module and invoke
sendToAuthor(message, ipOrPlaceholder)) and remove the LOCAL/URL logic and
fetch; also mark the file as a server action (add "use server") and add a type
annotation to the parameter (message: string) so sendAPImessage(message: string)
calls sendToAuthor directly.
- Line 2: The import sendToAuthor from "@/server/sendMessage" in
sendAPImessage.ts is unused; remove the unused import line (or if the intention
was to call sendToAuthor, add the appropriate invocation where messages are
dispatched). Locate the import statement referencing sendToAuthor and either
delete it or implement the missing call to the sendToAuthor function so the
import is actually used.
In `@src/server/sendMessage.ts`:
- Around line 6-14: In sendToAuthor, user content and ip are concatenated into
the URL (variable URL) with only newlines encoded, which allows URL injection
and can produce "undefined..." if process.env.URL is missing; fix by reading and
validating the base URL (process.env.URL) before use (throw or handle when
missing), build the full content string as currently done, then apply
encodeURIComponent to the entire content when appending to URL (i.e., use URL +
encodeURIComponent(content)) so characters like &, #, ?, and spaces are properly
escaped; ensure sendToAuthor returns/handles the case of a missing base URL
rather than concatenating "undefined".
🧹 Nitpick comments (9)
src/server/sendMessage.ts (2)
3-3: VariableURLshadows the globalURLconstructor.Renaming to something like
BASE_URLorSEND_URLavoids shadowing the built-inURLand improves clarity.
16-18: No timeout on the outboundfetch— could hang indefinitely.If the external service is unresponsive, this server action will block without a timeout. Consider using
AbortSignal.timeout()(available in Node 18+).Example
- const response = await fetch(url); + const response = await fetch(url, { signal: AbortSignal.timeout(10_000) });src/app/api/others/send/route.ts (1)
6-6:URLis declared but no longer used after extractingsendToAuthor.This is dead code — the URL is now consumed inside
src/server/sendMessage.ts.Proposed fix
-const URL = process.env.URL; -src/db/drizzle.ts (2)
6-8: Consider using Supabase's connection pooler URL or adding pool size limits.The PR objective mentions migrating to Supabase to avoid MongoDB's fixed connection limits. However, a bare
pg.Poolin a serverless environment (Vercel) can still exhaust Supabase's direct connection limit, since each serverless invocation may spin up a new pool. Consider either:
- Using Supabase's pooler connection string (port 6543) instead of the direct connection, or
- Setting
max: 1on the pool to minimize connection usage per invocation.Example: constrain pool size for serverless
const pool = new Pool({ connectionString: process.env.DATABASE_URL!, + max: 1, });
7-7: Missing validation forDATABASE_URLenvironment variable.The non-null assertion (
!) silently passesundefinedto the Pool constructor if the env var is unset, producing a confusing connection error at runtime. A guard (like inscripts/migrate-data.tsforMONGO_URI) would fail fast with a clear message.Proposed fix
+if (!process.env.DATABASE_URL) { + throw new Error("DATABASE_URL environment variable must be defined"); +} + const pool = new Pool({ - connectionString: process.env.DATABASE_URL!, + connectionString: process.env.DATABASE_URL, });src/db/updateStats.ts (2)
9-12: Avoidanytype — use Drizzle's inferred types for type safety.Using
anyforupdateValuesdefeats the purpose of a typed ORM. Drizzle provides utility types you can use here, or you can use a typed record.Proposed fix
- const updateValues: any = { + const updateValues: Record<string, ReturnType<typeof sql>> = { total: sql`${stats.total} + 1`, past24: sql`${stats.past24} + 1`, };
20-23: Dead code: theelsebranch is unreachable.TypeScript enforces
key: "api" | "page", so this branch can never execute at compile time. However, since the POST handler inroute.tspasses user-controlledpathwithout runtime validation, consider adding runtime validation there instead, or keep this as a defensive guard with a more explicit type narrowing comment.src/app/api/others/stats/route.ts (1)
37-45: Error response returns HTTP 200 withok: false— intentional?Returning a 200 status on error can confuse clients and monitoring tools. If this is intentional for backwards compatibility, consider adding a comment. Otherwise, use an appropriate error status code (e.g., 500).
scripts/migrate-data.ts (1)
62-63: Explicitly setting a serial column's value may desync the sequence.Inserting
id: 1explicitly doesn't advance the underlying PostgreSQL sequence. If a future insert relies on auto-increment, it could conflict with the existing row. Since this app only usesid=1, this is low risk, but for correctness you can reset the sequence after migration:SELECT setval(pg_get_serial_sequence('stats', 'id'), (SELECT MAX(id) FROM stats));
scripts/migrate-data.ts
Outdated
| } catch (error) { | ||
| console.error("Migration failed:", error); | ||
| } finally { | ||
| await mongoose.connection.close(); | ||
| process.exit(0); | ||
| } |
There was a problem hiding this comment.
process.exit(0) in finally masks migration failures.
If the catch block on line 82 executes (migration failed), the finally block still exits with code 0, making it impossible to detect failures in CI or scripts.
Proposed fix
+ let success = false;
try {
await db.insert(stats).values({
// ...
}).onConflictDoUpdate({
// ...
});
console.log("Migration complete!");
+ success = true;
} catch (error) {
console.error("Migration failed:", error);
} finally {
await mongoose.connection.close();
- process.exit(0);
+ process.exit(success ? 0 : 1);
}🤖 Prompt for AI Agents
In `@scripts/migrate-data.ts` around lines 82 - 87, The finally block currently
calls process.exit(0) and masks errors; change control flow so failures produce
a non-zero exit code: remove or stop calling process.exit(0) inside the finally
block (leave await mongoose.connection.close()), and instead set
process.exitCode = 1 or call process.exit(1) inside the catch(error) branch
after logging the error, and call process.exit(0) (or set process.exitCode = 0)
only on success after the try completes; reference the catch(error) block, the
finally block that awaits mongoose.connection.close(), and the process.exit
calls to locate and update the logic.
scripts/migrate-data.ts
Outdated
| } | ||
| } | ||
|
|
||
| migrate(); |
There was a problem hiding this comment.
Unhandled promise rejection if migrate() throws before the internal try/catch.
If an error occurs between the mongoose.connect call and the try block (e.g., connection failure), the promise rejection is unhandled.
Proposed fix
-migrate();
+migrate().catch((err) => {
+ console.error("Migration script failed:", err);
+ process.exit(1);
+});📝 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.
| migrate(); | |
| migrate().catch((err) => { | |
| console.error("Migration script failed:", err); | |
| process.exit(1); | |
| }); |
🤖 Prompt for AI Agents
In `@scripts/migrate-data.ts` at line 90, The top-level call migrate() can reject
before its internal try/catch (e.g., if mongoose.connect fails); wrap the
invocation in a top-level async wrapper or attach a .catch handler so rejections
are handled: call await migrate() inside an immediately-invoked async function
with a surrounding try/catch (or use migrate().catch(...)) and in the catch log
the error (include exception details) and exit non‑zero; reference the migrate()
invocation in scripts/migrate-data.ts and ensure any process termination happens
after logging.
| const result = await db.select().from(stats).where(eq(stats.id, 1)); | ||
| const data: any = { | ||
| ok: true, | ||
| ...stats.toObject(), | ||
| ...result[0], | ||
| href: req.nextUrl.href, | ||
| ip: req.headers.get("x-real-ip") || "127.0.0.1", | ||
| }; | ||
| delete data._id; | ||
|
|
||
| if (data.id) delete data.id; | ||
|
|
||
| return JsonResponse(data); |
There was a problem hiding this comment.
No guard for empty result set when id=1 row doesn't exist.
If the stats table is empty (e.g., migration ran but no seed data), result[0] is undefined. Spreading undefined is a no-op in JS, so the response will be { ok: true, href: ..., ip: ... } with no stats fields — silently wrong rather than failing.
Proposed fix
const result = await db.select().from(stats).where(eq(stats.id, 1));
+ if (!result.length) {
+ const cachedStats = await fetchStats();
+ return JsonResponse({ ok: false, ...cachedStats, href: req.nextUrl.href, ip: req.headers.get("x-real-ip") || "127.0.0.1" });
+ }
const data: any = {
ok: true,
...result[0],🤖 Prompt for AI Agents
In `@src/app/api/others/stats/route.ts` around lines 23 - 33, The code uses
result[0] from the query into data without guarding for an empty result set, so
when no row with id=1 exists you silently return a response missing stats
fields; update the handler to check the query result (e.g., inspect
result.length or result[0]) after calling db.select().from(stats) and before
spreading into data, and if no row is found return an appropriate response
(e.g., JsonResponse({ ok: false, error: "stats not found" }, 404) or populate
defaults) instead of spreading undefined; ensure you still remove id (data.id)
only when data exists and keep using JsonResponse for the final reply.
| @@ -0,0 +1,19 @@ | |||
| "use server"; | |||
| import sendToAuthor from "@/server/sendMessage"; | |||
There was a problem hiding this comment.
Unused import: sendToAuthor is imported but never used.
Proposed fix
"use server";
-import sendToAuthor from "@/server/sendMessage";📝 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.
| import sendToAuthor from "@/server/sendMessage"; | |
| "use server"; |
🤖 Prompt for AI Agents
In `@src/server/sendAPImessage.ts` at line 2, The import sendToAuthor from
"@/server/sendMessage" in sendAPImessage.ts is unused; remove the unused import
line (or if the intention was to call sendToAuthor, add the appropriate
invocation where messages are dispatched). Locate the import statement
referencing sendToAuthor and either delete it or implement the missing call to
the sendToAuthor function so the import is actually used.
Supabase doesn't have the fixed connection limit like mongodb.
Summary by CodeRabbit
Release Notes
New Features
Refactor