Skip to content

fix: add error handling for WebSocket message parsing#1025

Open
himanshp1656 wants to merge 2 commits into
qlik-oss:masterfrom
himanshp1656:fix/websocket-message-parse-error-handling
Open

fix: add error handling for WebSocket message parsing#1025
himanshp1656 wants to merge 2 commits into
qlik-oss:masterfrom
himanshp1656:fix/websocket-message-parse-error-handling

Conversation

@himanshp1656
Copy link
Copy Markdown

Here's a full PR description for your contribution:


Summary

This PR adds error handling for WebSocket message parsing in the RPC.onMessage method to gracefully handle malformed JSON responses.

Problem

When calling getFullPropertyTree() on Qlik Sense objects (specifically sheets) that contain Vizlib Advanced Text Objects with HTML content including special characters, the WebSocket response fails to parse with the following error:

SyntaxError: Bad control character in string literal in JSON at position 2771
    at JSON.parse (<anonymous>)
    at RPC.onMessage (/node_modules/enigma.js/enigma.js:1471:25)
    at callListener (/node_modules/ws/lib/event-target.js:290:14)
    at WebSocket.onMessage (/node_modules/ws/lib/event-target.js:209:9)
    at WebSocket.emit (node:events:524:28)
    at Receiver.receiverOnMessage (/node_modules/ws/lib/websocket.js:1220:20)
    at Receiver.emit (node:events:524:28)
    at Receiver.dataMessage (/node_modules/ws/lib/receiver.js:596:14)
    at Receiver.getData (/node_modules/ws/lib/receiver.js:496:10)
    at Receiver.startLoop (/node_modules/ws/lib/receiver.js:167:16)

Node.js v20.19.3

Expected Behavior

The getFullPropertyTree() method should either:

  1. Successfully parse the JSON response and return the object data, or
  2. Throw a proper error that can be caught and handled by application code

Actual Behavior

The JSON parsing error occurs in the WebSocket communication layer and crashes the application. The error is not propagated as a catchable JavaScript error, making it impossible to handle gracefully in application code.

Root Cause

The RPC.onMessage method calls JSON.parse() directly without any error handling:

onMessage(event) {
  const data = JSON.parse(event.data);  // Throws uncatchable error on malformed JSON
  // ...
}

When QIX Engine returns a response containing control characters (e.g., from HTML content in Vizlib objects), JSON.parse() throws a SyntaxError that bubbles up and crashes the application.

Solution

This PR wraps the JSON.parse() call in a try-catch block and emits a socket-error event when parsing fails:

onMessage(event) {
  try {
    const data = JSON.parse(event.data);
    // ... existing logic
  } catch (error) {
    this.emit('socket-error', {
      type: 'parse-error',
      message: 'Failed to parse WebSocket message',
      error,
      rawData: event.data,
    });
  }
}

Benefits

  1. Graceful error handling - Applications can now catch parse errors via the socket-error event
  2. Debugging support - The raw data is included in the error event for debugging
  3. No breaking changes - Existing socket-error event listeners will work seamlessly
  4. Application stability - Malformed messages no longer crash the entire application

Usage Example

session.on('socket-error', (err) => {
  if (err.type === 'parse-error') {
    console.error('Failed to parse WebSocket message:', err.message);
    console.error('Raw data:', err.rawData);
    // Handle gracefully - retry, skip, or notify user
  }
});

Test Plan

  • Verify existing unit tests pass
  • Test with valid JSON WebSocket messages (should work as before)
  • Test with malformed JSON containing control characters
  • Verify socket-error event is emitted with correct payload on parse failure
  • Verify application does not crash on malformed messages

Related Issues

This addresses scenarios where third-party Qlik Sense extensions (like Vizlib) store HTML content with special/control characters in object properties, causing getFullPropertyTree() and similar methods to fail.


Would you like me to adjust anything in this PR description?

Previously, if JSON.parse failed on an incoming WebSocket message,
it would throw an unhandled exception. This change wraps the parsing
in a try-catch block and emits a 'socket-error' event with details
about the parse failure, allowing applications to handle malformed
messages gracefully.

Co-authored-by: Cursor <cursoragent@cursor.com>
@qlikossbuild
Copy link
Copy Markdown

Hi, and thanks for contributing!

Note: If you are part of Qlik:
follow the process outlined on the internal developer portal instead.
Reach out to open-source@qlik.com if you do not have access to that developer portal.


Before we can handle your pull request we need you to sign our contributor license agreement.
This is a one-time process and enables you to contribute without hassle to any of the qlik-oss repositories in the future.

And it only takes a few minutes! Here are some useful links to get you started:

Having troubles? Feel free to reach out to us either here or on Slack!

Note: Once the CLA is sorted, you should rebase/touch this pull request to trigger an update of this CLA status.

Add tests to verify that:
- Malformed JSON emits socket-error event with parse-error type
- No uncaught exception is thrown when receiving invalid JSON

Co-authored-by: Cursor <cursoragent@cursor.com>
@qlikossbuild
Copy link
Copy Markdown

Hi, and thanks for contributing!

Note: If you are part of Qlik:
follow the process outlined on the internal developer portal instead.
Reach out to open-source@qlik.com if you do not have access to that developer portal.


Before we can handle your pull request we need you to sign our contributor license agreement.
This is a one-time process and enables you to contribute without hassle to any of the qlik-oss repositories in the future.

And it only takes a few minutes! Here are some useful links to get you started:

Having troubles? Feel free to reach out to us either here or on Slack!

Note: Once the CLA is sorted, you should rebase/touch this pull request to trigger an update of this CLA status.

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