Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve RPC message encoding #44

Closed
wants to merge 1 commit into from
Closed

Improve RPC message encoding #44

wants to merge 1 commit into from

Conversation

tortmayr
Copy link

What it does

  • Refactor RPCMessageEncoder to enable detection of circular object structures. Circular structures can not be serialized. The detection enables us to fail early instead of indirectly failing once the maximum stack frame limit is reached.
  • Add a custom encoder for functions. This ensures that direct function entries in arrays can be serialized.
  • Add custom encoder for common collection types (maps, sets). Enables direct use of collection types as rpc call arguments (e.g. logging statements)
  • Restore behavior of the old protocol if a log RPC call can not be sent to the backend due to an encoding error
    • Add custom EncodingError type to explicitly catch these errors
    • Refactor loggerFrontendModule to catch encoding errors and continue execution normally. i.e. log call is just logged in frontend console and not sent to backend

Fixes eclipse-theia#11249

Contributed on behalf of STMicroelectronics

How to test

The bug described in eclipse-theia#11249 (comment) should be fixed. i.e.:

  1. Add this log to DebugSessionManager set currentSession:
console.log('SENTINEL FOR SETTING THE CURRENT SESSION', current);
  1. Start a debug session inside the application.
  2. Observe that the frontend (browser) logs log the message as expected.
  3. Observe that no error logged in the frontend console
  4. Observe that the backend logs do not show the message
    ( Same behavior as with the old RPC protocol)

Review checklist

Reminder for reviewers

Copy link

@lucas-koehler lucas-koehler left a comment

Choose a reason for hiding this comment

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

LGTM. the described debugging test also worked for me. I added one minor suggestion to improve grammarof a comment.

packages/core/src/browser/logger-frontend-module.ts Outdated Show resolved Hide resolved
- Refactor `RPCMessageEncoder` to enable detection of circular object structures. Circular structures can not be serialized. The detection enables us to fail early instead of indirectly failing once the maximum stack frame limit is reached.
- Add a custom encoder for functions. This ensures that direct function entries in arrays can be serialized.
- Add custom encoder for common collection types (maps, sets). Enables direct use of collection types as rpc call arguments (e.g. logging statements)
- Restore behavior of the old protocol if a log RPC call can not be sent to the backend due to an encoding error 
   - Add custom `EncodingError` type to explicitly catch these errors
   - Refactor `loggerFrontendModule` to catch encoding errors and continue execution normally. i.e. log call is just logged in frontend console and not sent to backend

Fixes eclipse-theia#11249

Contributed on behalf of STMicroelectronics
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.

Application freeze at startup with many errors in RPC system
2 participants