feat: sync drizzle to go/pkg/db/schema.sql #3920
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
📝 WalkthroughWalkthroughUpdates Makefile SQL generation and Docker cleanup, bumps Cloudflare Worker compatibility dates, upgrades drizzle dependencies, adds a generated header to schema.sql, re-exports DrizzleQueryError from the DB package, and updates API route error handling to detect DrizzleQueryError-wrapped duplicate-entry errors. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Client
participant R as API Route (create*)
participant DB as Database
participant ORM as Drizzle ORM
C->>R: POST create (identity/key/permission/role)
R->>ORM: insert(...)
ORM->>DB: SQL INSERT
DB-->>ORM: Error (duplicate key)
note over ORM: Drizzle wraps DB error<br/>as DrizzleQueryError(cause=DatabaseError)
ORM-->>R: throw DrizzleQueryError
alt Duplicate entry detected
R->>R: if e instanceof DrizzleQueryError<br/>and e.cause instanceof DatabaseError<br/>and cause.message includes "Duplicate entry"
R-->>C: 409 CONFLICT (UnkeyApiError)
else Other errors
R-->>C: Propagate/handle as before
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
✨ Finishing Touches
🧪 Generate unit tests
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 |
chronark
requested review from
perkinsjr,
imeyer,
Flo4604,
mcstepp,
MichaelUnkey and
ogzhanolguncu
as code owners
|
Thank you for following the naming conventions for pull request titles! 🙏 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
Makefile(1 hunks)go/pkg/db/schema.sql(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Test Go API Local / Test
- GitHub Check: Test API / API Test Local
- GitHub Check: Build / Build
🔇 Additional comments (2)
go/pkg/db/schema.sql (1)
414-415: No-op change: trailing newline — OKThe extra newline at EOF is fine and conventional. No action needed.
Makefile (1)
39-41: Use dynamic SQL filename resolution in Makefile
Hard-coded “0000_init.sql” can break if Drizzle’s naming changes. Replace the direct copy with runtime detection and fail fast if no file is found:@@ Makefile:39-41 - @cd internal/db && pnpm drizzle-kit generate --schema=src/schema/index.ts --dialect=mysql --out=out --name=init --breakpoints=false - @echo "\n" >> ./internal/db/out/0000_init.sql - @cp ./internal/db/out/0000_init.sql ./go/pkg/db/schema.sql + @cd internal/db && pnpm drizzle-kit generate --schema=src/schema/index.ts --dialect=mysql --out=out --name=init --breakpoints=false + @file="$$(cd internal/db/out && ls -1 *_init.sql 2>/dev/null | head -n1)"; \ + test -n "$$file" || { echo "ERROR: drizzle output not found"; exit 1; }; \ + echo "\n" >> "./internal/db/out/$$file"; \ + cat "./internal/db/out/$$file" >> ./go/pkg/db/schema.sqlCannot autos-verify
make generate-sqlhere (nomakeavailable). Manually confirm the target runs end-to-end, cleansinternal/db/out, and appends the generated SQL (with header) correctly.
|
I am getting the following error: error: unknown option '--breakpoints=false'
make: *** [generate-sql] Error 1Which drizzle kit version do I have to have installed on my system for it to work? |
|
drizzle kit is already defined in the packagejson in the internal/db package |
Mind checking what yours says? |
|
looks like I had a newer version installed |
There was a problem hiding this comment.
Actionable comments posted: 9
♻️ Duplicate comments (3)
Makefile (3)
33-33: Remove redundant|| truewithrm -f
rm -falready suppresses missing-file errors;|| truecan mask real failures.- @rm -f ./go/pkg/db/schema.sql || true + @rm -f ./go/pkg/db/schema.sql
37-37: Clarify Source path in headerMatch the actual CLI input path (src/schema/index.ts).
- @echo "-- Source: internal/db/src/schema" >> ./go/pkg/db/schema.sql + @echo "-- Source: internal/db/src/schema/index.ts" >> ./go/pkg/db/schema.sql
40-41: Avoid modifying generated artifact; append newline to final file with printfDon’t mutate out/0000_init.sql; append a newline to the final schema.sql instead, and use printf for portability.
- @echo "\n" >> ./internal/db/out/0000_init.sql - @cat ./internal/db/out/0000_init.sql >> ./go/pkg/db/schema.sql + @cat ./internal/db/out/0000_init.sql >> ./go/pkg/db/schema.sql + @printf "\n" >> ./go/pkg/db/schema.sql
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (11)
Makefile(1 hunks)apps/api/src/routes/v1_identities_createIdentity.ts(2 hunks)apps/api/src/routes/v1_migrations_createKey.ts(2 hunks)apps/api/src/routes/v1_permissions_createPermission.ts(2 hunks)apps/api/src/routes/v1_permissions_createRole.ts(2 hunks)apps/api/wrangler.custom.toml(1 hunks)apps/api/wrangler.toml(1 hunks)go/pkg/db/schema.sql(2 hunks)internal/db/package.json(1 hunks)internal/db/src/index.ts(1 hunks)internal/encryption/package.json(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
apps/api/src/routes/v1_permissions_createPermission.ts (1)
internal/db/src/index.ts (1)
DrizzleQueryError(20-20)
apps/api/src/routes/v1_identities_createIdentity.ts (1)
internal/db/src/index.ts (1)
DrizzleQueryError(20-20)
apps/api/src/routes/v1_migrations_createKey.ts (2)
internal/db/src/index.ts (1)
DrizzleQueryError(20-20)apps/api/src/pkg/errors/http.ts (1)
UnkeyApiError(117-124)
apps/api/src/routes/v1_permissions_createRole.ts (1)
internal/db/src/index.ts (1)
DrizzleQueryError(20-20)
🔇 Additional comments (7)
apps/api/wrangler.toml (1)
3-3: Validate behavior for new compatibility flagsBumping
compatibility_dateto"2025-09-01"opts your Worker into a host of new runtime behaviors (e.g. global MessageChannel/MessagePort, uppercase HTTP methods, stricter fetch routing, TransformStream backpressure fix, disabled top-level await inrequire(), FinalizationRegistry/WeakRef, etc.). Smoke-test every environment before merging and, where needed, apply the correspondingno_*flags to retain legacy semantics.apps/api/wrangler.custom.toml (1)
3-3: Confirm compatibility_date and nodejs_compat flags
compatibility_date = "2025-09-01" is valid; since this date is on or after 2024-09-23, v2 Node.js compatibility is auto-enabled. Ensure yourcompatibility_flagsincludes"nodejs_compat"or"nodejs_compat_v2". No additional gating required.apps/api/src/routes/v1_permissions_createPermission.ts (1)
10-10: Import looks goodRe-export enables clean import from @unkey/db. No action needed.
apps/api/src/routes/v1_identities_createIdentity.ts (1)
8-8: Import OKUsing the re-export simplifies consumers. Keep it.
apps/api/src/routes/v1_migrations_createKey.ts (1)
10-17: Import addition is fineNo concerns on types or tree-shaking here.
apps/api/src/routes/v1_permissions_createRole.ts (1)
8-8: Good: surface DrizzleQueryError for nested DB errorsImport looks correct and aligns with the new re-export in @unkey/db.
go/pkg/db/schema.sql (1)
417-417: LGTM: trailing newline ensuredEOF newline present; avoids noisy diffs in future regenerations.
| if ( | ||
| e instanceof DrizzleQueryError && | ||
| e.cause instanceof DatabaseError && | ||
| e.cause.message.includes("Duplicate entry") | ||
| ) { | ||
| throw new UnkeyApiError({ | ||
| code: "CONFLICT", | ||
| message: e.body.message, | ||
| message: e.message, | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Don’t surface raw driver message to clients; standardize error text
Leaking e.message may expose internal SQL/driver details and is inconsistent with other routes that return friendly messages. Use a stable message.
Apply:
- throw new UnkeyApiError({
- code: "CONFLICT",
- message: e.message,
- });
+ throw new UnkeyApiError({
+ code: "CONFLICT",
+ message: "Key already exists in this workspace",
+ });Also adopt the shared isDuplicateEntry() helper to align detection across routes.
📝 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 ( | |
| e instanceof DrizzleQueryError && | |
| e.cause instanceof DatabaseError && | |
| e.cause.message.includes("Duplicate entry") | |
| ) { | |
| throw new UnkeyApiError({ | |
| code: "CONFLICT", | |
| message: e.body.message, | |
| message: e.message, | |
| }); | |
| if ( | |
| e instanceof DrizzleQueryError && | |
| e.cause instanceof DatabaseError && | |
| e.cause.message.includes("Duplicate entry") | |
| ) { | |
| throw new UnkeyApiError({ | |
| code: "CONFLICT", | |
| message: "Key already exists in this workspace", | |
| }); | |
| } |
🤖 Prompt for AI Agents
In apps/api/src/routes/v1_migrations_createKey.ts around lines 558-566, the
handler currently checks the raw driver error and re-throws UnkeyApiError using
e.message (which can leak SQL/driver details); replace this by importing and
using the shared isDuplicateEntry() helper to detect duplicate-key errors and
throw UnkeyApiError with a stable client-facing message (e.g. "Key already
exists") and the same "CONFLICT" code; remove reliance on e.message in the
response and keep the original error for internal logging if needed.
| if ( | ||
| e instanceof DrizzleQueryError && | ||
| e.cause instanceof DatabaseError && | ||
| e.cause.message.includes("Duplicate entry") | ||
| ) { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make duplicate-detection more robust (match message or SQL error code)
Matching only on "Duplicate entry" is brittle. Prefer checking DatabaseError code (1062 / ER_DUP_ENTRY) when available, and fall back to message includes. This keeps behavior stable across driver/version changes. (planetscale.com)
Apply:
- if (
- e instanceof DrizzleQueryError &&
- e.cause instanceof DatabaseError &&
- e.cause.message.includes("Duplicate entry")
- ) {
+ if (
+ e instanceof DrizzleQueryError &&
+ e.cause instanceof DatabaseError &&
+ (
+ // Prefer structured code when present
+ // @ts-expect-error Planetscale DatabaseError may expose body with codes
+ e.cause?.body?.code === "1062" ||
+ // Fallback to message substring
+ e.cause.message?.includes("Duplicate entry")
+ )
+ ) {📝 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 ( | |
| e instanceof DrizzleQueryError && | |
| e.cause instanceof DatabaseError && | |
| e.cause.message.includes("Duplicate entry") | |
| ) { | |
| if ( | |
| e instanceof DrizzleQueryError && | |
| e.cause instanceof DatabaseError && | |
| ( | |
| // Prefer structured code when present | |
| // @ts-expect-error Planetscale DatabaseError may expose body with codes | |
| e.cause?.body?.code === "1062" || | |
| // Fallback to message substring | |
| e.cause.message?.includes("Duplicate entry") | |
| ) | |
| ) { |
🤖 Prompt for AI Agents
In apps/api/src/routes/v1_permissions_createRole.ts around lines 91 to 95, the
duplicate-detection currently only checks e.cause.message.includes("Duplicate
entry"); update the conditional to first guard that e is a DrizzleQueryError and
e.cause is a DatabaseError, then detect duplicates by checking the error
code/errno (e.g. e.cause.code === 'ER_DUP_ENTRY' or e.cause.errno === 1062) and
fall back to message.includes("Duplicate entry") if those fields are
unavailable; ensure you access these fields safely (optional chaining) to avoid
runtime errors.
* feat: local first ratelimits * wip * feat: sync drizzle to go/pkg/db/schema.sql (#3920) * feat: sync drizzle to go/pkg/db/schema.sql * fix: upgrade drizzle-kit to use required flag * fix: autogeneration notice --------- Co-authored-by: Flo <[email protected]> * feat: script to notify users to migrate to v2 (#3924) * feat: script to notify users to migrate to v2 * Apply suggestions from code review * [autofix.ci] apply automated fixes * fix: address james' feedback * merge * fix: exclude analytics --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> * fix: prevent duplicates * fix: make reset async * fix: 404 * fix: rename variable for clarity * fix: reset in parallel * chore: revert zod version * fix: I don't even know, this restacking is fucking with my breain * chore: match zod schema --------- Co-authored-by: Flo <[email protected]> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
What does this PR do?
Enhances the
generate-sqlMakefile target to create a more structured SQL schema file. The updated target now:Fixes # (issue)
Type of change
How should this be tested?
make generate-sqland verify that the schema.sql file is generated correctlyChecklist
Required
pnpm buildpnpm fmtconsole.logsgit pull origin mainAppreciated
Summary by CodeRabbit
Bug Fixes
Chores