fix: embed capability limit in errors (#99) - #102
Conversation
Generate the capability size error as a quoted literal so the configured limit is fixed in the Dynamic Worker module. Cover the generated source to keep the user-facing value visible.
🦋 Changeset detectedLatest commit: fcb3977 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
commit: |
yunaremaia
left a comment
yunaremaia
left a comment
There was a problem hiding this comment.
Verified this PR locally. Solid change.
What I checked:
- Built the
@cloudflare/dofsand@cloudflare/computer-rpcworkspaces and ranworker-javascript.test.tson this branch: 31/31 tests pass, including the new regression test. - Counterfactual check: I reverted
module-graph.tsto themainversion (keeping only the new test) and the suite still passes — consistent with the analysis in #99 that the literal${maxCapabilityBytes}does not leak at runtime today, because the message line lives inside the outer template literal incapabilitiesModule, so the placeholder gets interpolated regardless of the inner double quotes.
Assessment: This is sound defensive hardening rather than a fix for an active bug: it removes the fragile nested-interpolation pattern and pins the generated message with an explicitly escaped string via JSON.stringify, while the regression test locks the contract (generated module must contain the real limit and never the raw placeholder name). Changeset present, test is cheap (loader-mock based). Nothing blocking.
One minor, non-blocking note: the test asserts on generated module text via the loader mock — if the loader contract ever changes, load.mock.calls[0]?.[0].modules[...] will need updating. A short comment in the test explaining what it pins would help future contributors.
1 participant
Requested by @aron-cf
Closes #99
capabilitiesModule()inpackages/computer/src/backends/worker-javascript/module-graph.ts:239built the request-size error inside generated source. Although nested template interpolation currently produces the configured value, the source still looked like a runtime placeholder and had no regression coverage.This change creates the complete message at build time and inserts it with
JSON.stringify(). The generated Dynamic Worker module now unambiguously contains a quoted literal such as"Workspace capability request exceeds 256 bytes.", while preserving safe JavaScript string escaping.The regression test captures the loader module map for
maxCapabilityBytes: 256and checks that the generated capability module contains the configured value and nomaxCapabilityBytesplaceholder. This follows the earlier investigation in the issue comment, which found that current interpolation works but recommended retaining this regression test.Verified with
npm run check,npm run typecheck --workspace @cloudflare/computer, andnpm test --workspace @cloudflare/computer(all 61 main test files and the four configured Worker suites passed). A local Worker demo using the packed build also returnedWorkspace capability request exceeds 256 bytes.for an oversizednode:fs/promiseswrite.Demo deployment skipped: Dynamic Workers require a paid Workers plan, while the temporary preview account used for demos rejected the Worker Loader binding with Cloudflare API error 10195. The local demo used the packed
@cloudflare/computerbuild from this branch.🤖 generated by the pr-agent — please review carefully