feat: implement error handling and logging for database transactions across multiple routes#67
Merged
Merged
Conversation
…e modules - Updated `authHandler.ts`, `preferences.ts`, `specialUseService.ts`, and various route handlers to accept a transaction object (`tx`) for database operations, allowing for better transaction management. - Introduced `DrizzleDB` type to encapsulate both the database instance and transaction objects. - Modified API routes for account management, user preferences, and authentication to utilize transactions, ensuring atomicity in operations. - Enhanced error handling during user creation and updates to manage conflicts more effectively. - Improved overall code consistency and maintainability by centralizing database access patterns.
…across multiple routes
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces transaction-scoped DB access across multiple v1 API routes, adding structured error handling and logging while enabling shared helpers to operate on either a live Drizzle DB instance or an active transaction object.
Changes:
- Added shared Drizzle typing (
DrizzleDatabase,DrizzleTx,DrizzleDB) to support passing transactions through helper layers. - Wrapped multi-step writes (create/update/delete flows) in Drizzle transactions across auth, account, API key, and mail-account routes, with route-level error logging.
- Updated shared helpers (auth, preferences, special-use) to accept an optional
txparameter so they can participate in caller-owned transactions.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/db/utils.ts | Introduces DB/transaction type aliases for passing a transaction object through services/routes. |
| src/db/index.ts | Wraps initial admin bootstrap in a transaction and tightens DB instance typing/casts. |
| src/api/versions/v1/routes/mail-accounts/index.ts | Makes mail-account create/update/delete atomic via transactions and logs failures. |
| src/api/versions/v1/routes/mail-accounts/identities/index.ts | Wraps identity create/update in transactions and logs failures. |
| src/api/versions/v1/routes/auth/reset-password/index.ts | Wraps reset consume/request DB mutations in transactions and adds error logging. |
| src/api/versions/v1/routes/auth/index.ts | Wraps session creation in a transaction and adds error logging for failures. |
| src/api/versions/v1/routes/account/index.ts | Wraps password change and account deletion flows in transactions with logging. |
| src/api/versions/v1/routes/account/apikeys/index.ts | Wraps API key creation in a transaction with failure logging. |
| src/api/utils/services/specialUseService.ts | Allows special-use persistence helpers to run on a provided transaction. |
| src/api/utils/preferences.ts | Allows preferences CRUD helpers to run on a provided transaction. |
| src/api/utils/authHandler.ts | Threads an optional tx through auth/session/api-key helpers for transactional composition. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import { int as integer_mysql } from 'drizzle-orm/mysql-core'; | ||
|
|
||
| import { serial as serial_postgresql } from 'drizzle-orm/pg-core'; | ||
| import type { SQLiteTransaction } from 'drizzle-orm/sqlite-core'; |
Comment on lines
+44
to
+60
| admin_user_id = await this.db.transaction(async (tx) => { | ||
| const admin_user_id = await tx.insert(DB.Tables.users).values({ | ||
| username, | ||
| email: "admin@delivr.local", | ||
| password_hash: await Bun.password.hash(crypto_randomBytes(32).toString('hex')), | ||
| display_name: "Default Administrator", | ||
| role: "admin" | ||
| }).returning().get().id; | ||
|
|
||
| await tx.insert(DB.Tables.passwordResets).values({ | ||
| token: crypto_createHash('sha256').update(passwordResetToken).digest('hex'), | ||
| user_id: admin_user_id, | ||
| expires_at: Date.now() + 7 * 24 * 60 * 60 * 1000 // 7 Days | ||
| }); | ||
|
|
||
| return admin_user_id; | ||
| }); |
Member
Author
|
@copilot is this pr safe to merge |
Contributor
Yes — based on the current checks, this PR is safe to merge. The CI run for commit |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.