fix(cloudflare-workers): do not use i/o operations outside fetch context #4042

jog1t · 2026-01-26T20:47:36Z

Closes RVT-5342

TL;DR

Only auto-prepare the runtime when necessary based on configuration.

What changed?

Added a conditional check before auto-preparing the runtime in the Registry constructor. Now, the runtime is only auto-prepared when either config.serverless?.spawnEngine or config.serveManager is set to a truthy value.

Also reordered the imports to place the Runtime import with the other imports from external files.

How to test?

Create a Registry instance with serverless.spawnEngine set to false and serveManager set to false
Verify that the runtime is not automatically prepared
Create a Registry instance with either serverless.spawnEngine or serveManager set to true
Verify that the runtime is automatically prepared

Why make this change?

This optimization prevents unnecessary runtime preparation when it's not needed based on the configuration. This can improve performance and reduce resource usage in scenarios where the runtime isn't required.

jog1t · 2026-01-26T20:47:52Z

fix(cloudflare-workers): do not use i/o operations outside fetch context #4042 👈 (View in Graphite)
main

How to use the Graphite Merge Queue

Add the label merge-queue to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

_{An organization admin has enabled the Graphite Merge Queue in this repository.} _{Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.}

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

linear · 2026-01-26T20:48:24Z

RVT-5342 # Bug: "Disallowed operation called within global scope" error in v2.0.41

pkg-pr-new · 2026-01-26T20:49:59Z

More templates

@rivetkit/cloudflare-workers

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/cloudflare-workers@4042

@rivetkit/db

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/db@4042

@rivetkit/framework-base

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/framework-base@4042

@rivetkit/next-js

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/next-js@4042

@rivetkit/react

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/react@4042

rivetkit

pnpm add https://pkg.pr.new/rivet-dev/rivet/rivetkit@4042

@rivetkit/sql-loader

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/sql-loader@4042

@rivetkit/virtual-websocket

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/virtual-websocket@4042

@rivetkit/engine-runner

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/engine-runner@4042

@rivetkit/engine-runner-protocol

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/engine-runner-protocol@4042

commit: d987869

claude · 2026-01-26T20:50:19Z

Code Review - PR #4042

Summary

This PR addresses a Cloudflare Workers-specific issue by conditionally preparing the runtime only when necessary. The change prevents I/O operations from running outside of the fetch context, which is prohibited in Cloudflare Workers.

✅ Positive Aspects

Correct Problem Identification: The fix appropriately addresses the Cloudflare Workers constraint where I/O operations cannot occur outside the request/fetch context.
Minimal, Targeted Change: The change is surgical and focused on the specific issue without over-engineering.
Import Organization: The reordering of the Runtime import to group it with other external imports improves code organization.

🔍 Issues & Concerns

1. Logic Error - Missing `serveManager` Check 🔴 Critical

The conditional check on line 51 has a logical flaw:

