2

I have a little FTP script which basically transfer an entire directory tree (by walking it with fs.readdir) to an FTP server one file at a time (I have to do some analysis on each file as it's uploaded hence the one-at-a-time behaviour).

However, the bit that does a single file (there's another bit for directories which uses c.mkdir rather than c.put) looks like this:

console.log('Transferring [' + ival + ']');
var c = new Ftp();
c.on('ready', function() {
    c.put(ival, ival, function(err) {
        console.log(err);
    });
    c.end();
});

As you can see, it's using a very simple method of logging in that failures simply get sent to the console.

Unfortunately, since the FTPs are done asynchronously, errors are being delivered to the console in a sequence totally unrelated to the file name output.

Is there a way to force the FTP to be done synchronously so that errors would immediately follow the file name? Basically, I want the entire sequence from the initial console.log to the final }); to be done before moving on to the next file.

paxdiablo
  • 854,327
  • 234
  • 1,573
  • 1,953
  • you don't have to (can you even?) do anything synchronously. You can have asynchronous code that will do exactly what you describe. There's simply not enough of your code for me at least to offer a solution. Perhaps someone with a little more imagination than myself will be able to show you how it's done – Jaromanda X Nov 26 '15 at 09:10

3 Answers3

4

Even if there is, it's not recommended. You generally don't want to block the event loop with such a long synchronous operation.

What would probably be more useful is using recursion or Promises to ensure that things happen in a sequence.

Example:

let ivals = [/* lots of ivals here */];
function putItems(ivals) {
    let ival = ivals[0];
    console.log('Transferring [' + ival + ']');
    var c = new Ftp();
    c.on('ready', function() {
        c.put(ival, ival, function(err) {
            console.log(err);
            c.end();
            // Don't continue if we're out of items.
            if (ivals.length === 1) { return; } 
            putItems(ivals.slice(1)); // Call again with the rest of the items.
        });
    });
}

putItems(ivals);

It can probably be done more intelligently by using a nested function and a single FTP context. But you get the point.

Madara's Ghost
  • 172,118
  • 50
  • 264
  • 308
  • As an exercise, try using `ivals.reduce()` with `new Promise()`, I'm 99% sure the resulting code would come out a lot nicer :) – Madara's Ghost Nov 26 '15 at 09:14
  • Well, now i **wont** post my answer - and the code isn't that much nicer anyway :D - in fact it's one line longer and 2 nested levels deeper :D – Jaromanda X Nov 26 '15 at 09:16
  • @JaromandaX Here's my take: https://gist.github.com/MadaraUchiha/2e4bf78fd57e2a531fa4 – Madara's Ghost Nov 26 '15 at 09:24
  • This answers the OP perfectly, although I'd agree that using callbacks or promises would be better as I'm not sure why the OP must have the logs in order, I'd have thought they just want the `ival` to match up with the log, in which case, fire off as many puts as node will handle rather than queuing. – Matt Styles Nov 26 '15 at 09:25
  • @MattStyles The way I understand it, OP wants "Transferring 1, potential errors with 1, Transferring 2, potential errors with 2". Also, OP did mention he wanted to do things synchronously, which implies he did want them to happen one after the other. The other answer by jfriend00 handles the other case. – Madara's Ghost Nov 26 '15 at 09:27
  • @MadaraUchiha - a minor errors in your `gist` but, yeah, similar to my thinking, except I did it all inside the `reduce` which is why the nesting got out of hand :p – Jaromanda X Nov 26 '15 at 09:28
  • @MadaraUchiha yep, I agree, definitely answers the question as it is posed! – Matt Styles Nov 26 '15 at 09:36
  • That looks rather good. I'm not sure the event loop is going to be an issue, it's not running in a browser. Rather it's a script being run from command line node.js. What I am concerned about here is that the `ivals` array may have about 9000 items in it and that `putItems(ivals.slice(1))` looks like recursion to me. Is that potentially going to be a problem? – paxdiablo Nov 26 '15 at 11:04
  • @paxdiablo: It's exactly recursion, but 9000 items shouldn't pose too big of a problem. – Madara's Ghost Nov 26 '15 at 12:05
  • Okay, thanks, I'll give the answers a shot when I get back to work in the morning. Congrats, by the way, on your ascension :-) – paxdiablo Nov 26 '15 at 12:15
  • Actually, this doesn't appear to work. The callback function is called only on error and that's where you've put the code to recurse. Hence it's only doing the first one. – paxdiablo Nov 27 '15 at 05:48
  • @paxdiablo Wait what? What kind of callback only calls on error? What library are you using? – Madara's Ghost Nov 27 '15 at 07:06
  • Hang on, I'm checking up on that assertion. When I integrated this into my code, it didn't attempt the rest of the array but it's quite possible I integrated it wrongly :-) I finally got it to work by using a close event to continue the process with the sliced array. – paxdiablo Nov 28 '15 at 00:50
3

Without making things synchronous, you can solve your error logging problem by just logging the name with the error. You can just wrap this in a closure so you can keep track of ival that goes with a particular error:

(function(ival) {
    console.log('Transferring [' + ival + ']');
    var c = new Ftp();
    c.on('ready', function() {
        c.put(ival, ival, function(err) {
            console.log('[' + ival + ']', err);
        });
        c.end();
    });
})(ival);
jfriend00
  • 683,504
  • 96
  • 985
  • 979
  • I understand that whole thing other than the first and last line :-) I'm going to have to bone up on JS a bit more. Actually, now that I look closer, it seems to be an anonymous function definition which is then called. So presumably I can just run that within a (synchronous) loop for each `ival` and it will do both logs (if indeed the second one happens, which is only on error) before moving on to the next loop iteration. Not bad. – paxdiablo Nov 26 '15 at 11:10
  • It's called an IIFE (Immediately Invoked Function Expression). See http://stackoverflow.com/questions/33909751/wrapped-function-with-global-variable-on-window-object/33909821#33909821 for an explanation. – jfriend00 Nov 26 '15 at 15:33
  • Just tried this and the error log is *still* coming out later. The `c.put` appears to be async here so it returns before the actual attempt to FTP is made. If I log `DONE` after the `c.end` (and a `c.connect` to kick it off), I see `transferring`, `DONE` and *then* an error. – paxdiablo Nov 27 '15 at 01:28
  • @paxdiablo - YES, the error log comes out later because that's when the error actually occurs. It is asynchronous so it occurs later. My code put the `iVal` name in with the error so you can see exactly which one is causing the error - thus there shouldn't be any issue knowing which one has an error. – jfriend00 Nov 27 '15 at 05:18
-1

Why dont you just push the errors to an array, and when all uploads are done, you will have that array with all those errors in order ?

I will do something like this:

var errArray = [];
console.log('Transferring [' + ival + ']');
var c = new Ftp();
c.on('ready', function() {
  c.put(ival, ival, function(err) {
    errArray.push( err );
  });
  c.end();
});
c.on('end', function() {
  errArray.forEach( function( err ){
    console.log( err );
  })
});
avcajaraville
  • 9,041
  • 2
  • 28
  • 37