Use marks for streaming #35

Merged
merged 5 commits into from Dec 1, 2012

Conversation

2 participants
Contributor

bancek commented Nov 27, 2012

Streaming big files (more than one chunk) currently doesn't work.

When STOR command is sent jsftp is waiting for result but FTP server only sends mark that passive socket is ready.

I added support for passing marks to callbacks. If mark is received and callback function has property acceptsMarks, callback function will be called for mark and actual result. It's up to user to check if response code is mark.

getGetSocket and getPutSocket now call callback after mark is received (no more timeout hacks for getPutSocket). getPutSocket also supports done callback (when file is actually uploaded on server).

@sergi sergi commented on an outdated diff Nov 30, 2012

@@ -370,23 +365,30 @@ Ftp.getPasvPort = function(text) {
var ftpResponse = action[0];
var command = action[1];
var callback = command[1];
- if (callback) {
- if (!ftpResponse) {
- callback(new Error("FTP response not defined"));
- }
- // In FTP every response code above 399 means error in some way.
- // Since the RFC is not respected by many servers, we are going to
- // overgeneralize and consider every value above 399 as an error.
- else if (ftpResponse.code > 399) {
- var err = new Error(ftpResponse.text || "Unknown FTP error.");
- err.code = ftpResponse.code;
- callback(err);
- }
- else {
+ if (Ftp.isMark(ftpResponse.code)) {
+ if (callback.acceptsMarks) {
@sergi

sergi Nov 30, 2012

Owner

callback could not exist, and this would crash the program. Needs to be checked.

@sergi

sergi Nov 30, 2012

Owner

ftpResponse could also be undefined.

@sergi sergi commented on an outdated diff Nov 30, 2012

- // with any errors that STOR gave back.
- // A way to make this more reliable would be to look at the response
- // mark (Should be 150) and only then if it is ok send the socket back.
- setTimeout(function(err, res) {
- callback(hadErr, socket);
- }, 100);
+ var cmdCallback = function(err, res) {
+ if (!called) {
+ called = true;
+ callback(err, socket);
+ } else {
+ if (doneCallback) {
+ if (err) {
+ doneCallback(err)
+ } else {
+ if (!Ftp.isMark(res.code)) {
@sergi

sergi Nov 30, 2012

Owner

Why this check? In what case could it be a mark?

Contributor

bancek commented Nov 30, 2012

I've added null checks for callback and ftpResponse. I don't remember, why I left isMark check for doneCallback but I guess it's unnecessary.

@sergi sergi pushed a commit that referenced this pull request Dec 1, 2012

Sergi Mansilla Merge pull request #35 from bancek/streaming-marks
Use marks for streaming
acf4911

@sergi sergi merged commit acf4911 into sergi:master Dec 1, 2012

1 check passed

default The Travis build passed
Details
Owner

sergi commented Dec 1, 2012

Good stuff, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment