basic-ftp - npm Package Compare versions

Comparing version 2.17.1 to 3.0.0

CHANGELOG.md

 # Changelog
 
+## 3.0.0
+
+This release contains the following breaking changes:
+
+- Changed: `Client` is now single-use only. It can't be used anymore once it closes and a new client has to be instantiated.
+- Changed: All exceptions are now instances of `Error`, not custom error objects. Introduced `FTPError` for errors specific to FTP. (#37)
+
+Non-breaking changes:
+
+- Added: If there is a socket error outside of a task, the following task will receive it. (#43)
+- Changed: Improved feedback if a developer forgets to use `await` or `.then()` for tasks. (#36)
+
+Special thanks to @broofa for feedback and reviews.
+
 ## 2.17.1
@@ -4,0 +18,0 @@

lib/ftp.js

@@ -10,3 +10,3 @@ "use strict";
 const nullObject = require("./nullObject");
-const FTPContext = require("./FtpContext");
+const { FTPContext, FTPError } = require("./FtpContext");
 const FileInfo = require("./FileInfo");
@@ -51,3 +51,5 @@ const StringWriter = require("./StringWriter");
     /**
-     * Close all connections. You may continue using this client instance and reconnect again.
+     * Close the client and all open socket connections.
+     *
+     * The client can’t be used anymore after calling this method, you have to instantiate a new one to continue any work.
      */
@@ -60,2 +62,9 @@ close() {
     /**
+     * @returns {boolean}
+     */
+    get closed() {
+        return this.ftp.closed;
+    }
+
+    /**
      * Connect to an FTP server.
@@ -73,4 +82,7 @@ *
         }, () => this.ftp.log(`Connected to ${describeAddress(this.ftp.socket)}`));
-        return this.ftp.handle(undefined, (res, task) => {
-            if (positiveCompletion(res.code)) {
+        return this.ftp.handle(undefined, (err, res, task) => {
+            if (err) {
+                task.reject(err);
+            }
+            else if (positiveCompletion(res.code)) {
                 task.resolve(res);
@@ -80,3 +92,3 @@ }
                 // Reject all other codes, including 120 "Service ready in nnn minutes".
-                task.reject(res);
+                task.reject(new FTPError(res));
             }
@@ -98,9 +110,11 @@ });
     send(command, ignoreErrorCodes = false) {
-        return this.ftp.handle(command, (res, task) => {
-            const success = res.code >= 200 && res.code < 400;
-            if (success || (res.code && ignoreErrorCodes)) {
+        return this.ftp.handle(command, (err, res, task) => {
+            if (err instanceof FTPError && ignoreErrorCodes) {
                 task.resolve(res);
             }
+            else if (err) {
+                task.reject(err);
+            }
             else {
-                task.reject(res);
+                task.resolve(res);
             }
@@ -134,4 +148,7 @@ });
         this.ftp.log(`Login security: ${describeTLS(this.ftp.socket)}`);
-        return this.ftp.handle("USER " + user, (res, task) => {
-            if (positiveCompletion(res.code)) { // User logged in proceed OR Command superfluous
+        return this.ftp.handle("USER " + user, (err, res, task) => {
+            if (err) {
+                task.reject(err);
+            }
+            else if (positiveCompletion(res.code)) { // User logged in proceed OR Command superfluous
                 task.resolve(res);
@@ -143,3 +160,3 @@ }
             else { // Also report error on 332 (Need account)
-                task.reject(res);
+                task.reject(new FTPError(res));
             }
@@ -481,12 +498,13 @@ });
     constructor(ftp) {
+        /** @type {FTPContext} */
         this.ftp = ftp;
-        this.result = undefined;
+        /** @type {(import("./FtpContext").FTPResponse | undefined)} */
+        this.response = undefined;
+        /** @type {boolean} */
         this.confirmed = false;
     }
 
-    resolve(task, result) {
-        this.result = result;
-        this._tryResolve(task);
-    }
-
+    /**
+     * @param {import("./FtpContext").TaskResolver} task
+     */
     confirm(task) {
@@ -497,11 +515,27 @@ this.confirmed = true;
 
-    reject(task, reason) {
+    /**
+     * @param {import("./FtpContext").TaskResolver} task
+     * @param {Error} err
+     */
+    reject(task, err) {
         this.ftp.dataSocket = undefined;
-        task.reject(reason);
+        task.reject(err);
     }
 
+    /**
+     * @param {import("./FtpContext").TaskResolver} task
+     * @param {import("./FtpContext").FTPResponse} response
+     */
+    resolve(task, response) {
+        this.response = response;
+        this._tryResolve(task);
+    }
+
+    /**
+     * @param {import("./FtpContext").TaskResolver} task
+     */
     _tryResolve(task) {
-        if (this.confirmed && this.result !== undefined) {
+        if (this.confirmed && this.response !== undefined) {
             this.ftp.dataSocket = undefined;
-            task.resolve(this.result);
+            task.resolve(this.response);
         }
@@ -514,2 +548,3 @@ }
     FTPContext,
+    FTPError,
     FileInfo,
@@ -621,4 +656,7 @@ // Expose some utilities for custom extensions:
             catch(err) {
-                if (!err.code) { // Don't log out FTP response error again.
-                    client.ftp.log(err.toString());
+                // Receiving an FTPError means that the last transfer strategy failed and we should
+                // try the next one. Any other exception should stop the evaluation of strategies because
+                // something else went wrong.
+                if (!(err instanceof FTPError)) {
+                    throw err;
                 }
@@ -768,4 +806,9 @@ }
     const command = "STOR " + remoteFilename;
-    return ftp.handle(command, (res, task) => {
-        if (res.code === 150 || res.code === 125) { // Ready to upload
+    return ftp.handle(command, (err, res, task) => {
+        if (err) {
+            ftp.enableControlTimeout(true);
+            progress.updateAndStop();
+            resolver.reject(task, err);
+        }
+        else if (res.code === 150 || res.code === 125) { // Ready to upload
             // If we are using TLS, we have to wait until the dataSocket issued
@@ -775,3 +818,3 @@ // 'secureConnect'. If this hasn't happened yet, getCipher() returns undefined.
             const canUpload = ftp.hasTLS === false || ftp.dataSocket.getCipher() !== undefined;
-            conditionOrEvent(canUpload, ftp.dataSocket, "secureConnect", () => {
+            onConditionOrEvent(canUpload, ftp.dataSocket, "secureConnect", () => {
                 ftp.log(`Uploading to ${describeAddress(ftp.dataSocket)} (${describeTLS(ftp.dataSocket)})`);
@@ -793,9 +836,5 @@ // Let the data socket be in charge of tracking timeouts.
         else if (positiveCompletion(res.code)) { // Transfer complete
-            resolver.resolve(task, res.code);
+            resolver.resolve(task, res);
         }
-        else if (res.code >= 400 || res.error) {
-            ftp.enableControlTimeout(true);
-            progress.updateAndStop();
-            resolver.reject(task, res);
-        }
+        // Ignore any other FTP response
     });
@@ -819,4 +858,9 @@ }
     const resolver = new TransferResolver(ftp);
-    return ftp.handle(command, (res, task) => {
-        if (res.code === 150 || res.code === 125) { // Ready to download
+    return ftp.handle(command, (err, res, task) => {
+        if (err) {
+            ftp.enableControlTimeout(true);
+            progress.updateAndStop();
+            resolver.reject(task, err);
+        }
+        else if (res.code === 150 || res.code === 125) { // Ready to download
             ftp.log(`Downloading from ${describeAddress(ftp.dataSocket)} (${describeTLS(ftp.dataSocket)})`);
@@ -830,3 +874,3 @@ // Let the data connection be in charge of tracking timeouts during transfer.
             // Check if the data socket is not already closed.
-            conditionOrEvent(ftp.dataSocket.destroyed, ftp.dataSocket, "end", () => {
+            onConditionOrEvent(ftp.dataSocket.destroyed, ftp.dataSocket, "end", () => {
                 ftp.enableControlTimeout(true);
@@ -841,9 +885,5 @@ progress.updateAndStop();
         else if (positiveCompletion(res.code)) { // Transfer complete
-            resolver.resolve(task, res.code);
+            resolver.resolve(task, res);
         }
-        else if (res.code >= 400 || res.error) {
-            ftp.enableControlTimeout(true);
-            progress.updateAndStop();
-            resolver.reject(task, res);
-        }
+        // Ignore any other FTP response
     });
@@ -861,3 +901,3 @@ }
  */
-function conditionOrEvent(condition, emitter, eventName, action) {
+function onConditionOrEvent(condition, emitter, eventName, action) {
     if (condition === true) {
@@ -864,0 +904,0 @@ action();

lib/FtpContext.js

@@ -8,11 +8,39 @@ "use strict";
  * @typedef {Object} Task
+ * @property {ResponseHandler} responseHandler - Handles a response for a task.
+ * @property {TaskResolver} resolver - Resolves or rejects a task.
+ * @property {string} stack - Call stack when task is run.
+ */
+
+/**
+ * @typedef {(err: Error | undefined, response: FTPResponse | undefined, task: TaskResolver) => void} ResponseHandler
+ */
+
+/**
+ * @typedef {Object} TaskResolver
  * @property {(...args: any[]) => void} resolve - Resolves the task.
- * @property {(...args: any[]) => void} reject - Rejects the task.
+ * @property {(err: Error) => void} reject - Rejects the task.
  */
 
 /**
- * @typedef {(response: Object, task: Task) => void} ResponseHandler
+ * @typedef {Object} FTPResponse
+ * @property {number} code - FTP response code
+ * @property {string} message - FTP response including code
  */
 
 /**
+ * Describes an FTP server error response including the FTP response code.
+ */
+class FTPError extends Error {
+    /**
+     * @param {FTPResponse} res
+     */
+    constructor(res) {
+        super(res.message);
+        this.name = this.constructor.name;
+        this.code = res.code;
+    }
+}
+exports.FTPError = FTPError;
+
+/**
  * FTPContext holds the control and data sockets of an FTP connection and provides a
@@ -24,3 +52,3 @@ * simplified way to interact with an FTP server, handle responses, errors and timeouts.
  */
-module.exports = class FTPContext {
+exports.FTPContext = class FTPContext {
 
@@ -47,8 +75,2 @@ /**
         /**
-         * Function that handles incoming messages and resolves or rejects a task.
-         * @private
-         * @type {(ResponseHandler | undefined)}
-         */
-        this._handler = undefined;
-        /**
          * A multiline response might be received as multiple chunks.
@@ -60,2 +82,8 @@ * @private
         /**
+         * TODO describe
+         * @private
+         * @type {(Error | undefined)}
+         */
+        this._closingError = undefined;
+        /**
          * The encoding used when reading from and writing to the control socket.
@@ -93,9 +121,28 @@ * @type {string}
     /**
-     * Close the context by resetting its state.
+     * Close the context.
+     *
+     * The context can’t be used anymore after calling this method.
      */
     close() {
-        this._passToHandler({ error: { info: "User closed client during task." }});
-        this._reset();
+        // If this context already has been closed, don't overwrite the reason.
+        if (this._closingError) {
+            return;
+        }
+        // Close with an error: If there is an active task it will receive it justifiably because the user
+        // closed while a task was still running. If no task is running, no error will be thrown (see _closeWithError)
+        // but all newly submitted tasks after that will be rejected because "the client is closed". Plus, the user
+        // gets a stack trace in case it's not clear where exactly the client was closed. We use _closingError to
+        // determine whether a context is closed. This also allows us to have a single code-path for closing a context.
+        const message = this._task ? "User closed client during task" : "User closed client";
+        const err = new Error(message);
+        this._closeWithError(err);
     }
 
+    /**
+     * @returns {boolean}
+     */
+    get closed() {
+        return this._closingError !== undefined;
+    }
+
     /** @type {Socket} */
@@ -126,3 +173,3 @@ get socket() {
             socket.on("data", data => this._onControlSocketData(data));
-            this._setupErrorHandlers(socket, "control");
+            this._setupErrorHandlers(socket, "control socket");
         }
@@ -149,3 +196,3 @@ else {
             socket.setTimeout(this._timeout);
-            this._setupErrorHandlers(socket, "data");
+            this._setupErrorHandlers(socket, "data socket");
         }
@@ -222,17 +269,14 @@ this._dataSocket = socket;
      * @param {string} command
-     * @param {ResponseHandler} handler
+     * @param {ResponseHandler} responseHandler
      * @returns {Promise<any>}
      */
-    handle(command, handler) {
-        if (this._handler !== undefined) {
-            this._closeWithError(new Error("There is still a task running. Did you forget to use '.then()' or 'await'?"));
+    handle(command, responseHandler) {
+        if (this._task) {
+            // The user or client instance called `handle()` while a task is still running.
+            const err = new Error("User launched a task while another one is still running. Forgot to use 'await' or '.then()'?");
+            err.stack += `\nRunning task launched at: ${this._task.stack}`;
+            this._closeWithError(err);
         }
-        // Only track control socket timeout during the lifecycle of a task associated with a handler.
-        // That way we avoid timeouts on idle sockets, a behaviour that is not expected by most users.
-        this.enableControlTimeout(true);
         return new Promise((resolvePromise, rejectPromise) => {
-            this._handler = handler;
-            this._task = {
-                // When resolving or rejecting we also want the handler
-                // to no longer receive any responses or errors.
+            const resolver = {
                 resolve: (...args) => {
@@ -242,8 +286,25 @@ this._stopTrackingTask();
                 },
-                reject: (...args) => {
+                reject: err => {
                     this._stopTrackingTask();
-                    rejectPromise(...args);
+                    rejectPromise(err);
                 }
             };
-            if (command !== undefined) {
+            this._task = {
+                stack: new Error().stack,
+                resolver,
+                responseHandler
+            };
+            if (this._closingError) {
+                // This client has been closed. Provide an error that describes this one as being caused
+                // by `_closingError`, include stack traces for both.
+                const err = new Error("Client is closed");
+                err.stack += `\nClosing reason: ${this._closingError.stack}`;
+                // @ts-ignore that `Error` doesn't have `code` by default.
+                err.code = this._closingError.code !== undefined ? this._closingError.code : 0;
+                this._passToHandler(err);
+            }
+            else if (command) {
+                // Only track control socket timeout during the lifecycle of a task. This avoids timeouts on idle sockets,
+                // the default socket behaviour which is not expected by most users.
+                this.enableControlTimeout(true);
                 this.send(command);
@@ -261,3 +322,2 @@ }
         this._task = undefined;
-        this._handler = undefined;
     }
@@ -283,3 +343,5 @@
             const code = parseInt(message.substr(0, 3), 10);
-            this._passToHandler({ code, message });
+            const response = { code, message };
+            const err = code >= 400 ? new FTPError(response) : undefined;
+            this._passToHandler(err, response);
         }
@@ -293,35 +355,52 @@ }
      * @private
-     * @param {Object} response
+     * @param {(Error | undefined)} err
+     * @param {(FTPResponse | undefined)} [response]
      */
-    _passToHandler(response) {
-        if (this._handler) {
-            this._handler(response, this._task);
+    _passToHandler(err, response) {
+        if (this._task) {
+            this._task.responseHandler(err, response, this._task.resolver);
         }
+        // Errors other than FTPError always close the client. If there isn't an active task to handle the error,
+        // the next one submitted will receive it using `_closingError`.
+        // There is only one edge-case: If there is an FTPError while no task is active, the error will be dropped.
+        // But that means that the user sent an FTP command with no intention of handling the result. So why should the
+        // error be handled? Maybe log it at least? Debug logging will already do that and the client stays useable after
+        // FTPError. So maybe no need to do anything here.
     }
 
     /**
-     * Reset the state of this context.
+     * Send an error to the current handler and close all connections.
      *
      * @private
+     * @param {Error} err
      */
-    _reset() {
-        this.log("Closing connections.");
-        this._stopTrackingTask();
-        this._partialResponse = "";
+    _closeWithError(err) {
+        this._closingError = err;
+        // Before giving the user's task a chance to react, make sure we won't be bothered with any inputs.
         this._closeSocket(this._socket);
         this._closeSocket(this._dataSocket);
-        // Set a new socket instance to make reconnecting possible.
-        this.socket = new Socket();
+        // Give the user's task a chance to react, maybe cleanup resources.
+        this._passToHandler(err);
+        // The task might not have been rejected by the user after receiving the error.
+        this._stopTrackingTask();
     }
 
     /**
-     * Send an error to the current handler and close all connections.
+     * Setup all error handlers for a socket.
      *
      * @private
-     * @param {*} error
+     * @param {Socket} socket
+     * @param {string} identifier
      */
-    _closeWithError(error) {
-        this.log(error);
-        this._passToHandler({ error });
-        this._reset();
+    _setupErrorHandlers(socket, identifier) {
+        socket.once("error", error => {
+            error.message += ` (${identifier})`;
+            this._closeWithError(error);
+        });
+        socket.once("close", hadError => {
+            if (hadError) {
+                this._closeWithError(new Error(`Socket closed due to transmission error (${identifier})`));
+            }
+        });
+        socket.once("timeout", () => this._closeWithError(new Error(`Timeout (${identifier})`)));
     }
@@ -343,19 +422,2 @@
     /**
-     * Setup all error handlers for a socket.
-     *
-     * @private
-     * @param {Socket} socket
-     * @param {string} identifier
-     */
-    _setupErrorHandlers(socket, identifier) {
-        socket.once("error", error => this._closeWithError({ ...error, ftpSocket: identifier }));
-        socket.once("timeout", () => this._closeWithError({ info: "socket timeout", ftpSocket: identifier }));
-        socket.once("close", hadError => {
-            if (hadError) {
-                this._closeWithError({ info: "socket closed due to transmission error", ftpSocket: identifier});
-            }
-        });
-    }
-
-    /**
      * Remove all default listeners for socket.
@@ -367,3 +429,4 @@ *
     _removeSocketListeners(socket) {
-        // socket.removeAllListeners() without name doesn't work: https://github.com/nodejs/node/issues/20923
+        socket.removeAllListeners();
+        // Before Node.js 10.3.0, using `socket.removeAllListeners()` without any name did not work: https://github.com/nodejs/node/issues/20923.
         socket.removeAllListeners("timeout");
@@ -370,0 +433,0 @@ socket.removeAllListeners("data");

package.json

 {
   "name": "basic-ftp",
-  "version": "2.17.1",
+  "version": "3.0.0",
   "description": "FTP client for Node.js with support for explicit FTPS over TLS.",
@@ -5,0 +5,0 @@ "main": "./lib/ftp",

README.md

@@ -9,3 +9,3 @@ # Basic FTP
 
-Prefer alternative transfer protocols like SFTP if you can. Use this library when you have no choice and need to use FTP. Try to use FTPS whenever possible, FTP alone does not encrypt your data.
+Prefer alternative transfer protocols like HTTP or SFTP (SSH) if you can. Use this library when you have no choice and need to use FTP. Try to use FTPS whenever possible, FTP alone does not encrypt your data.
 
@@ -68,4 +68,8 @@ ## Dependencies
 
-Close all socket connections.
+Close the client and all open socket connections. The client can’t be used anymore after calling this method, you have to instantiate a new one to continue any work. A client is also closed automatically if any timeout or connection error occurs. See the section on [Error Handling](#error-handling) below.
 
+`closed`
+
+True if the client has been closed, either by the user or by an error.
+
 `access(options): Promise<Response>`
@@ -185,51 +189,16 @@
 
-## Errors and Timeouts
+## Error Handling
 
-Errors reported by the FTP server will throw an exception. The connection to the FTP server stays intact and you can continue to use it.
+Any error reported by the FTP server will be thrown as `FTPError`. The connection to the FTP server stays intact and you can continue to use your `Client` instance.
 
-This is different with a timeout or connection error: In addition to an exception being thrown, any connection to the FTP server will be closed. You'll have to reconnect to resume any operation.
+This is different with a timeout or connection error: In addition to an `Error` being thrown, any connection to the FTP server will be closed. You’ll have to instantiate a new `Client` and reconnect, if you want to continue any work.
 
-Here are examples for the different types of error messages you'll receive:
+## Logging
 
-### FTP error response
+Using `client.ftp.verbose = true` will log debug-level information to the console. You can use your own logging library by overriding `client.ftp.log`. This method is called regardless of what `client.ftp.verbose` is set to. For example:
 
-FTP response messages that have been interpreted as errors will throw an exception that holds an object describing the response message and the FTP response code.
-
-```js
-{
-    code: 530, // FTP response code
-    message: '530 Login failed.' // Complete FTP response message including code
-}
 ```
-
-### Timeouts
-
-The following format will be encountered for timeouts or closed connections due to transmission error.
-
-```js
-{
-    error: {
-        info: 'socket timeout', // Error type
-        ftpSocket: 'data' // Responsible socket, 'data' or 'control'.
-    }
-}
+myClient.ftp.log = myLogger.debug
 ```
 
-### General connection or other errors
-
-Error objects from Node.js also have an additional field `ftpSocket` that says which socket was responsible for the error. There are two identifiers: `control` for control connections and `data` for data connections. 
-
-```js
-{
-    error: {
-        errno: 'ENETUNREACH', // Typical Node.js error object…
-        code: 'ENETUNREACH',
-        syscall: 'connect',
-        address: '192.168.1.123',
-        port: 21
-        ftpSocket: 'control' // Responsible socket
-    }
-}
-```
-
 ## Extending the library
@@ -247,3 +216,2 @@
 
-
 ### FTPContext
@@ -292,4 +260,7 @@
     const command = "STOR " + remoteFilename
-    return ftp.handle(command, (res, task) => {
-        if (res.code === 150) { // Ready to upload
+    return ftp.handle(command, (err, res, task) => {
+        if (err) {
+	          task.reject(err)
+        }
+        else if (res.code === 150) { // Ready to upload
             readableStream.pipe(ftp.dataSocket)
@@ -300,5 +271,2 @@ }
         }
-        else if (res.code >= 400 || res.error) {
-            task.reject(res)
-        }
     })
@@ -305,0 +273,0 @@ }

test/clientSpec.js

 const assert = require("assert");
 const Client = require("../lib/ftp").Client;
 const SocketMock = require("./SocketMock");
+const { FTPError } = require("../lib/FtpContext");
 
@@ -16,8 +17,2 @@ const featReply = `
 
-class MockError {
-    constructor(info) {
-        this.info = info;
-    }
-}
-
 describe("Convenience API", function() {
@@ -93,3 +88,3 @@ this.timeout(100);
             reply: "500 Error\r\n",
-            result: new MockError({ code: 500, message: "500 Error" })
+            result: new FTPError({code: 500, message: "500 Error"})
         },
@@ -108,3 +103,3 @@ {
             reply: undefined,
-            result: new MockError({ error: { info: "SocketError", ftpSocket: "control" } })
+            result: new Error("some error (control socket)")
         },
@@ -142,8 +137,8 @@ {
                 else {
-                    client.ftp.socket.emit("error", { info: "SocketError" });
+                    client.ftp.socket.emit("error", new Error("some error"));
                 }
             });
             const promise = test.func(client);
-            if (test.result instanceof MockError) {
-                return promise.catch(err => assert.deepEqual(err, test.result.info));
+            if (test.result instanceof Error) {
+                return promise.catch(err => assert.deepEqual(err, test.result));
             }
@@ -175,3 +170,3 @@ else {
         };
-        return client.connect("host", 22).catch(result => assert.deepEqual(result, { code: 120, message: "120 Ready in 5 hours"}));
+        return client.connect("host", 22).catch(result => assert.deepEqual(result, new FTPError({code: 120, message: "120 Ready in 5 hours"})));
     });
@@ -199,4 +194,4 @@
         });
-        return client.login("user", "pass").catch(result => assert.deepEqual(result, { code: 332, message: "332 Account needed" }));
+        return client.login("user", "pass").catch(result => assert.deepEqual(result, new FTPError({code: 332, message: "332 Account needed"})));
     });
 });

test/downloadSpec.js

@@ -5,2 +5,3 @@ const assert = require("assert");
 const SocketMock = require("./SocketMock");
+const { FTPError } = require("../lib/FtpContext");
 
@@ -115,8 +116,3 @@ /**
 
-    it("relays FTP error response even if data transmitted completely", function(done) {
-        client.list().catch(err => {
-            assert.equal(err.code, 500, "Error code");
-            assert.equal(err.message, "500 Error");
-            done();
-        });
+    it("relays FTP error response even if data transmitted completely", function() {
         setTimeout(() => {
@@ -128,3 +124,6 @@ client.ftp.socket.emit("data", "125 Sending");
         });
+        return client.list().catch(err => {
+            assert.deepEqual(err, new FTPError({code: 500, message: "500 Error"}));
+        });
     });
 });

test/ftpContextSpec.js

@@ -8,3 +8,3 @@ const assert = require("assert");
 describe("FTPContext", function() {
-
+    this.timeout(100);
     let ftp;
@@ -36,4 +36,4 @@ beforeEach(function() {
     it("Relays control socket timeout event", function(done) {
-        ftp.handle(undefined, res => {
-            assert.deepEqual(res, { error: { info: "socket timeout", ftpSocket: "control" }});
+        ftp.handle(undefined, err => {
+            assert.deepEqual(err, new Error("Timeout (control socket)"));
             done();
@@ -45,12 +45,12 @@ });
     it("Relays control socket error event", function(done) {
-        ftp.handle(undefined, res => {
-            assert.deepEqual(res, { error: { foo: "bar", ftpSocket: "control" } });
+        ftp.handle(undefined, err => {
+            assert.deepEqual(err, new Error("hello (control socket)"));
             done();
         });
-        ftp.socket.emit("error", { foo: "bar" });
+        ftp.socket.emit("error", new Error("hello"));
     });
 
     it("Relays data socket timeout event", function(done) {
-        ftp.handle(undefined, res => {
-            assert.deepEqual(res, { error: { info: "socket timeout", ftpSocket: "data" }});
+        ftp.handle(undefined, err => {
+            assert.deepEqual(err, new Error("Timeout (data socket)"));
             done();
@@ -62,11 +62,11 @@ });
     it("Relays data socket error event", function(done) {
-        ftp.handle(undefined, res => {
-            assert.deepEqual(res, { error: { foo: "bar", ftpSocket: "data" } });
+        ftp.handle(undefined, err => {
+            assert.deepEqual(err, new Error("hello (data socket)"));
             done();
         });
-        ftp.dataSocket.emit("error", { foo: "bar" });
+        ftp.dataSocket.emit("error", new Error("hello"));
     });
 
     it("Relays single line control response", function(done) {
-        ftp.handle(undefined, res => {
+        ftp.handle(undefined, (err, res) => {
             assert.deepEqual(res, { code: 200, message: "200 OK"});
@@ -79,3 +79,3 @@ done();
     it("Relays multiline control response", function(done) {
-        ftp.handle(undefined, res => {
+        ftp.handle(undefined, (err, res) => {
             assert.deepEqual(res, { code: 200, message: "200-OK\nHello\n200 OK"});
@@ -89,3 +89,3 @@ done();
         const exp = new Set(["200-OK\n200 OK", "200-Again\n200 Again" ]);
-        ftp.handle(undefined, res => {
+        ftp.handle(undefined, (err, res) => {
             assert.equal(true, exp.has(res.message));
@@ -101,3 +101,3 @@ exp.delete(res.message);
     it("Relays chunked multiline response as a single response", function(done) {
-        ftp.handle(undefined, res => {
+        ftp.handle(undefined, (err, res) => {
             assert.deepEqual(res, { code: 200, message: "200-OK\nHello\n200 OK"});
@@ -111,3 +111,3 @@ done();
     it("Stops relaying if task is resolved", function(done) {
-        ftp.handle(undefined, (res, task) => {
+        ftp.handle(undefined, (err, res, task) => {
             if (res.code === 220) {
@@ -140,9 +140,2 @@ assert.fail("Relayed message when it shouldn't have.");
 
-    it("creates a new control socket when closing", function() {
-        const oldSocket = ftp.socket;
-        ftp.close();
-        assert.notEqual(ftp.socket, oldSocket, "Control socket");
-        assert.equal(ftp.dataSocket, undefined, "Data socket");
-    });
-
     it("reports whether socket has TLS", function() {
@@ -154,2 +147,12 @@ ftp.socket = new net.Socket();
     });
+
+    it("queues an error if no task is active and assigns it to the next task", function() {
+        ftp.socket.emit("error", new Error("some error"));
+        return ftp.handle("TEST", (err, res, task) => {
+            err = new Error("Client has been closed: some error (control socket)");
+            err.code = 0;
+            assert.deepEqual(err, err);
+            task.resolve();
+        });
+    });
 });

test/uploadSpec.js

 const assert = require("assert");
+const fs = require("fs");
 const Client = require("../lib/ftp").Client;
 const SocketMock = require("./SocketMock");
-const fs = require("fs");
+const { FTPError } = require("../lib/FtpContext");
 
@@ -20,3 +21,2 @@ describe("Upload", function() {
     afterEach(function() {
-        client.ftp._reset();
         client.close();
@@ -30,3 +30,3 @@ });
         });
-        client.upload(readable, "NAME.TXT");
+        client.upload(readable, "NAME.TXT").catch(() => {});
     });
@@ -41,3 +41,3 @@
         });
-        client.upload(readable, "NAME.TXT");
+        client.upload(readable, "NAME.TXT").catch(() => {});
         setTimeout(() => {
@@ -58,3 +58,3 @@ didSendReady = true;
         });
-        client.upload(readable, "NAME.TXT");
+        client.upload(readable, "NAME.TXT").catch(() => {});
         setTimeout(() => {
@@ -78,3 +78,3 @@ client.ftp.socket.emit("data", "150 Ready");
         });
-        client.upload(readable, "NAME.TXT");
+        client.upload(readable, "NAME.TXT").catch(() => {});
         setTimeout(() => {
@@ -107,8 +107,3 @@ client.ftp.socket.emit("data", "150 Ready");
 
-    it("handles errors", function(done) {
-        client.upload(readable, "NAME.TXT").catch(err => {
-            assert.equal(err.code, 500, "Error code");
-            assert.equal(err.message, "500 Error", "Error message");
-            done();
-        });
+    it("handles errors", function() {
         setTimeout(() => {
@@ -118,3 +113,6 @@ client.ftp.socket.emit("data", "150 Ready");
         });
+        return client.upload(readable, "NAME.TXT").catch(err => {
+            assert.deepEqual(err, new FTPError({code: 500, message: "500 Error"}));
+        });
     });
 });

No alert changes

Improved metrics

Total package byte prevSize

119585

increased by4.12%

Lines of code

2625

increased by3.8%

Worsened metrics

Number of lines in readme file

274

decreased by-10.46%

No dependency changes