if (config.serverless?.spawnEngine || config.serveManager) {

Problem: Looking at runtime/index.ts:77-149, the Runtime.create() method shows that runtime preparation involves I/O operations when either:

config.serverless.spawnEngine is true (downloads/spawns engine process)
config.serveManager is true (starts manager server)

However, your condition will auto-prepare even when only serveManager is true. This defeats the purpose of the fix because:

In Cloudflare Workers, serveManager should typically be false since:

Cloudflare Workers uses the CloudflareActorsManagerDriver instead of the built-in manager
The Cloudflare handler (packages/cloudflare-workers/src/handler.ts:73-80) explicitly disables many features and doesn't serve a manager
Auto-preparing when serveManager=true would still trigger I/O operations outside fetch context

Recommendation: The condition should be more precise. Consider:

// Only auto-prepare when we know we need runtime infrastructure upfront
// For serverless platforms like Cloudflare Workers, preparation happens lazily on first request
if (config.serverless?.spawnEngine) {
  // Auto-prepare on next tick (gives time for sync config modification)
  setTimeout(() => {
    // biome-ignore lint/nursery/noFloatingPromises: fire-and-forget auto-prepare
    this.#ensureRuntime();
  }, 0);
}

Or if you need both conditions:

// Auto-prepare when serving local infrastructure (not for pure serverless like CF Workers)
if (config.serverless?.spawnEngine || (config.serveManager && \!config.serverless?.configureRunnerPool)) {

2. Incomplete Testing Guidance 🟡 Medium

The "How to test?" section in the PR description doesn't address the actual Cloudflare Workers use case:

Current test plan:

Create a Registry instance with serverless.spawnEngine set to false and serveManager set to false

Missing:

How to test this specifically fixes the Cloudflare Workers fetch context issue?
What error was occurring before? (Should document the original error message)
A test case that actually deploys to Cloudflare Workers or uses their local dev environment

Recommendation: Add specific Cloudflare Workers test steps:

### Cloudflare Workers Test:
1. Deploy a Registry to Cloudflare Workers with default config
2. Verify no I/O errors occur during module initialization
3. Verify the first request successfully initializes the runtime
4. Verify subsequent requests use the cached runtime

3. No Test Coverage 🟡 Medium

There are no automated tests added to verify this behavior. Given that this is a platform-specific fix, consider:

Adding a test in packages/cloudflare-workers/tests/ that verifies runtime is not auto-prepared
Adding a test that verifies lazy initialization works correctly on first request

4. Import Reordering 🟢 Low Priority (Style)

While the import reordering is good practice, it's unrelated to the main fix. In projects with strict PR practices, this would typically be a separate commit to keep changes focused. Not a blocker, just a note for future PRs.

5. Potential Edge Case 🟡 Medium

Question: What happens if someone calls registry.startRunner() or registry.handler() before the runtime is prepared?

Looking at the code:

Line 103: startRunner() calls this.#ensureRuntime() ✅ Safe
Line 82-85: handler() calls this.#ensureRuntime() ✅ Safe

This is fine - the lazy initialization pattern is correctly implemented. However, it might be worth adding a comment explaining this pattern for maintainability.

📝 Suggested Improvements

Fix the conditional logic (see Issue [SVC-2555] Set up issue templates #1 above)
Add JSDoc comment explaining the lazy initialization strategy:

/**
 * Registry for Rivet Actors.
 *
 * Runtime initialization is deferred to support serverless platforms that
 * prohibit I/O during module initialization (e.g., Cloudflare Workers).
 * The runtime auto-prepares on platforms that need it (spawnEngine),
 * otherwise initialization happens lazily on first handler/startRunner call.
 */
export class Registry<A extends RegistryActors> {

Add tests for the Cloudflare Workers package verifying this behavior
Document the original error in the commit message or PR for future reference

🎯 Recommendation

Request Changes - The logic in the conditional check needs to be corrected before merging. The current implementation may still cause issues in certain configurations.

Files Changed: 1 file, +8/-6 lines
Risk Level: Low-Medium (single file, but critical path for initialization)
Deployment Impact: Affects all Cloudflare Workers deployments

fix(cloudflare-workers): do not use i/o operations outside fetch context

d987869

jog1t requested a review from NathanFlurry January 26, 2026 20:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(cloudflare-workers): do not use i/o operations outside fetch context #4042

fix(cloudflare-workers): do not use i/o operations outside fetch context #4042

jog1t commented Jan 26, 2026 •

edited

Loading

Uh oh!

jog1t commented Jan 26, 2026

Uh oh!

linear bot commented Jan 26, 2026

Uh oh!

pkg-pr-new bot commented Jan 26, 2026

Uh oh!

claude bot commented Jan 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

fix(cloudflare-workers): do not use i/o operations outside fetch context #4042

Are you sure you want to change the base?

fix(cloudflare-workers): do not use i/o operations outside fetch context #4042

Conversation

jog1t commented Jan 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

TL;DR

What changed?

How to test?

Why make this change?

Uh oh!

jog1t commented Jan 26, 2026

How to use the Graphite Merge Queue

Uh oh!

linear bot commented Jan 26, 2026

Uh oh!

pkg-pr-new bot commented Jan 26, 2026

Uh oh!

claude bot commented Jan 26, 2026

Code Review - PR #4042

Summary

✅ Positive Aspects

🔍 Issues & Concerns

1. Logic Error - Missing serveManager Check 🔴 Critical

2. Incomplete Testing Guidance 🟡 Medium

3. No Test Coverage 🟡 Medium

4. Import Reordering 🟢 Low Priority (Style)

5. Potential Edge Case 🟡 Medium

📝 Suggested Improvements

🎯 Recommendation

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jog1t commented Jan 26, 2026 •

edited

Loading

1. Logic Error - Missing `serveManager` Check 🔴 Critical