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

Ftp.get(remotePath, localPath, callback) seems to be calling callback before write to file ends #285

Open
yehudaadler opened this issue Jun 7, 2018 · 0 comments

Comments

@yehudaadler
Copy link

yehudaadler commented Jun 7, 2018

I was trying to run code similar to the code example in the README:

ftp.get("remote/file.txt", "local/file.txt", err => {
  if (hadErr) {
    return console.error("There was an error retrieving the file.");
  }

  const stats = fs.statSync("local/file.txt");
  console.log(stats.size);

  console.log("File copied successfully!");
});

I found that the file size is not always the full file at this point, when the callback is called. Only after a short wait, the full file is on the disk.

Looking at Ftp.get code, it looks like the problem might be that the callback is being called on socket.end and socket.close and actually should only be called once the "finish" event of the fileWriteStream is emitted.

const writeStream = fs.createWriteStream(localPath);
writeStream.on("error", callback);
writeStream.on("finish", callback); // should this be added ???

socket.on("readable", () => {
  this.emitProgress({
    filename: remotePath,
    action: "get",
    socket: socket
  });
});

// This ensures that any expected outcome is handled. There is no
// danger of the callback being executed several times, because it is
// wrapped in `once`.
socket.on("error", callback);
socket.on("end", callback); // should this call the callback ???
socket.on("close", callback); // should this call the callback ???
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant