fix(ci): start cloudflare e2e wrangler directly#1539
Conversation
Avoid routing the long-running Wrangler dev process through pnpm exec so the Cloudflare Durable Object e2e lane stays stable after the pnpm 11 upgrade.
📝 WalkthroughWalkthroughThis PR refactors the e2e test harness for Cloudflare Durable Objects to start Wrangler via direct node invocation instead of pnpm, expose runtime output for better error diagnostics, and updates all test call sites to use the new ChangesRuntime harness and error handling
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 1
🧹 Nitpick comments (1)
packages/cloudflare-durable-objects-db-sqlite-persistence/e2e/cloudflare-do-runtime-bridge.e2e.test.ts (1)
239-240: ⚡ Quick winWrap JSON parsing with runtime-output-enriched error handling.
If
response.json()throws (empty/non-JSON body), diagnostics lose the wrangler stdout/stderr context added in this PR.Suggested diff
- const parsed = (await response.json()) as WranglerRuntimeResponse<TPayload> - return parsed + try { + const parsed = (await response.json()) as WranglerRuntimeResponse<TPayload> + return parsed + } catch (error) { + const output = runtime.getOutput() + throw createRuntimeError( + `Invalid wrangler dev runtime response body: ${String(error)}`, + output.stderr, + output.stdout, + ) + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/cloudflare-durable-objects-db-sqlite-persistence/e2e/cloudflare-do-runtime-bridge.e2e.test.ts` around lines 239 - 240, The JSON parse can throw and lose the enriched wrangler diagnostics; replace the direct await response.json() used to produce parsed (typed as WranglerRuntimeResponse<TPayload>) with a safe parse: read the raw body via await response.text(), then try JSON.parse that string inside a try/catch and on error throw a new Error that includes the original parse error message plus the raw response body and the wrangler stdout/stderr variables introduced in this PR (e.g. wranglerStdout, wranglerStderr) so test failures retain full runtime output context.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@packages/cloudflare-durable-objects-db-sqlite-persistence/e2e/cloudflare-do-runtime-bridge.e2e.test.ts`:
- Around line 230-234: The error message in the catch block that builds the
runtime error currently says "Timed out waiting for wrangler dev runtime
response" but this catch handles all fetch/failure cases; update the message
passed to createRuntimeError (where runtime.getOutput() and createRuntimeError
are used) to a non-timeout-specific wording such as "Failed to fetch wrangler
dev runtime response" or "Error fetching wrangler dev runtime response", and
include String(error) and output.stderr as before so DNS/connection/reset errors
are not mislabeled.
---
Nitpick comments:
In
`@packages/cloudflare-durable-objects-db-sqlite-persistence/e2e/cloudflare-do-runtime-bridge.e2e.test.ts`:
- Around line 239-240: The JSON parse can throw and lose the enriched wrangler
diagnostics; replace the direct await response.json() used to produce parsed
(typed as WranglerRuntimeResponse<TPayload>) with a safe parse: read the raw
body via await response.text(), then try JSON.parse that string inside a
try/catch and on error throw a new Error that includes the original parse error
message plus the raw response body and the wrangler stdout/stderr variables
introduced in this PR (e.g. wranglerStdout, wranglerStderr) so test failures
retain full runtime output context.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7fb1f666-05c1-4de6-aa72-c814c2703d72
📒 Files selected for processing (1)
packages/cloudflare-durable-objects-db-sqlite-persistence/e2e/cloudflare-do-runtime-bridge.e2e.test.ts
| } catch (error) { | ||
| const output = runtime.getOutput() | ||
| throw createRuntimeError( | ||
| `Timed out waiting for wrangler dev runtime response: ${String(error)}`, | ||
| output.stderr, |
There was a problem hiding this comment.
Use non-timeout-specific wording in the fetch failure path.
At Line 233, this catch handles all fetch errors, not only timeouts. The current text can mislead debugging for DNS/connection/reset failures.
Suggested diff
- `Timed out waiting for wrangler dev runtime response: ${String(error)}`,
+ `Failed waiting for wrangler dev runtime response: ${String(error)}`,📝 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.
| } catch (error) { | |
| const output = runtime.getOutput() | |
| throw createRuntimeError( | |
| `Timed out waiting for wrangler dev runtime response: ${String(error)}`, | |
| output.stderr, | |
| } catch (error) { | |
| const output = runtime.getOutput() | |
| throw createRuntimeError( | |
| `Failed waiting for wrangler dev runtime response: ${String(error)}`, | |
| output.stderr, |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@packages/cloudflare-durable-objects-db-sqlite-persistence/e2e/cloudflare-do-runtime-bridge.e2e.test.ts`
around lines 230 - 234, The error message in the catch block that builds the
runtime error currently says "Timed out waiting for wrangler dev runtime
response" but this catch handles all fetch/failure cases; update the message
passed to createRuntimeError (where runtime.getOutput() and createRuntimeError
are used) to a non-timeout-specific wording such as "Failed to fetch wrangler
dev runtime response" or "Error fetching wrangler dev runtime response", and
include String(error) and output.stderr as before so DNS/connection/reset errors
are not mislabeled.
More templates
@tanstack/angular-db
@tanstack/browser-db-sqlite-persistence
@tanstack/capacitor-db-sqlite-persistence
@tanstack/cloudflare-durable-objects-db-sqlite-persistence
@tanstack/db
@tanstack/db-ivm
@tanstack/db-sqlite-persistence-core
@tanstack/electric-db-collection
@tanstack/electron-db-sqlite-persistence
@tanstack/expo-db-sqlite-persistence
@tanstack/node-db-sqlite-persistence
@tanstack/offline-transactions
@tanstack/powersync-db-collection
@tanstack/query-db-collection
@tanstack/react-db
@tanstack/react-native-db-sqlite-persistence
@tanstack/rxdb-db-collection
@tanstack/solid-db
@tanstack/svelte-db
@tanstack/tauri-db-sqlite-persistence
@tanstack/trailbase-db-collection
@tanstack/vue-db
commit: |
|
Size Change: 0 B Total Size: 114 kB ℹ️ View Unchanged
|
|
Size Change: 0 B Total Size: 4.24 kB ℹ️ View Unchanged
|
Summary
pnpm exec.Test plan
pnpm test:e2e -- --reporter=verboseinpackages/cloudflare-durable-objects-db-sqlite-persistenceMade with Cursor
Summary by CodeRabbit