Skip to content

Comments

fix: remove busy-wait loop and add missing await in EmbeddedChatApi#1167

Open
AbhiramVSA wants to merge 1 commit intoRocketChat:developfrom
AbhiramVSA:fix/api-method-correctness
Open

fix: remove busy-wait loop and add missing await in EmbeddedChatApi#1167
AbhiramVSA wants to merge 1 commit intoRocketChat:developfrom
AbhiramVSA:fix/api-method-correctness

Conversation

@AbhiramVSA
Copy link

@AbhiramVSA AbhiramVSA commented Feb 20, 2026

Acceptance Criteria fulfillment

  • Remove synchronous busy-wait (while (typingHandlerLock) {}) and unnecessary lock from
    handleTypingEvent — JS is single-threaded, no mutex needed
  • Add await on fetch() in sendAttachment so the try/catch block catches network errors
    instead of silently bypassing them
  • Add await on response.json() in getUserStatus, userInfo, and userData for consistent
    error handling matching the rest of the file

Fixes # (issue)

Video/Screenshots

Proof that missing await bypasses try/catch:

A minimal script (proof.js) using real fetch() against an unreachable host demonstrates the bug:

=== CURRENT CODE (no await — from EmbeddedChatApi.ts L1068) ===

ERROR ESCAPED past try/catch to caller: fetch failed

The try/catch inside sendAttachment NEVER ran.

=== FIXED CODE (with await added) ===

[INTERNAL try/catch] Caught: fetch failed
Result: error was handled internally

Without await, the try/catch in sendAttachment is completely bypassed — fetch errors become
unhandled rejections. With await, errors are caught as intended.

Busy-wait loop (before):
Line 366 contained while (typingHandlerLock) {} with a suppressed ESLint warning (// eslint-disable-next-line no-empty). This synchronous busy-wait is unnecessary in single-threaded
JavaScript and risks freezing the browser tab if the lock is ever stuck.

PR Test Details

Note: The PR will be ready for live testing at
https://rocketchat.github.io/EmbeddedChat/pulls/pr-1167 after approval. Contributors are
requested to replace 1167 with the actual PR number.

Replace Fixes # (issue) with the actual issue number once you've created it

#1166

  Remove synchronous busy-wait mutex (while loop) from handleTypingEvent
  that is unnecessary in single-threaded JavaScript and risks freezing the
  browser tab. Add missing await on fetch() in sendAttachment so the
  try/catch block properly catches network errors. Add missing await on
  response.json() in getUserStatus, userInfo, and userData for consistent
  error handling.
@CLAassistant
Copy link

CLAassistant commented Feb 20, 2026

CLA assistant check
All committers have signed the CLA.

@AbhiramVSA
Copy link
Author

This addresses #1166 — the changes are scoped to a single file
(packages/api/src/EmbeddedChatApi.ts). Happy to adjust anything based on review feedback.

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