Skip to content

Cherry pick/optimize excalidraw sync code - #179

Open
shuntian wants to merge 3 commits into
mainfrom
cherry-pick/optimize-excalidraw-sync-code
Open

Cherry pick/optimize excalidraw sync code#179
shuntian wants to merge 3 commits into
mainfrom
cherry-pick/optimize-excalidraw-sync-code

Conversation

@shuntian

Copy link
Copy Markdown
Collaborator

No description provided.

@seafile-dev seafile-dev 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.

Verdict: Comment

Blocking:

  • 未发现当前 HEAD 上可独立定位的阻塞性实现缺陷。

Warnings:

  • join-room 参数错误的 ACK 契约尚无回归测试;见行内评论。
    Suggested fix: 为缺少 doc_uuid、缺少 user 与空 params 添加 callback 断言,并验证不会加入房间或修改用户状态。

usersManager.addUser(docUuid, socket.id, userInfo);
socket.on('join-room', async (params, callback) => {
try {
const { doc_uuid: docUuid, user: userInfo } = params || {};

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[Warning] 参数错误 ACK 未覆盖

Why this matters:
实现已把缺少 doc_uuid 或 user 的 join-room 请求转换为 join_room_error,但新增测试只覆盖成功路径和有效参数下的内部异常。参数校验分支一旦被后续重构破坏,客户端会重新落入 ACK 超时与重复重连。

Suggested fix: 增加缺少 doc_uuid、缺少 user 和空 params 三个用例,断言 callback 收到 { success: false, error_type: "join_room_error" },且 socket.join 和 UsersManager 均未调用。

@seafile-dev seafile-dev 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.

Verdict: Comment

Warnings:

  • join-room 仅校验 user 是否 truthy;畸形但 truthy 的 user 会进入房间并返回成功 ACK,违反参数错误处理约定;见行内评论。
    Suggested fix: 校验 user 对象形状及必需用户名字段,失败时不加入房间并返回失败 ACK。

socket.on('join-room', async (params, callback) => {
try {
const { doc_uuid: docUuid, user: userInfo } = params || {};
if (!docUuid || !userInfo) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[Warning] user 对象形状未校验

Why this matters:
user: {} 或任意 truthy 非用户对象会通过 !userInfo 检查,随后加入房间、写入 UsersManager 并 ACK success。缺失 username/_username 的成员会破坏成员展示或去重,也不符合参数错误必须明确 ACK 的协议。

Suggested fix: 校验 userInfo 为对象且含协议约定的用户名字段;失败时不调用 socket.join/UsersManager,返回失败 ACK,并增加 user: {} 回归用例。

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.

2 participants