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

ignore 'screen size is bogus' error; fixes microsoft/vscode/issues/100563 (and microsoft/vscode/issues/75932) #5669

Merged
merged 2 commits into from
Jun 23, 2020

Conversation

nukoyluoglu
Copy link
Contributor

@nukoyluoglu nukoyluoglu commented Jun 20, 2020

Corresponding issue: microsoft/vscode#100563
The error below appears when using the Debugger with "type": "cppdbg" and "request": "attach", while picking and attaching to a process through WSL. The root of the error is in the process of fetching remote diagnostics for 'WSL: Ubuntu' since the same error occurs when viewing the Process Explorer, as well. The proposed fix ignores this error silently. The fix imitates a similar commit implemented for the Node - Debug (Legacy) Extension (microsoft/vscode-node-debug@5298920), which also fixes the same issue (microsoft/vscode#75932). The issue was closed, but the fix was only implemented for that particular extension.
'screen size is bogus' error

…itiates similar fix for a different extension microsoft/vscode-node-debug/commit/5298920
@nukoyluoglu nukoyluoglu marked this pull request as ready for review June 20, 2020 09:34
@nukoyluoglu nukoyluoglu changed the title ignore 'screen size is bogus' error; fixes microsoft/vscode#75932 ignore 'screen size is bogus' error; fixes microsoft/vscode/issues/100563 (and microsoft/vscode#75932) Jun 20, 2020
@nukoyluoglu nukoyluoglu changed the title ignore 'screen size is bogus' error; fixes microsoft/vscode/issues/100563 (and microsoft/vscode#75932) ignore 'screen size is bogus' error; fixes microsoft/vscode/issues/100563 (and microsoft/vscode/issues/75932) Jun 20, 2020
@@ -175,6 +175,10 @@ function execChildProcess(process: string, workingDirectory?: string): Promise<s
}

if (stderr && stderr.length > 0) {
if (stderr.indexOf('screen size is bogus') >= 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand how this fixes it. Should there some code in the "if" statement?

Copy link
Contributor

Choose a reason for hiding this comment

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

Or put the "reject" line in an "else".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

putting the reject statement inside the “else” statement was the intention, i believe i must have committed a wrong version. will fix it.

@sean-mcmanus sean-mcmanus requested review from a team and WardenGnaw June 23, 2020 20:57
@sean-mcmanus
Copy link
Contributor

Thanks

@sean-mcmanus sean-mcmanus merged commit 9db05b6 into microsoft:release Jun 23, 2020
@sean-mcmanus
Copy link
Contributor

@nukoyluoglu
Copy link
Contributor Author

nukoyluoglu commented Jun 26, 2020

@sean-mcmanus

Your fix should be available with https://github.com/microsoft/vscode-cpptools/releases/tag/0.29.0-insiders .

I could not find my fix in this version. See: 0.28.3...0.29.0-insiders and https://github.com/microsoft/vscode-cpptools/blob/0.29.0-insiders/Extension/src/Debugger/nativeAttach.ts - where you can see that the fix has not been merged.

Also, currently the fix does not allow process picking. Please request changes to move the "return" statement on line 184 inside the else statement above.

// see similar fix for the Node - Debug (Legacy) Extension at https://github.com/microsoft/vscode-node-debug/commit/5298920
} else {
reject(new Error(stderr));
}
return;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This return statement must be inside the else statement above it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay, thanks. I'll submit a new PR to master for our next release.

Copy link
Contributor

Choose a reason for hiding this comment

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

@sean-mcmanus
Copy link
Contributor

@nukoyluoglu Oops. Sorry about that -- I didn't realize the PR was to our release branch and not master.

sean-mcmanus pushed a commit that referenced this pull request Jun 26, 2020
…0563 (and microsoft/vscode/issues/75932) (#5669)

* ignore 'screen size is bogus' error; fixes microsoft/vscode#75932; imitiates similar fix for a different extension microsoft/vscode-node-debug/commit/5298920
@sean-mcmanus
Copy link
Contributor

This should be available (for real) this time with https://github.com/microsoft/vscode-cpptools/releases/tag/0.29.0-insiders2

@github-actions github-actions bot locked and limited conversation to collaborators Oct 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants