Skip to content

fix(ws): surface inbound frame errors instead of silently dropping events (fixes #201) - #204

Open
Xuxchloris wants to merge 1 commit into
larksuite:mainfrom
Xuxchloris:fix/ws-receive-error-handling
Open

fix(ws): surface inbound frame errors instead of silently dropping events (fixes #201)#204
Xuxchloris wants to merge 1 commit into
larksuite:mainfrom
Xuxchloris:fix/ws-receive-error-handling

Conversation

@Xuxchloris

Copy link
Copy Markdown

Fixes #201

Problem

Under load, the WSClient silently loses events that Feishu delivered and ACKed at the transport level: the [ws] receive message debug line never appears for the dropped event, no error is logged, no disconnect fires, and the handler is never invoked.

The receive path is an async 'message' listener with no error handling:

wsInstance?.on('message', async (buffer) => {
  const data = protoBuf.decode(buffer);       // throws on malformed frames
  ...
  await this.handleControlData(data);          // JSON.parse of pong payload
  await this.handleEventData(data);            // dataCache.mergeData -> JSON.parse
});

Any exception (frame decode failure, malformed pong, invalid fragment metadata, fragment JSON parse failure) rejects the listener promise — the event is dropped with no log, and the unhandled rejection can crash the process.

mergeData had an additional silent-loss amplifier: new Array(sum) throws a RangeError for invalid sum, and a seq outside [0, sum) creates sparse holes that produce a corrupted merged buffer (so JSON.parse fails and the event is lost).

Changes

  • ws-client/index.ts
    • communicate(): wrap the whole inbound-frame handler in try/catch and log failures (failed to handle inbound frame: ...) instead of dropping them silently or surfacing them as unhandled rejections.
    • handleEventData(): catch fragment-merge failures and log them with message_id / trace_id so operators can reconcile.
    • handleControlData(): guard the pong JSON.parse and log malformed payloads.
  • ws-client/data-cache.ts: validate fragment metadata (sum integer > 0, seq in [0, sum)) before allocating/merging, so malformed metadata throws a clear error instead of corrupting the buffer.
  • ws-client/__tests__/receive-events.ts (new): 8 tests covering decode-failure logging with connection survival, malformed fragment metadata, malformed pong, the happy path (fragmented event merges → dispatches → ACKs), and DataCache.mergeData validation.

Verification

  • npx jest --runInBand ws-client channel — 32 suites, 325 tests passing (including the 8 new ones), 0 failures.
  • Behavior on success is unchanged; the fix only adds error surfacing and guards.

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.

WSClient (long connection) silently drops im.message.receive_v1 events under load — no dispatch, no log, no disconnect

1 participant