Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,18 @@ Version 2.0.23

To be released.

### @fedify/cfworkers

- Fixed `WorkersMessageQueue.enqueueMany()` failing when the given messages
exceeded Cloudflare Queues' batch limits of 100 messages or 256 KB per
batch, which could happen when delivering activities to a large audience.
The method now estimates the serialized size of each message and splits
the messages into multiple `sendBatch()` calls that stay within the
limits. [[#958], [#960] by SJang1]

[#958]: https://github.com/fedify-dev/fedify/issues/958
[#960]: https://github.com/fedify-dev/fedify/pull/960


Version 2.0.22
--------------
Expand Down
66 changes: 57 additions & 9 deletions packages/cfworkers/src/mod.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,13 @@ interface WrappedMessage {
readonly __fedify_payload__: any;
}

/**
* enqueueMany Limit settings.
*/
const MAX_BATCH_MESSAGES = 100;
const MAX_ESTIMATED_BATCH_BYTES = 240_000;
const ESTIMATED_METADATA_BYTES_PER_MESSAGE = 128;

/**
* Result from {@link WorkersMessageQueue.processMessage}.
* @since 2.0.0
Expand Down Expand Up @@ -339,16 +346,57 @@ export class WorkersMessageQueue implements MessageQueue {
messages: readonly any[],
options?: MessageQueueEnqueueOptions,
Comment on lines 346 to 347

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Include required bugfix artifacts

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 👍 / 👎.

): Promise<void> {
const requests: MessageSendRequest[] = messages.map((msg) => ({
body: {
const utf8Encoder = new TextEncoder();

const estimateMessageBytes = (body: WrappedMessage): number => {
const serialized = JSON.stringify(body);

if (serialized === undefined) {
throw new TypeError("Queue message must be JSON-serializable.");
}
Comment on lines +352 to +356

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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.

Suggested change
const serialized = JSON.stringify(body);
if (serialized === undefined) {
throw new TypeError("Queue message must be JSON-serializable.");
}
const serialized = JSON.stringify(body);


return utf8Encoder.encode(serialized).byteLength +
ESTIMATED_METADATA_BYTES_PER_MESSAGE;
};

const delaySeconds = options?.delay?.total("seconds") ?? 0;

let batch: MessageSendRequest[] = [];
let estimatedBatchBytes = 0;

const flush = async (): Promise<void> => {
if (batch.length === 0) return;

await this.#queue.sendBatch(batch, { delaySeconds });

batch = [];
estimatedBatchBytes = 0;
};

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,
Comment on lines +376 to +379
} satisfies WrappedMessage;

const request = {
body,
contentType: "json",
} satisfies MessageSendRequest;

const messageBytes = estimateMessageBytes(body);
const exceedsBatchLimit = batch.length >= MAX_BATCH_MESSAGES ||
estimatedBatchBytes + messageBytes > MAX_ESTIMATED_BATCH_BYTES;
Comment on lines +388 to +389

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reject messages over Cloudflare's item limit

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 👍 / 👎.


if (batch.length > 0 && exceedsBatchLimit) {
await flush();
}

batch.push(request);
Comment on lines +391 to +395

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reject oversized Queue items before flushing

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 👍 / 👎.

estimatedBatchBytes += messageBytes;
}

await flush();
}

/**
Expand Down
98 changes: 98 additions & 0 deletions packages/cfworkers/test/mq.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,104 @@ describe("WorkersMessageQueue", () => {
sendBatchSpy.mockRestore();
});

it("enqueueMany() splits batches at the 100-message limit", async () => {
const sendBatchSpy = vi
.spyOn(env.Q1, "sendBatch")
.mockImplementation(async () => {});

const queue = new WorkersMessageQueue(env.Q1);
const messages = Array.from({ length: 101 }, (_, id) => ({ id }));
await queue.enqueueMany(messages);

expect(sendBatchSpy).toHaveBeenCalledTimes(2);
expect(
sendBatchSpy.mock.calls.map(([requests]) =>
Array.from(requests).length
),
).toEqual([100, 1]);

sendBatchSpy.mockRestore();
});

it.each([
["single-byte ASCII", "a", 90_000],
["multi-byte Unicode", "🙂", 22_500],
])(
"enqueueMany() splits %s payloads by serialized UTF-8 size",
async (_label, character, repetitions) => {
const sendBatchSpy = vi
.spyOn(env.Q1, "sendBatch")
.mockImplementation(async () => {});

const queue = new WorkersMessageQueue(env.Q1);
const messages = Array.from({ length: 3 }, (_, id) => ({
id,
content: character.repeat(repetitions),
}));
await queue.enqueueMany(messages);

expect(sendBatchSpy).toHaveBeenCalledTimes(2);
expect(
sendBatchSpy.mock.calls.map(([requests]) =>
Array.from(requests).length
),
).toEqual([2, 1]);

const queuedIds = sendBatchSpy.mock.calls.flatMap(([requests]) =>
Array.from(requests, (request) => {
const body = request.body as {
__fedify_payload__: { id: number };
};
return body.__fedify_payload__.id;
})
);
expect(queuedIds).toEqual([0, 1, 2]);

sendBatchSpy.mockRestore();
},
);

it("enqueueMany() preserves options across split batches", async () => {
const sendBatchSpy = vi
.spyOn(env.Q1, "sendBatch")
.mockImplementation(async () => {});

const queue = new WorkersMessageQueue(env.Q1);
const messages = Array.from({ length: 3 }, (_, id) => ({
id,
content: "a".repeat(90_000),
}));
await queue.enqueueMany(messages, {
orderingKey: "activity:123",
delay: mockDuration(5) as unknown as Temporal.Duration,
});

expect(sendBatchSpy).toHaveBeenCalledTimes(2);
for (const [requests, options] of sendBatchSpy.mock.calls) {
expect(options).toEqual({ delaySeconds: 5 });
for (const request of requests) {
expect(request.body).toMatchObject({
__fedify_ordering_key__: "activity:123",
});
}
}

sendBatchSpy.mockRestore();
});

it("enqueueMany() does not send an empty batch", async () => {
const sendBatchSpy = vi
.spyOn(env.Q1, "sendBatch")
.mockImplementation(async () => {});

const queue = new WorkersMessageQueue(env.Q1);
await queue.enqueueMany([]);

expect(sendBatchSpy).not.toHaveBeenCalled();

sendBatchSpy.mockRestore();
});

it("listen() throws TypeError", () => {
const queue = new WorkersMessageQueue(env.Q1);
expect(() => queue.listen(() => {})).toThrow(TypeError);
Expand Down
Loading