Repository navigation
Surface package workflow and connector ingress failures - #442
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughPackage workflow export invocations now validate HTTP-2xx responses and extract user-facing error messages from response bodies. Tests cover 500-like and 302-like error scenarios. Wrangler configuration routes ChangesPackage Workflow Export Error Handling and Asset Routing
🎯 2 (Simple) | ⏱️ ~10 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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
🤖 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/worker/src/package-runtime/package-workflows.ts`:
- Around line 1013-1015: The current check in the package export response
handling (the conditional using response.status and the thrown Error created by
getWorkflowInvocationErrorMessage) only rejects 4xx/5xx; change the condition so
any non-2xx response is rejected — i.e., replace the check that uses
response.status >= 400 with one that rejects status codes outside the 200–299
range (for example response.status < 200 || response.status >= 300) so redirects
(3xx) are treated as errors and the same getWorkflowInvocationErrorMessage is
thrown.
🪄 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 Plus
Run ID: ea3fe65c-3927-495a-918e-1e39e3a3b7ba
📒 Files selected for processing (4)
packages/worker/src/package-invocations/service.node.test.tspackages/worker/src/package-invocations/service.tspackages/worker/src/package-runtime/package-workflows.node.test.tspackages/worker/src/package-runtime/package-workflows.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/worker/wrangler.jsonc`:
- Line 102: The test environment's asset routing omits "/connectors/*" from the
run_worker_first list, causing inconsistent worker-first behavior vs
production/preview; update the env.test.assets.run_worker_first array to include
"/connectors/*" (matching the top-level "run_worker_first" and the
production/preview configurations) so connector routes are exercised in
worker-first order during tests; locate and modify the
"env.test.assets.run_worker_first" entry and any other run_worker_first arrays
(e.g., the one referenced near the preview/production blocks) to ensure
"/connectors/*" is present across environments.
🪄 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 Plus
Run ID: 2dc63a4e-9074-46f4-82be-bb71979bb516
📒 Files selected for processing (1)
packages/worker/wrangler.jsonc
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
006a3bc to
e705a2b
Compare
|
🔎 Preview deployed: https://kody-pr-442.kentcdodds.workers.dev Worker: Mocks:
|
Summary
/connectors/*through the Worker before Cloudflare Assets in production, preview, and test so home connector websocket ingress reachesRemoteConnectorSessioninstead of the SPA shell.mainafter the remote-connector workflow support landed there.Verification
npx oxfmt --check packages/worker/wrangler.jsonc packages/worker/src/package-runtime/package-workflows.ts packages/worker/src/package-runtime/package-workflows.node.test.tsnpx vitest run --project node-unit packages/worker/src/package-runtime/package-workflows.node.test.tsnpx vitest run --project workers-unit packages/worker/src/security/public-route-hardening.workers.test.tsnpm run typechecknpm run buildws://127.0.0.1:3742/connectors/home/defaultopens and receives{"type":"server.ping"}, while plain HTTP returns 404.Summary by CodeRabbit
Tests
Improvements