Improve RPC message encoding

[tortmayr]

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 #11249

Contributed on behalf of STMicroelectronics

How to test

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

1. Add this log to DebugSessionManager set currentSession:

console.log('SENTINEL FOR SETTING THE CURRENT SESSION', current);

2. Start a debug session inside the application.
3. Observe that the frontend (browser) logs log the message as expected.
4. Observe that no error logged in the frontend console
5. Observe that the backend logs do not show the message
   ( Same behavior as with the old RPC protocol)

Review checklist

• As an author, I have thoroughly tested my changes and carefully followed the review guidelines

Reminder for reviewers

• As a reviewer, I agree to behave in accordance with the review guidelines

[colin-grant-work]

I've only glanced over the code, but functionally this appears to be in order. Circular objects (tested on all Widgets that pass through the WidgetManager) are logged to the front end console without error and nothing appears in the backend log output. Logging many logs also appears much snappier than before the change.

[tsmaeder]

I think circular structures can still slip through, see comments.

[tsmaeder]

Don't we have to treat anything that can hold a reference the same as the typeof x === object? I mean I can do

const x= new Set();
const y= new Set();
x.add(y);
y.add(x);

Bingo, circular reference 🤷

[colin-grant-work]

Sets aren't a problem here, but JavaScript is, since if you want to, you can treat almost anything as an object:

const a = new Set();
// undefined
typeof a
// 'object'
const b = 'hello'
// undefined
const c = 'other string'
// undefined
 b.c = c;
// 'other string'
 c.b = b
// 'hello'
typeof b
// 'string'

TypeScript wouldn't allow exactly that syntax, but it would be happy with this:

const d = 'yet another string'
// undefined
const e = 'a fourth string'
// undefined
Object.assign(d, {e});
// [String: 'yet another string'] { e: 'a fourth string' }
typeof d
// 'string'

But as long as we have know where the border is between things whose fields we'll visit (all object types other than null?) and those whose fields we won't (primitives, functions), and we use the reference checker for the former, it should be safe - i.e. we need to make sure that all handlers of object types are delegates of the object handler.

[tortmayr]

Good catch. I have refactored the code and now all object types (other than null/undefined) are going through the reference check. I have also added a test case for circular references based on the example that @tsmaeder provided.

[tortmayr]

Thanks for the review @tsmaeder & @colin-grant-work. I have pushed an update that should address the review comments.

[tsmaeder]

The code looks correct to me now and cursory testing revealed no obvious problems.