Message byte estimation and batching on @fedify/cfworkers#960
Conversation
Added message byte estimation and batching logic to handle message limits. Assisted-By: codex-5.6-sol
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements batching logic in WorkersMessageQueue.enqueueMany to comply with Cloudflare Queue limits (maximum of 100 messages or 240 KB per batch). It estimates message sizes using JSON.stringify and TextEncoder and flushes batches accordingly. The review feedback points out that the check for serialized === undefined is redundant and can be removed because JSON.stringify on a plain object will never return undefined.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| const serialized = JSON.stringify(body); | ||
|
|
||
| if (serialized === undefined) { | ||
| throw new TypeError("Queue message must be JSON-serializable."); | ||
| } |
There was a problem hiding this comment.
Since body is a plain object literal, JSON.stringify(body) will never return undefined (it would only return undefined if the top-level value itself was undefined, a function, or a symbol). Therefore, the check for serialized === undefined is unreachable dead code and can be safely removed.
| const serialized = JSON.stringify(body); | |
| if (serialized === undefined) { | |
| throw new TypeError("Queue message must be JSON-serializable."); | |
| } | |
| const serialized = JSON.stringify(body); |
There was a problem hiding this comment.
Pull request overview
This PR fixes WorkersMessageQueue.enqueueMany() in @fedify/cfworkers so it no longer fails Cloudflare Queues’ sendBatch() limits by estimating message sizes and splitting large fan-out operations into multiple batches.
Changes:
- Added batch limit constants and a UTF-8 byte-size estimator for JSON-serialized messages.
- Implemented incremental batching with
flush()to issue multiplesendBatch()calls based on message count and estimated total bytes.
| const messageBytes = estimateMessageBytes(body); | ||
| const exceedsBatchLimit = batch.length >= MAX_BATCH_MESSAGES || | ||
| estimatedBatchBytes + messageBytes > MAX_ESTIMATED_BATCH_BYTES; | ||
|
|
||
| if (batch.length > 0 && exceedsBatchLimit) { | ||
| await flush(); | ||
| } |
| for (const message of messages) { | ||
| const body = { | ||
| __fedify_ordering_key__: options?.orderingKey, | ||
| __fedify_payload__: msg, | ||
| } satisfies WrappedMessage, | ||
| contentType: "json", | ||
| })); | ||
| await this.#queue.sendBatch(requests, { | ||
| delaySeconds: options?.delay?.total("seconds") ?? 0, | ||
| }); | ||
| __fedify_payload__: message, |
Codecov Report✅ All modified and coverable lines are covered by tests. 🚀 New features to boost your workflow:
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b7322dc5d2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const exceedsBatchLimit = batch.length >= MAX_BATCH_MESSAGES || | ||
| estimatedBatchBytes + messageBytes > MAX_ESTIMATED_BATCH_BYTES; |
There was a problem hiding this comment.
When one serialized wrapped message is over Cloudflare's 128 KB per-item limit but under this 240 KB batch threshold, exceedsBatchLimit is false for an empty batch, so enqueueMany() still sends an invalid item; if this occurs after a previous size-triggered flush, earlier messages have already been enqueued before the method rejects, so retrying can duplicate deliveries. Cloudflare documents that each sendBatch item is limited to 128 KB (https://developers.cloudflare.com/queues/configuration/javascript-apis/#queue), so these items should be preflighted/rejected before any flush or send.
Useful? React with 👍 / 👎.
| messages: readonly any[], | ||
| options?: MessageQueueEnqueueOptions, |
There was a problem hiding this comment.
This commit is explicitly a bug fix for #958, but the patch only changes the implementation; the root AGENTS.md “Bugfix process” requires both a regression test demonstrating the bug and a CHANGES.md entry. Without tests covering the >100-message and >256 KB split cases, this batching fix can regress silently, and the required release note for the user-facing Cloudflare Queues failure is missing.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5fa3eb9a28
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (batch.length > 0 && exceedsBatchLimit) { | ||
| await flush(); | ||
| } | ||
|
|
||
| batch.push(request); |
There was a problem hiding this comment.
Fresh evidence: this version still only flushes when batch.length > 0 and then unconditionally pushes the current request, so a single wrapped activity over Cloudflare Queues' documented 128 KB per-item limit can be sent when it starts a batch or immediately after a prior flush (https://developers.cloudflare.com/queues/configuration/javascript-apis/). In the after-flush case, earlier batches have already been written before this deterministic rejection, so retrying the whole enqueueMany() call can duplicate those deliveries; preflight each message against the item limit before any flush/send.
Useful? React with 👍 / 👎.
Summary
Fixed
WorkersMessageQueue.enqueueMany()failing when the given messages exceeded Cloudflare Queues' batch limits of 100 messages or 256 KB per batch.Assisted-by: chatgpt codex-5.6-sol
Changes
sendBatch()calls