fix(api): remove typing busy-wait and harden API handling#1153
fix(api): remove typing busy-wait and harden API handling#1153Shreyas2004wagh wants to merge 2 commits intoRocketChat:developfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates EmbeddedChatApi to eliminate a UI-freezing typing busy-wait and to standardize API error handling by rethrowing caught errors, while also improving message processing and query-string encoding.
Changes:
- Removed the
handleTypingEventbusy-wait/lock to prevent UI unresponsiveness. - Introduced
throwApiErrorand replaced manyconsole.*catch handlers with consistent error propagation. - Replaced deep-clone (
JSON.parse(JSON.stringify(...))) with shallow copies for incoming message objects and updatedchat.searchto useURLSearchParams.
Comments suppressed due to low confidence (1)
packages/api/src/EmbeddedChatApi.ts:1289
- There appears to be an extra trailing blank line at the end of the file after the closing brace. If your lint/formatting rules enforce a single final newline, please remove the extra blank line(s) to avoid formatting-only diffs/noise.
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
packages/api/src/EmbeddedChatApi.ts
Outdated
| } | ||
|
|
||
| private throwApiError(error: unknown): never { | ||
| if (error instanceof Error || error instanceof ApiError) { |
There was a problem hiding this comment.
ApiError extends Error, so error instanceof Error already covers ApiError. The || error instanceof ApiError clause is redundant and can be removed to simplify the helper.
| if (error instanceof Error || error instanceof ApiError) { | |
| if (error instanceof Error) { |
| }); | ||
| return await response.json(); | ||
| } catch (err) { | ||
| return { | ||
| error: err, | ||
| }; | ||
| this.throwApiError(err); | ||
| } |
There was a problem hiding this comment.
pinMessage used to return an { error: ... } object on failure, but now it throws via throwApiError. There are existing call sites that check pinOrUnpin.error (e.g. packages/react/src/views/Message/Message.js) and will now crash or fail to revert optimistic UI state because the promise rejects instead of returning an object. Please either keep the previous return-shape contract for this method (and any similar ones), or update downstream consumers to handle errors via try/catch consistently.
| const message = { ...data }; | ||
| if (message.ts?.$date) { | ||
| console.log(message.ts?.$date); | ||
| message.ts = message.ts.$date; |
There was a problem hiding this comment.
There is a console.log in the message normalization path. This will spam logs in production for every message that has ts.$date and can leak data into client logs. Please remove it or route through the project’s structured/logger abstraction if one exists.
1 participant
fix(api): prevent typing update UI freeze and improve API robustness
Acceptance Criteria fulfillment
handleTypingEvent.throwApiError.JSON.parse(JSON.stringify(data))with shallow object copies in message processing.URLSearchParams.Fixes #1151
PR Test Details
yarn workspace @embeddedchat/api build(passed).typingHandlerLock/ busy-wait logic remains inEmbeddedChatApi.ts.chat.searchnow builds query strings usingURLSearchParams.