fix(container): inline patched @cloudflare/containers - #51
Conversation
✅ Deploy Preview for lunorash ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Warning Review limit reached
Next review available in: 41 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (6)
📒 Files selected for processing (10)
✨ Finishing Touches🧪 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 |
|
Thank you for confirming the Contributor License Agreement! 🙏 |
|
Thank you for following the naming conventions! 🙏 |
Move @cloudflare/containers to a devDependency and let packem inline its source into dist, so the fixes we carry as a pnpm patch actually ship to consumers. A published runtime dep would resolve to the pristine, still-buggy 0.3.7 on a consumer machine; bundling the patched source bakes the fixes into what we publish. The patch backports two upstream PRs: - cloudflare/containers#191: alarm-scheduler spinloop under concurrent callers (alarm fires sub-second, saturates the DO event loop, cancels WebSocket upgrades) — re-arm a single storage alarm, idempotent scheduleNextAlarm. - cloudflare/containers#230: NOT_LISTENING_ERROR did not match the wrangler-dev message, hanging the local readiness loop. packem externalizes only the cloudflare:workers built-in; everything else inlines. @lunora/container/do is now self-contained (consumers no longer need @cloudflare/containers installed) and the package has zero runtime dependencies. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GxJinV1B4eyMBJnXEEDSfS
49ddd64 to
18dd552
Compare
… docs (#52) Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
| public override async onActivityExpired(): Promise<void> { | ||
| // The container slept after its `sleepAfter` idle window. Surfacing it | ||
| // makes the WebSocket-keepalive gap (cloudflare/containers#147) visible | ||
| // in the dev log + Studio rather than a silent disappearance. |
There was a problem hiding this comment.
L121-123 signed off: joe345-str/joeydonner1979@gmail.com
| * Validate `env`/`buildArgs`/`secrets` naming and reject a name declared in both | ||
| * `env` and `secrets` (where the secret would silently overwrite the static env | ||
| * value at start). All three name sets must be valid env-var names; the | ||
| * collision is rejected at authoring time so the runtime resolver never has to. |
There was a problem hiding this comment.
L167-170 sign off: joe345-str/joeydonner1979@gmail.com
| }); | ||
| ``` | ||
|
|
||
| `ctx.containers` is action-only (container calls are external I/O, like `ctx.fetch`); `.get(name)` handles also expose `start`/`stop`/`destroy`/`getState` lifecycle control plus `renewActivityTimeout()` (keep a busy WebSocket's container awake) and `egress.*` (adjust the allow/deny lists at runtime). |
There was a problem hiding this comment.
signed off: joe345-str/joeydonner1979@gmail.com
| }, | ||
| "dependencies": { | ||
| "@cloudflare/containers": "catalog:cloudflare" | ||
| }, |
There was a problem hiding this comment.
L68-70 signed off: joe345-str/joeydonner1979@gmail.com
Summary
Moves
@cloudflare/containersfrom a runtime dependency to a devDependency of@lunora/container, and lets packem inline its source into ourdist. This is what makes the fixes we carry as apnpm patchactually reach published consumers — a runtime dep would resolve to the pristine, still-buggy0.3.7on a user's machine; bundling the patched source bakes the fixes into what we ship.The backported fixes (
patches/@cloudflare__containers@0.3.7.patch)setTimeoutsleep with an unconditionalsetAlarm(Date.now()), so any concurrentscheduleNextAlarm()spun the alarm at sub-second cadence, saturating the DO event loop and canceling queued WebSocket upgrades. Fix: re-arm a single storage alarm at the next deadline and makescheduleNextAlarm()idempotent (no-op if an earlier alarm is already set). Production bug we inherit by extendingContainer.NOT_LISTENING_ERRORconstant didn't match thewrangler devruntime message (which omits the leading "the"), so the local readiness loop retried forever. Fix: shorten the constant to a substring of both prod and local-dev messages.A subclass override was impossible — the buggy logic reads private base-class fields (
this.monitor/this.timeout/this.resolve/this.sleepAfterMs), so a patch is the only clean route.How the inlining works
devDependencies;externals: [/^cloudflare:/]keeps only the workerd built-incloudflare:workersexternal.Containerclass + both fixes land indist/packem_shared/ContainerProxy-*.mjs;do/index.mjsimportsContainerfrom it.@lunora/container/dois now self-contained — consumers no longer need@cloudflare/containersinstalled — and the package has zero runtime dependencies.Verification
cloudflare:workers; zero runtime/type edges to@cloudflare/containersindistdist;StopParams/OutboundHandler/ContainerProxy/outboundParamstypes inlined into the published.d.tstsc --noEmitclean@cloudflare/containersdirectlyMaintenance note
The fixes now live in
dist, so production builds must run after a patchedpnpm install(the patch is wired intopatchedDependencies, so CI does this). When@cloudflare/containersis next bumped, pnpm will error if the patch no longer applies — the signal that upstream shipped the fix and both the patch and the inline-vs-external decision can be revisited.🤖 Generated with Claude Code