feat: better error messages for invalid targets

Closes #43.

.eslintrc.yml

@@ -1,11 +1,18 @@
+extends:
+  - eslint:recommended
+  - plugin:node/recommended
 env:
-  es6: true
   node: true
   mocha: true
-extends: 'eslint:recommended'
-parserOptions:
-  sourceType: module
+plugins:
+  - import
 rules:
+  no-console: 0
+  # Sometimes I like a single 'return' statement in a full bloc, e.g. see the
+  # tests, so relax the requirement to have single return statements
+  arrow-body-style: 0
+  import/no-extraneous-dependencies: [error, { devDependencies: ['**/*.spec.js'] }]
+  no-process-exit: 0
   indent:
     - error
     - 2
@@ -18,4 +25,3 @@ rules:
   semi:
     - error
     - always
-  no-console: 0

.npmignore

@@ -0,0 +1,10 @@
+# Logs
+npm-debug.log*
+
+# Tests
+**/*.spec.js
+
+# Dependency directories
+node_modules
+artifacts/
+.nyc_output/

README.md

@@ -85,6 +85,7 @@ The following error codes are returned:
 | `1`  | A timeout occurred waiting for the port to open. |
 | `2`  | An unknown error occurred waiting for the port to open. The program cannot establish whether the port is open or not. |
 | `3`  | The address cannot be found (e.g. no DNS entry, or unresolvable). |
+| `4`  | The target (host and port) is invalid. |
 
 # API
 

bin/wait-port.js

@@ -4,8 +4,9 @@ const debug = require('debug')('wait-port');
 const program = require('commander');
 const pkg = require('../package.json');
 const extractTarget = require('../lib/extract-target');
-const ValidationError = require('../lib/errors/validation-error');
 const ConnectionError = require('../lib/errors/connection-error');
+const TargetError = require('../lib/errors/target-error');
+const ValidationError = require('../lib/errors/validation-error');
 const waitPort = require('../lib/wait-port');
 
 program
@@ -15,44 +16,45 @@ program
   .option('-o, --output [mode]', 'Output mode (silent, dots). Default is silent.')
   .option('--wait-for-dns', 'Do not fail on ENOTFOUND, meaning you can wait for DNS record creation. Default is false.')
   .arguments('<target>')
-  .action((target) => {
-    //  Validate the parameters (extractTarget) will throw if target is invalid).
-    const { protocol, host, port, path } = extractTarget(target);
-    const timeout = program.timeout || 0;
-    const output = program.output;
-    const waitForDns = program.waitForDns;
+  .action(async (target) => {
+    try {
+      const { protocol, host, port, path } = extractTarget(target);
+      const timeout = program.timeout || 0;
+      const output = program.output;
+      const waitForDns = program.waitForDns;
 
-    debug(`Timeout: ${timeout}`);
-    debug(`Target: ${target} => ${protocol}://${host}:${port}${path}`);
-    debug(`waitForDns: ${waitForDns}`);
+      debug(`Timeout: ${timeout}`);
+      debug(`Target: ${target} => ${protocol}://${host}:${port}${path}`);
+      debug(`waitForDns: ${waitForDns}`);
 
-    const params = {
-      timeout,
-      protocol,
-      host,
-      port,
-      path,
-      output,
-      waitForDns,
-    };
+      const params = {
+        timeout,
+        protocol,
+        host,
+        port,
+        path,
+        output,
+        waitForDns,
+      };
 
-    waitPort(params)
-      .then((open) => {
-        process.exit(open ? 0 : 1);
-      })
-      .catch((err) => {
-        //  Show validation errors in red.
-        if (err instanceof ValidationError) {
-          console.error(`\n\n  ${chalk.red(err.message)}`);
-          process.exit(2);
-        } else if (err instanceof ConnectionError) {
-          console.error(`\n\n  ${chalk.red(err.message)}`);
-          process.exit(4);
-        } else {
-          console.error(`Unknown error occurred waiting for ${target}: ${err}`);
-          process.exit(3);
-        }
-      });
+      const open = await waitPort(params);
+      process.exit(open ? 0 : 1);
+    } catch (err) {
+      //  Show validation errors in red.
+      if (err instanceof ValidationError) {
+        console.error(chalk.red(`\n  Validation Error: ${err.message}`));
+        process.exit(2);
+      } else if (err instanceof ConnectionError) {
+        console.error(chalk.red(`\n\n  Connection Error Error: ${err.message}`));
+        process.exit(4);
+      } else if (err instanceof TargetError) {
+        console.error(chalk.red(`\n  Target Error: ${err.message}`));
+        process.exit(4);
+      } else {
+        console.error(chalk.red(`\n  Unknown error occurred waiting for ${target}: ${err}`));
+        process.exit(3);
+      }
+    }
   });
 
 //  Enrich the help.

lib/errors/target-error.js

@@ -0,0 +1,9 @@
+class TargetError extends Error {
+  constructor(message) {
+    super(message);
+    Error.captureStackTrace(this, this.constructor);
+    this.name = 'TargetError';
+  }
+}
+
+module.exports = TargetError;

lib/extract-target.js

@@ -1,5 +1,7 @@
+const TargetError = require('./errors/target-error');
+
 function extractTarget(target) {
-  if (!target) throw new Error('\'target\' is required');
+  if (!target) throw new TargetError('\'target\' is required');
 
   //  First, check to see if we have a protocol specified.
   const protocol = target.toLowerCase().startsWith('http://') ? 'http' : undefined;
@@ -14,14 +16,14 @@ function extractTarget(target) {
 
   //  Split the target by the separator (which might not be present.
   const split = target.split(':');
-  if (split.length > 2) throw new Error('\'target\' is invalid');
+  if (split.length > 2) throw new TargetError(`'${target}' is an invalid target, it has more than two ':' symbols`);
 
   //  Grab the host and port (which will still be a string).
   const host = split.length === 2 ? (split[0] || undefined) : undefined;
   const portString = split.length === 1 ? split[0] : split[1];
 
   //  Make sure the port is numeric.
-  if (!/^[0-9]+$/.test(portString)) throw new Error('\'port\' must be a number');
+  if (!/^[0-9]+$/.test(portString)) throw new TargetError(`'${target}' is an invalid target, '${portString}' is not a valid port number - try something like 'host:port'`);
   const port = parseInt(portString, 10);
 
   //  That's it, return the extracted target.

lib/extract-target.spec.js

@@ -1,25 +1,39 @@
 const assert = require('assert');
 const extractTarget = require('./extract-target');
+const TargetError = require('./errors/target-error');
+
+//  Note: from Node 10 onwards we can use the much cleaner format:
+//    assert.throws(() => extractTarget('808X'), {
+//      name: 'TargetError',
+//      message: /808X.*port.*number/,
+//    });
+//  however, Node 8 won't use a regex in 'messagae'.
+
 
 describe('extractTarget', () => {
   it('should throw if no target is provided', () => {
+    assert.throws(() => extractTarget(), TargetError);
     assert.throws(() => extractTarget(), /target/);
   });
 
   it('should throw if more than one separator is used', () => {
-    assert.throws(() => extractTarget('host:port:wtf'), /invalid/);
+    assert.throws(() => extractTarget('host:port:wtf'), TargetError);
+    assert.throws(() => extractTarget('host:port:wtf'), /invalid.*:/);
   });
 
   it('should throw if a non-numeric port target is provided', () => {
-    assert.throws(() => extractTarget('808O', /number/));
+    assert.throws(() => extractTarget('808X'), TargetError);
+    assert.throws(() => extractTarget('808X'), /808X.*port.*number/);
   });
 
   it('should throw if a non-numeric :port target is provided', () => {
-    assert.throws(() => extractTarget(':808O', /number/));
+    assert.throws(() => extractTarget(':808X'), TargetError);
+    assert.throws(() => extractTarget(':808X'), /808X.*port.*number/);
   });
 
   it('should throw if a non-numeric host:port target is provided', () => {
-    assert.throws(() => extractTarget('host:808O', /number/));
+    assert.throws(() => extractTarget('host:808X'), TargetError);
+    assert.throws(() => extractTarget('host:808X'), /808X.*port.*number/);
   });
 
   it('should extract a valid port', () => {

lib/validate-parameters.spec.js

@@ -16,7 +16,7 @@ describe('validateParameters', () => {
   it('should allow an undefined protocol', () => {
 
     const params = validParams();
-    assert.equal(validateParameters(params).protocol, undefined);
+    assert.strictEqual(validateParameters(params).protocol, undefined);
 
   });
 
@@ -53,22 +53,22 @@ describe('validateParameters', () => {
     const params = validParams();
     delete params.host;
     const validatedParams = validateParameters(params);
-    assert.equal(validatedParams.host, 'localhost');
+    assert.strictEqual(validatedParams.host, 'localhost');
 
   });
 
   it('should allow an undefined path', () => {
 
     const params = validParams();
-    assert.equal(validateParameters(params).path, undefined);
+    assert.strictEqual(validateParameters(params).path, undefined);
 
   });
 
   it('should set the path to root if the http protocol is used but no path is specified', () => {
 
     let params = validParams();
     params.protocol = 'http';
-    assert.equal(validateParameters(params).path, '/');
+    assert.strictEqual(validateParameters(params).path, '/');
 
   });
 
@@ -77,7 +77,7 @@ describe('validateParameters', () => {
     const params = validParams();
     delete params.interval;
     const validatedparams = validateParameters(params);
-    assert.equal(validatedparams.interval, 1000);
+    assert.strictEqual(validatedparams.interval, 1000);
 
   });
 
@@ -96,7 +96,7 @@ describe('validateParameters', () => {
     const params = validParams();
     delete params.timeout;
     const validatedparams = validateParameters(params);
-    assert.equal(validatedparams.timeout, 0);
+    assert.strictEqual(validatedparams.timeout, 0);
 
   });
 
@@ -115,7 +115,7 @@ describe('validateParameters', () => {
     const params = validParams();
     delete params.output;
     const validatedparams = validateParameters(params);
-    assert.equal(validatedparams.output, 'dots');
+    assert.strictEqual(validatedparams.output, 'dots');
 
   });
 
@@ -132,7 +132,7 @@ describe('validateParameters', () => {
     const params = validParams();
     delete params.waitForDns;
     const validatedParams = validateParameters(params);
-    assert.equal(validatedParams.waitForDns, false);
+    assert.strictEqual(validatedParams.waitForDns, false);
 
   });
 

lib/wait-port.spec.js

@@ -77,7 +77,7 @@ describe('wait-port', () => {
         assert.fail('The operation should throw, rather than completing.');
       })
       .catch((err) => {
-        assert.equal(err.name, 'ConnectionError', 'A ConnectionFailed error should be thrown');
+        assert.strictEqual(err.name, 'ConnectionError', 'A ConnectionFailed error should be thrown');
         assert(/.*address.*ireallyhopethatthisdomainnamedoesnotexist.com/.test(err.message));
       });
   });