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

rpc: maximum call stack size exceeded #12173

Closed
vince-fugnitto opened this issue Feb 10, 2023 · 7 comments · Fixed by #12295
Closed

rpc: maximum call stack size exceeded #12173

vince-fugnitto opened this issue Feb 10, 2023 · 7 comments · Fixed by #12295
Assignees
Labels
messaging issues related to messaging

Comments

@vince-fugnitto
Copy link
Member

Bug Description:

I noticed when using the vscode-git and vscode-git-base builtins that we get a maximum call stack exceeded error:

Uncaught (in promise) Error: Error during encoding: 'Maximum call stack size exceeded'
    at MsgPackMessageEncoder.encode (rpc-message-encoder.ts:151)
    at MsgPackMessageEncoder.request (rpc-message-encoder.ts:137)
    at RpcProtocol.sendRequest (rpc-protocol.ts:161)
    at proxy-handler.ts:74

image

The error is thrown when interacting with the inline toolbar item for resources.

Steps to Reproduce:

  1. start the application without @theia/git in either the browser or electron
  2. comment out the following so yarn download:plugins pulls the builtins

    theia/package.json

    Lines 122 to 123 in 371ab02

    "vscode.git",
    "vscode.git-ui",
  3. build, download plugins, and start the application
  4. open a workspace under git version control
  5. make changes
  6. attempt to interact with the scm view with the inline toolbar item for the changes
@tsmaeder
Copy link
Contributor

Maybe something like #12172?

@vince-fugnitto
Copy link
Member Author

Maybe something like #12172?

I don't believe so, I believe it has to do with some circular reference. I tried #12172 and could still reproduce the issue.

@marcdumais-work
Copy link
Contributor

marcdumais-work commented Feb 22, 2023

update: I traced the warnings below in the dev-tools and do not believe they are obviously related to this ticket. Seems more like they are relatively harmless warnings.


Here are vscode.git warning traces, perhaps related to this issue, from the browser console. They are about the extension registering menu items and finding missing sub-menus scm/sourceControl and scm/change/title:

image

@safisa
Copy link
Contributor

safisa commented Feb 28, 2023

I got these errors too...

@vince-fugnitto vince-fugnitto added the messaging issues related to messaging label Feb 28, 2023
@vince-fugnitto
Copy link
Member Author

The issue happens as I've mentioned due to circular reference issues when we attempt to encode, we should do a better job at serializing:

Screen Shot 2023-02-28 at 9 05 52 AM

The ScmPluginResource is referencing itself.

@tsmaeder
Copy link
Contributor

tsmaeder commented Mar 1, 2023

@vince-fugnitto I would argue that the problem is that we're trying to send plugin-provided objects over the main/ext interface, when we really should be sending well-known DTO's instead. Is this what's happening? Handling general recursive structures is out of scope for our rpc framework.

@msujew
Copy link
Member

msujew commented Mar 1, 2023

@tsmaeder The PluginScmResource actually isn't a plugin provided object, but created on the frontend side:

export class PluginScmResource implements ScmResource {
constructor(
private readonly proxy: ScmExt,
private readonly sourceControlHandle: number,
private readonly groupHandle: number,
readonly handle: number,
readonly sourceUri: URI,
readonly group: PluginScmResourceGroup,
readonly decorations: ScmResourceDecorations,
readonly contextValue: string | undefined,
readonly command: ScmCommand | undefined
) { }
open(): Promise<void> {
return this.proxy.$executeResourceCommand(this.sourceControlHandle, this.groupHandle, this.handle);
}
}

But you're right anyway, as we need to send a DTO instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
messaging issues related to messaging
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants