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

Password JSON output is cut off when using CLI with Nodejs #235

Closed
tmwrnr opened this issue Apr 9, 2024 · 4 comments · Fixed by #236
Closed

Password JSON output is cut off when using CLI with Nodejs #235

tmwrnr opened this issue Apr 9, 2024 · 4 comments · Fixed by #236
Labels
bug Something isn't working

Comments

@tmwrnr
Copy link

tmwrnr commented Apr 9, 2024

Describe the bug
Since the new feature "user presence verification with biometrics" (#231) was introduced, you can not fetch the complete output from the CLI via Nodejs, if a lot of passwords are stored.

For getting all passwords (dcli p --output json) with childProcess.execFile() you would have to use the maxBuffer option. Otherwise the printed output of the JSON is cut off after 8190 characters.

With the commit e16b137 everything works, but with the next commit 0507f57 it seems that maxBuffer is ignored.

To Reproduce

Steps to reproduce the behavior:

  1. Create a file with the following content:
const childProcess = require("node:child_process");
const util = require("node:util");

const execFile = util.promisify(childProcess.execFile);

const CLI_PATH = "path-to-cli";
const MAX_BUFFER = 4096 * 1024;

execFile(CLI_PATH, ["p", "--output", "json"], { maxBuffer: MAX_BUFFER }).then(({ stdout, stderr }) => {
  if (stderr) throw new Error(stderr);

  console.log("Length: ", stdout.length);
  console.log("Last characters", stdout.slice(-10));
});
  1. Checkout e16b137, compile CLI and run the Node file
  2. The stdout should be a valid JSON
  3. Checkout 0507f57, compile CLI and run the Node file
  4. The stdout should be a invalid JSON and have a different length

Expected behavior

I would expect, that maxBuffer has an effect and can be used to get to full stdout.

Environment (please complete the following information):

  • OS: macOS 14.4.1
  • Version: v6.2415.0
  • Node Version: v20.10.0

Additional context

The new feature was discussed in raycast/extensions#11326.

@tmwrnr tmwrnr added the bug Something isn't working label Apr 9, 2024
@Mikescops
Copy link
Member

Hello,

Thanks for reporting this issue, I'll spend time on it next week when I'm back.

@Mikescops
Copy link
Member

Hello,

So I have spent quite some times deep diving this issue.

I know what line to blame between the two commits.

program
    .parseAsync()
    .catch((error: Error) => {
        console.error(errorColor(`error: ${error.message}`));
        process.exit(1);
    })
    .finally(() => process.exit(0)); // <-- here

The reason I added this is to make sure the program exit once the command is run, that way any potential thread hanging in the background is closed.
When I made my tests it was working just fine from the terminal but indeed as you reported the execFile is not able to buffer the output.

Took me a while to figure out, but using process.exit() terminates the child program instantly, and it doesn't wait for any I/O to complete, including the stdout. It seems then that the exit is faster that the stdout write+flush

There are a couple of issues mentioning this:

What I've come around is to write an empty buffer to the stdout and use the callback function to exit once it's done, this will force to wait for the previous write+flush to end and then it can exit.

process.stdout.write('', 'utf-8', () => process.exit(0))

I know it feels like a monkey patch but it's the best I could come up with for now.

Mikescops added a commit that referenced this issue Apr 19, 2024
@Mikescops
Copy link
Member

Patched in v6.2416.0

@tmwrnr
Copy link
Author

tmwrnr commented Apr 23, 2024

Hi @Mikescops, thank you very much. With the new version the issue is fixed!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants