Address Race Conditions and Better Error Handling to fix Issue #256 (#316)

* Issue #256 Better Error Handling

There were a couple of anti patterns being used, and improper use of promises that lead to race conditions:

1. Using `process.exit()`  directly is usually not advisable as it prevents things from shutting down
nicely. It also means if someone is trying to use this as a library, which I think may be the
case for issue #256, you will exit from their code as well (i.e., error handling becomes impossible).

2. Functions that caught errors were printing them directly to STDERR rather than rethrowing them or
throwing a new error. Again, if someone is trying to use it as a library as in the issues, this will
make it impossible for them to do error handling.

3. There were some places where errors were being caught and printed to STDERR, but not causing the
the process to exit with a non-zero exit code. It's always advisable to exit with a non-zero exit code
when there is a failure.

4. There were some places where promises were not being waited for, leading to race conditions.

* Fix audit issues

Author: mearns

.gitignore

@@ -4,3 +4,4 @@ npm-debug.log
 .idea
 coverage/
 .nyc_output
+.vscode/

bin/tldr

@@ -84,29 +84,36 @@ if (program.theme) {
 
 const tldr = new Tldr(cfg);
 
+let p = null;
 if (program.list) {
-  tldr.list(program.singleColumn);
+  p = tldr.list(program.singleColumn);
 } else if (program.listAll) {
-  tldr.listAll(program.singleColumn);
+  p = tldr.listAll(program.singleColumn);
 } else if (program.random) {
-  tldr.random(program);
+  p = tldr.random(program);
 } else if (program.randomExample) {
-  tldr.randomExample();
+  p = tldr.randomExample();
 } else if (program.clearCache) {
-  tldr.clearCache();
+  p = tldr.clearCache();
 } else if (program.update) {
-  tldr.updateCache()
+  p = tldr.updateCache()
     .then(() => {
-      tldr.updateIndex();
+      return tldr.updateIndex();
     });
 } else if (program.render) {
-  tldr.render(program.render);
+  p = tldr.render(program.render);
 } else if (program.search) {
   program.args.unshift(program.search);
-  tldr.search(program.args);
+  p = tldr.search(program.args);
 } else if (program.args.length >= 1) {
-  tldr.get(program.args, program);
-} else {
+  p = tldr.get(program.args, program);
+}
+
+if (p === null) {
   program.outputHelp();
-  process.exit(1);
+  process.exitCode = 1;
 }
+p.catch((err) => {
+  console.error(err);
+  process.exitCode = 1;
+});

lib/tldr.js

@@ -11,7 +11,7 @@ const messages = require('./messages');
 const parser = require('./parser');
 const render = require('./render');
 const index = require('./index');
-const exit = process.exit;
+
 
 class Tldr {
   constructor(config) {
@@ -23,34 +23,33 @@ class Tldr {
 
   list(singleColumn) {
     let os = platform.getPreferredPlatformFolder(this.config);
-    index.commandsFor(os)
+    return index.commandsFor(os)
       .then((commands) => {
-        this.printPages(commands, singleColumn);
+        return this.printPages(commands, singleColumn);
       });
   }
 
   listAll(singleColumn) {
-    index.commands()
+    return index.commands()
       .then((commands) => {
-        this.printPages(commands, singleColumn);
+        return this.printPages(commands, singleColumn);
       });
   }
 
   get(commands, options) {
-    this.printBestPage(commands.join('-'), options);
+    return this.printBestPage(commands.join('-'), options);
   }
 
   random(options) {
     let os = platform.getPreferredPlatformFolder(this.config);
-    index.commandsFor(os)
+    return index.commandsFor(os)
       .then((pages) => {
         if (pages.length === 0) {
-          console.error(messages.emptyCache());
-          exit(1);
+          throw new Error(messages.emptyCache());
         }
         let page = sample(pages);
         console.log('PAGE', page);
-        this.printBestPage(page, options);
+        return this.printBestPage(page, options);
       })
       .catch((err) => {
         console.error(err);
@@ -59,138 +58,107 @@ class Tldr {
 
   randomExample() {
     let os = platform.getPreferredPlatformFolder(this.config);
-    index.commandsFor(os)
+    return index.commandsFor(os)
       .then((pages) => {
         if (pages.length === 0) {
-          console.error(messages.emptyCache());
-          exit(1);
+          throw new Error(messages.emptyCache());
         }
         let page = sample(pages);
         console.log('PAGE', page);
-        this.printBestPage(page, {randomExample: true});
+        return this.printBestPage(page, {randomExample: true});
       })
       .catch((err) => {
         console.error(err);
       });
   }
 
   render(file) {
-    fs.readFile(file, 'utf8')
+    return fs.readFile(file, 'utf8')
       .then((content) => {
         // Getting the shortindex first to populate the shortindex var
         return index.getShortIndex().then(() => {
           this.renderContent(content);
         });
-      })
-      .catch((err) => {
-        console.error(err);
-        exit(1);
       });
   }
 
   clearCache() {
-    this.cache.clear().then(() => {
+    return this.cache.clear().then(() => {
       console.log('Done');
     });
   }
 
   updateCache() {
-    const spinner = ora();
-    spinner.start('Updating...');
-    return this.cache.update()
-      .then(() => {
-        spinner.succeed();
-      })
-      .catch((err) => {
-        console.error(err);
-        exit(1);
-      });
+    return spinningPromise('Updating...', () => {
+      return this.cache.update();
+    });
   }
 
   updateIndex() {
-    const spinner = ora();
-    spinner.start('Creating index...');
-    search.createIndex()
-      .then(() => {
-        spinner.succeed();
-      })
-      .catch((err) => {
-        console.error(err);
-      });
+    return spinningPromise('Creating index...', () => {
+      return search.createIndex();
+    });
   }
 
   search(keywords) {
-    search.getResults(keywords.join(' '))
+    return search.getResults(keywords.join(' '))
       .then((results) => {
         // TODO: make search into a class also.
         search.printResults(results, this.config);
-      })
-      .catch((err) => {
-        console.error(err);
       });
   }
 
   printPages(pages, singleColumn) {
     if (pages.length === 0) {
-      console.error(messages.emptyCache());
-      exit(1);
+      throw new Error(messages.emptyCache());
     }
-    this.checkStale();
-    let endOfLine = require('os').EOL;
-    let delimiter = singleColumn ? endOfLine : ', ';
-    console.log('\n' + pages.join(delimiter));
+    return this.checkStale()
+      .then(() => {
+        let endOfLine = require('os').EOL;
+        let delimiter = singleColumn ? endOfLine : ', ';
+        console.log('\n' + pages.join(delimiter));
+      });
   }
 
   printBestPage(command, options={}) {
-    const spinner = ora();
-
     // Trying to get the page from cache first
-    this.cache.getPage(command)
+    return this.cache.getPage(command)
       .then((content) => {
         // If found in first try, render it
-        if (content) {
-          this.checkStale();
-          this.renderContent(content, options);
-          return exit();
+        if (!content) {
+          // If not found, try to update
+          return spinningPromise('Page not found. Updating cache...', () => {
+            return this.cache.update();
+          })
+            .then(() => {
+              return spinningPromise('Creating index...', () => {
+                return search.createIndex();
+              });
+            })
+            .then(() => {
+              // And then, try to check in cache again
+              return this.cache.getPage(command);
+            });
         }
-        // If not found, try to update
-        spinner.start('Page not found. Updating cache...');
-        return this.cache.update();
-      })
-      .then(() => {
-        spinner.succeed();
-        spinner.start('Creating index...');
-        return search.createIndex();
-      })
-      .then(() => {
-        spinner.succeed();
-        // And then, try to check in cache again
-        return this.cache.getPage(command);
+        return content;
       })
       .then((content) => {
         if (!content) {
-          console.error(messages.notFound());
-          return exit(1);
+          throw new Error(messages.notFound());
         }
-        this.checkStale();
-        this.renderContent(content, options);
-      })
-      .catch((err) => {
-        console.error(err);
-        exit(1);
+        return this.checkStale()
+          .then(() => {
+            this.renderContent(content, options);
+          });
       });
   }
 
   checkStale() {
-    this.cache.lastUpdated()
+    return this.cache.lastUpdated()
       .then((stats) => {
         if (stats.mtime < Date.now() - ms('30d')) {
           console.warn('Cache is out of date. You should run "tldr --update"');
         }
-      })
-      .catch((err) => {
-        console.error(err);
-        exit(1);
       });
   }
 
@@ -209,4 +177,18 @@ class Tldr {
   }
 }
 
+function spinningPromise(text, factory) {
+  const spinner = ora();
+  spinner.start(text);
+  return factory()
+    .then((val) => {
+      spinner.succeed();
+      return val;
+    })
+    .catch((err) => {
+      spinner.fail();
+      throw err;
+    });
+}
+
 module.exports = Tldr;