[DO NOT MERGE] Add a simple SFDX: SObject Describe Account Command for testing #111
Conversation
| @@ -22,9 +22,9 @@ export interface CancellationToken { | |||
|
|
|||
| export class CliCommandExecutor { | |||
| private readonly command: Command; | |||
| private readonly options: ExecOptions; | |||
| private readonly options: SpawnOptions; | |||
There was a problem hiding this comment.
This is a legit typing issue that we need to fix. So far it has not affected us since we don't use any of the other options.
| @@ -77,12 +78,27 @@ export class CommandExecution { | |||
| childProcess, | |||
| 'error' | |||
| ) as Observable<Error | undefined>; | |||
| this.processExitSubject.subscribe(next => { | |||
| this.processErrorSubject.subscribe(next => { | |||
There was a problem hiding this comment.
This is another error that should be fixed.
| childProcess.stdout.setEncoding('utf8'); | ||
|
|
||
| childProcess.stdout.on('error', err => { | ||
| console.log(err); |
There was a problem hiding this comment.
Just logging to see if we have any errors on stdout and we don't.
| cwd: vscode.workspace.rootPath, | ||
| stdio: [ | ||
| 'pipe', | ||
| 'pipe', // fs.openSync(path.join(vscode.workspace.rootPath!, 'stdout.txt'), 'w'), |
There was a problem hiding this comment.
A possible, but annoying workaround, is to pipe the result to another stream.
Using a temp file seems to gather all the data.
However, with that, you lose the ability to stream data as it arrives.
|
The CI tests will fail since I'm adding a lot of logging. Like I mentioned, this is not for merging but for others to take out this branch, try it out and have a discussion on the PR. |
|
Confirming that nodejs/node#2360 still happens on Node v7.10.0 on OS X. |
|
The next version of the CLI, aka V6, should explicitly flush the buffer for us. See https://github.com/heroku/cli-engine/blob/master/src/cli.js#L118-L123 Closing this for now, will reopen if the issue reappears in V6. |
|
I just tested this with V6 of the CLI and it still exhibits this truncated issue.
|
What does this PR do?
Adds a simple top-level command that is used to fetch the result of a describe call.
This is meant to illustrate the problem that for large data from the CLI, the results could be truncated. Now, before anyone is alarmed, this only happens under very special circumstances and doesn't affect any of the commands that we have in the released version of our extensions:
child_process.spawnand are using theprocess.stdout.on('data',....)events.This might be related to
where we might need to change the SFDX CLI to call
process.stdout._handle.setBlocking(true)However, having an interactive top-level command that reproduces this issue makes it easier for us to debug this on different platforms.
What issues does this PR fix or reference?
This PR is meant to serve as an easy way for us to debug this and discuss solutions with traceability.