Node-SMB / marsaud-smb2

SMB2 Client
53 stars 46 forks source link

connect function with callback #8

Closed vdiez closed 6 years ago

vdiez commented 8 years ago

I added a new connect function to the prototype which only takes a callback. That way we can call the client like that before doing any additional transfers:

    self.client.connect(function (err) {
        if (err) {
            self.connection_closed = true;
            console.log("Connection SMB error: " + err);
            return;
        }

        console.log("SMB connection established");
        self.connection_closed = false;
        self.transfer();
    });

I also added new listeners for all the socket events to monitor better the connection. Also removed the private connect and only use requireConnect. I understand this might not be ideal, but I thought this way the code is easier to follow (very subjective though).

marsaud commented 8 years ago

Please, do not add compiled files. npm run build is here to re-compile any time, and is called on prepublish so compiled files are shipped with the npm package.

marsaud commented 8 years ago

I'm not sure I see the point of #connect. What would you do in self.transfer ?

If guess you feel the need of a "simple" SMB client, which is hidden there under a library API. If I'm right, I suggest you try to get the pure SMB client stuff out as a single module (that could become a dependency for this one), but I wouldn't try to pull out the client through the full library API.

But maybe I don't guess your purpose well...

vdiez commented 8 years ago

Just reverted the compiled files commit.

The point is that without these changes I cannot be sure when the connection is set up. And there is no feed back from the smb2 module whether the connection was successful or not. My use case is as follows: users can set up different destinations (ftp, ssh, smb) to forward files automatically from a NAS as files arrive. They can only save a destination once a test has been passed. Please check:

https://github.com/mscdex/node-ftp

https://github.com/mscdex/ssh2

My point is to have a callback on connect similar to those libraries. I have my version in production and working as expected, reporting the error to the user when the destination does not work. Still, I'm open to new ideas if the current version does not suit you.

Cheers

vdiez commented 6 years ago

no point in this PR any more, closing