Skip to content

Fix batch sync protobuf request body#363

Merged
appflowy merged 1 commit into
mainfrom
fix/collab-batch-sync-protobuf-body
May 22, 2026
Merged

Fix batch sync protobuf request body#363
appflowy merged 1 commit into
mainfrom
fix/collab-batch-sync-protobuf-body

Conversation

@appflowy

@appflowy appflowy commented May 22, 2026

Copy link
Copy Markdown
Contributor

Summary

  • bypass axios' default typed-array transform for batch sync protobuf requests
  • add a regression test that preserves the encoded protobuf view instead of sending the pooled backing buffer

Why

The web batch-sync request was encoded by protobufjs as a small Uint8Array view backed by an 8192-byte buffer. Axios default transform converted the view to .buffer, causing the server to receive an 8192-byte body with zero padding and fail protobuf decode with invalid tag value: 0.

Tests

  • pnpm exec jest src/application/services/js-services/http/tests/collab-api.test.ts --runInBand --no-coverage
  • pnpm run type-check

Summary by Sourcery

Ensure collab full-sync batch requests send the correct protobuf-encoded payload without including pooled buffer padding.

Bug Fixes:

  • Prevent Axios from converting protobuf batch sync Uint8Array views into their full backing buffers, which previously produced oversized request bodies and decode failures.

Tests:

  • Add a regression test verifying that collab full-sync batch requests send the encoded protobuf view rather than the pooled backing buffer and that the custom Axios transform preserves the view.

@sourcery-ai

sourcery-ai Bot commented May 22, 2026

Copy link
Copy Markdown

Reviewer's Guide

Adds an explicit Axios transformRequest for collab batch sync protobuf calls to ensure the original Uint8Array view is sent instead of its backing ArrayBuffer, and introduces a regression test that verifies the request body and config preserve the encoded protobuf view semantics.

Sequence diagram for batch sync protobuf request with custom Axios transformRequest

sequenceDiagram
  participant Client
  participant ProtobufJS
  participant Axios
  participant Server

  Client->>ProtobufJS: encode(collabFullSyncBatchRequest)
  ProtobufJS-->>Client: Uint8Array(encodedView)

  Client->>Axios: collabFullSyncBatch(Uint8Array)
  Axios->>Axios: transformRequest(Uint8Array data) => data
  Axios-->>Server: HTTP POST body(Uint8Array view)

  Server-->>Axios: 200/429/503 (arraybuffer)
  Axios-->>Client: ArrayBuffer response
Loading

File-Level Changes

Change Details Files
Ensure collab batch sync sends the Uint8Array protobuf view instead of the backing ArrayBuffer.
  • Set Axios request config transformRequest to an identity function for Uint8Array data so Axios does not coerce it to .buffer.
  • Document in a code comment that the transform avoids sending protobufjs pooled zero-padded backing buffers.
src/application/services/js-services/http/collab-api.ts
Add regression test verifying protobuf view is preserved in collabFullSyncBatch requests.
  • Mock getAxios and its post method to capture the outgoing request body and config.
  • Encode a minimal CollabBatchSyncResponse using protobufjs to act as a realistic response payload.
  • Call collabFullSyncBatch with small Uint8Array fields and assert that the request body is a typed-array view with byteLength less than its backing buffer.
  • Assert that the configured transformRequest returns the same view instance, confirming the identity transform behavior.
src/application/services/js-services/http/__tests__/collab-api.test.ts

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hey - I've found 1 issue, and left some high level feedback:

  • The test assumption expect(requestBody.byteLength).toBeLessThan(requestBody.buffer.byteLength); is a bit brittle and ties correctness to the current protobufjs pooling behavior; consider instead asserting that Axios receives a view (ArrayBuffer.isView) and that the sent byteLength matches the encoded message length you expect.
  • The transformRequest: [(data: Uint8Array) => data] signature is very narrow compared to Axios’ actual transformRequest type; widening the parameter type (e.g., to any or a union) will make this more robust against future Axios type changes while still returning the Uint8Array unmodified.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The test assumption `expect(requestBody.byteLength).toBeLessThan(requestBody.buffer.byteLength);` is a bit brittle and ties correctness to the current protobufjs pooling behavior; consider instead asserting that Axios receives a view (`ArrayBuffer.isView`) and that the sent `byteLength` matches the encoded message length you expect.
- The `transformRequest: [(data: Uint8Array) => data]` signature is very narrow compared to Axios’ actual `transformRequest` type; widening the parameter type (e.g., to `any` or a union) will make this more robust against future Axios type changes while still returning the `Uint8Array` unmodified.

## Individual Comments

### Comment 1
<location path="src/application/services/js-services/http/__tests__/collab-api.test.ts" line_range="51" />
<code_context>
+    const [, requestBody, config] = post.mock.calls[0];
+
+    expect(ArrayBuffer.isView(requestBody)).toBe(true);
+    expect(requestBody.byteLength).toBeLessThan(requestBody.buffer.byteLength);
+    expect(config.transformRequest[0](requestBody)).toBe(requestBody);
+  });
</code_context>
<issue_to_address>
**issue (testing):** This assertion makes the test brittle because it relies on the encoder always returning a view smaller than its backing buffer.

Depending on `requestBody.byteLength < requestBody.buffer.byteLength` makes this test fragile: a future protobuf encoder change could return a tightly sized `Uint8Array`, breaking the test while the behavior is still correct. The regression is specifically about Axios using `.buffer` (including zero padding), not about the current size relationship. To make this robust, either:

- Create a larger `ArrayBuffer` in the test and a `Uint8Array` view over it, and use that as the request body so the condition is under your control, or
- Remove this assertion and instead assert behavior that directly captures the regression (e.g., comparing what Axios’ default transform would send vs. what your override sends).
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

const [, requestBody, config] = post.mock.calls[0];

expect(ArrayBuffer.isView(requestBody)).toBe(true);
expect(requestBody.byteLength).toBeLessThan(requestBody.buffer.byteLength);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

issue (testing): This assertion makes the test brittle because it relies on the encoder always returning a view smaller than its backing buffer.

Depending on requestBody.byteLength < requestBody.buffer.byteLength makes this test fragile: a future protobuf encoder change could return a tightly sized Uint8Array, breaking the test while the behavior is still correct. The regression is specifically about Axios using .buffer (including zero padding), not about the current size relationship. To make this robust, either:

  • Create a larger ArrayBuffer in the test and a Uint8Array view over it, and use that as the request body so the condition is under your control, or
  • Remove this assertion and instead assert behavior that directly captures the regression (e.g., comparing what Axios’ default transform would send vs. what your override sends).

@appflowy appflowy merged commit aa24850 into main May 22, 2026
13 checks passed
@appflowy appflowy deleted the fix/collab-batch-sync-protobuf-body branch May 22, 2026 12:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant