Skip to content

fix(rivetkit): remove async drizzle db proxy#4984

Open
abcxff wants to merge 1 commit into
mainfrom
05-06-_slop_fix_rivetkit_remove_async_drizzle_db_proxy
Open

fix(rivetkit): remove async drizzle db proxy#4984
abcxff wants to merge 1 commit into
mainfrom
05-06-_slop_fix_rivetkit_remove_async_drizzle_db_proxy

Conversation

@abcxff
Copy link
Copy Markdown
Contributor

@abcxff abcxff commented May 6, 2026

Description

Please include a summary of the changes and the related issue. Please also include relevant motivation and context.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@railway-app
Copy link
Copy Markdown

railway-app Bot commented May 6, 2026

🚅 Deployed to the rivet-pr-4984 environment in rivet-frontend

Service Status Web Updated (UTC)
kitchen-sink ❌ Build Failed (View Logs) Web May 11, 2026 at 3:42 am
website 😴 Sleeping (View Logs) Web May 7, 2026 at 3:08 am
frontend-inspector ❌ Build Failed (View Logs) Web May 6, 2026 at 10:02 pm
frontend-cloud ❌ Build Failed (View Logs) Web May 6, 2026 at 10:01 pm
ladle ❌ Build Failed (View Logs) Web May 6, 2026 at 4:19 am
mcp-hub ✅ Success (View Logs) Web May 6, 2026 at 4:18 am

Copy link
Copy Markdown
Contributor Author

abcxff commented May 6, 2026

This stack of pull requests is managed by Graphite. Learn more about stacking.

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 6, 2026

PR #4984 Review: remove async drizzle db proxy

Overview

This PR removes an async Proxy-based lazy initializer from ActorContextHandleAdapter.get db and replaces it with a synchronous accessor. The old design wrapped the Drizzle client in a Proxy that called ensureDatabaseClient() asynchronously on every property access. The new design requires the client to be pre-warmed during the onMigrate lifecycle phase (runDatabaseMigrations() then ensureDatabaseClient()) and then reads it synchronously from runtimeState.databaseClient.


What Is Good

  • Removes a subtle footgun. The old Proxy needed a then: undefined guard to prevent Promise.resolve from treating the Drizzle client as a thenable. That guard was easy to miss and caused confusing behavior.
  • No more per-call async overhead. Every Drizzle method call previously awaited ensureDatabaseClient() unconditionally. The client is now resolved once and reused.
  • Follows Fail-By-Default. databaseClientNotReadyError fails loudly with actionable context and a reproduction request instead of silently deferring via lazy init.
  • Correct close/re-warm semantics. closeDatabase() clears both the instance-local #db field and runtimeState.databaseClient, so the onMigrate re-warm on actor wake always starts from a clean state.
  • Good lifecycle test coverage. actor-db-init-order.test.ts exercises ctx.db from createState, onCreate, createVars, onWake, onSleep, and onDestroy.

Issues

Dead code: prepare() method (minor)

prepare() (native.ts:2641) calls ensureDatabaseClient() to pre-warm the database client, but it is never called from anywhere in the codebase. With the new synchronous get db accessor, the only warm-up path is runDatabaseMigrations() via onMigrate. prepare() is now unreachable dead code that contradicts the new contract: the new get db throws if the client is not pre-warmed, but prepare() would still lazily create it if someone called it. It should be removed unless there is a future caller planned.


Suggestions

  • Remove or wire up prepare(). If it is truly unused, delete it so no future caller accidentally bypasses the onMigrate-first invariant.
  • The PR description template is mostly unfilled. A one-line summary of what changed and why would help reviewers and git blame readers.

@abcxff abcxff marked this pull request as ready for review May 6, 2026 15:06
@abcxff abcxff changed the base branch from 05-06-fix_rivetkit-napi_demote_bridge_error_decode_log_from_warn_to_debug to graphite-base/4984 May 6, 2026 21:59
@abcxff abcxff force-pushed the graphite-base/4984 branch from 2bdaa73 to 0499832 Compare May 6, 2026 21:59
@abcxff abcxff force-pushed the 05-06-_slop_fix_rivetkit_remove_async_drizzle_db_proxy branch from 1134c24 to abb6ff1 Compare May 6, 2026 21:59
@abcxff abcxff changed the base branch from graphite-base/4984 to 05-05-fix_rivetkit_use_keepawake_for_websocket_callback_tracking_to_prevent_c.vars_crash_after_grace_deadline May 6, 2026 21:59
@abcxff abcxff changed the base branch from 05-05-fix_rivetkit_use_keepawake_for_websocket_callback_tracking_to_prevent_c.vars_crash_after_grace_deadline to graphite-base/4984 May 7, 2026 01:26
@abcxff abcxff force-pushed the 05-06-_slop_fix_rivetkit_remove_async_drizzle_db_proxy branch from abb6ff1 to 38d756b Compare May 7, 2026 01:27
@abcxff abcxff changed the base branch from graphite-base/4984 to 05-06-fix_rivetkit-napi_demote_bridge_error_decode_log_from_warn_to_debug May 7, 2026 01:27
@abcxff abcxff mentioned this pull request May 7, 2026
11 tasks
@abcxff abcxff changed the base branch from 05-06-fix_rivetkit-napi_demote_bridge_error_decode_log_from_warn_to_debug to graphite-base/4984 May 7, 2026 17:07
@abcxff abcxff force-pushed the 05-06-_slop_fix_rivetkit_remove_async_drizzle_db_proxy branch from 38d756b to d12b4c4 Compare May 7, 2026 17:07
@abcxff abcxff changed the base branch from graphite-base/4984 to main May 7, 2026 17:07
@abcxff abcxff force-pushed the 05-06-_slop_fix_rivetkit_remove_async_drizzle_db_proxy branch from d12b4c4 to c58ef25 Compare May 7, 2026 17:09
@abcxff abcxff requested a review from NathanFlurry May 7, 2026 20:23
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.

1 participant