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

Support process.stdout.write from Extension Host to debug output #74173

Closed
octref opened this issue May 23, 2019 · 12 comments
Closed

Support process.stdout.write from Extension Host to debug output #74173

octref opened this issue May 23, 2019 · 12 comments
Assignees
Labels
debt Code quality issues

Comments

@octref
Copy link
Contributor

octref commented May 23, 2019

Root cause of #56211

Repro:

/**
 * @param {vscode.ExtensionContext} context
 */
function activate(context) {
	console.log('console.log');
	process.stdout.write('process.stdout.write\n')
}

image

This is not specific to debug console. process.stdout.write does nothing in extension code.

@octref
Copy link
Contributor Author

octref commented May 23, 2019

I'm on:
Version: 1.35.0-insider
Commit: 4ca38ce
Date: 2019-05-21T05:17:12.031Z
Electron: 3.1.8
Chrome: 66.0.3359.181
Node.js: 10.2.0
V8: 6.6.346.32
OS: Darwin x64 18.5.0

@alexdima
Copy link
Member

alexdima commented Oct 8, 2019

It looks like this was done intentionally a long time ago - https://dev.azure.com/monacotools/_git/Monaco/commit/309d5296ab7f09d50f3e04f391617a26fe3335d2?_a=compare&path=%2Fsrc%2Fbootstrap.js

It currently is here --

process['__defineGetter__']('stdout', function () { return writable; });

I don't know exactly why we did this or if it is now safe to remove...

@bpasero thoughts?

@alexdima alexdima assigned bpasero and alexdima and unassigned alexdima Oct 8, 2019
@alexdima alexdima added the debt Code quality issues label Oct 8, 2019
@bpasero
Copy link
Member

bpasero commented Oct 8, 2019

@alexandrudima yeah I cannot recall for sure, probably some Electron issue which may easily no longer hold true. I think we can try to remove it and see what breaks.

@bpasero bpasero added this to the October 2019 milestone Oct 10, 2019
@bpasero
Copy link
Member

bpasero commented Oct 24, 2019

@joaomoreno hey, do you remember maybe? Looking at EBADF issues on Electron I see some old discussions with you involved: electron/electron#3482 and electron/electron#1471

@joaomoreno
Copy link
Member

joaomoreno commented Oct 24, 2019

long time ago

🤷‍♂️

@bpasero
Copy link
Member

bpasero commented Oct 24, 2019

Maybe we push this rather early in debt week and not now so close before endgame. I also do not want to overload the Electron 6 update with a potential breaking change such as this one.

@bpasero bpasero modified the milestones: October 2019, November 2019 Oct 24, 2019
@bpasero bpasero closed this as completed in a642dae Nov 1, 2019
@bpasero bpasero added the bug Issue identified by VS Code Team member as probable bug label Nov 1, 2019
@alexdima alexdima removed their assignment Nov 4, 2019
@alexdima
Copy link
Member

alexdima commented Nov 4, 2019

Thanks @bpasero !

@roblourens
Copy link
Member

The code change is clearly correct but I'm not sure it makes a difference because as far as I know, the extension host stdout isn't actually going anywhere? When I run process.stdout.write('something\n') in an extension, it doesn't show up in the debug console. And if I use the 'dot' reporter from the original issue, I still don't see the dots, in the debug console or when running in the terminal.

@roblourens roblourens reopened this Dec 4, 2019
@roblourens roblourens added the verification-found Issue verification failed label Dec 4, 2019
@roblourens roblourens assigned bpasero and octref and unassigned bpasero Dec 4, 2019
@roblourens
Copy link
Member

It does go to the devtools for the window running the extension. It would be on vscode (not the DA) to pass that along to the Debug Console. But if we do that, we would get duplicate messages in the Debug Console.

So I wonder whether the fix you really want is to redirect process.stdout.write to the same place console.log is redirected to. cc @isidorn

@bpasero bpasero removed bug Issue identified by VS Code Team member as probable bug verification-found Issue verification failed labels Dec 5, 2019
@bpasero bpasero removed this from the November 2019 milestone Dec 5, 2019
@bpasero bpasero changed the title process.stdout.write in Extension Host does nothing Support process.stdout.write from Extension Host to debug output Dec 5, 2019
@bpasero
Copy link
Member

bpasero commented Dec 5, 2019

Thanks, I do not think this warrants anymore changes for November.

@octref feel free to check out Robs suggestion if you can make it work.

@bpasero
Copy link
Member

bpasero commented Feb 15, 2020

This does not seem possible due to nodejs/node#8033

@bpasero bpasero closed this as completed Feb 15, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Mar 31, 2020
@connor4312
Copy link
Member

I did this in a more primitive way in 538988c

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
debt Code quality issues
Projects
None yet
Development

No branches or pull requests

6 participants