chore(systemd): improve error handling

refs #587

extensions/nginx/index.js

@@ -70,11 +70,19 @@ class NginxExtension extends cli.Extension {
             'nginx config',
             confFile,
             '/etc/nginx/sites-available'
-        ).then(() => {
-            return this.ui.sudo(`ln -sf /etc/nginx/sites-available/${confFile} /etc/nginx/sites-enabled/${confFile}`);
-        }).then(() => this.restartNginx()).catch((error) => {
-            return Promise.reject(new cli.errors.ProcessError(error));
-        });
+        ).then(
+            () => this.ui.sudo(`ln -sf /etc/nginx/sites-available/${confFile} /etc/nginx/sites-enabled/${confFile}`)
+        ).then(
+            () => this.restartNginx()
+        ).catch(
+            (error) => {
+                // CASE: error is already a cli error, just pass it along
+                if (error instanceof cli.errors.CliError) {
+                    return Promise.reject(error);
+                }
+
+                return Promise.reject(new cli.errors.ProcessError(error));
+            });
     }
 
     setupSSL(argv, ctx, task) {
@@ -204,6 +212,8 @@ class NginxExtension extends cli.Extension {
                     '/etc/nginx/sites-available'
                 ).then(
                     () => this.ui.sudo(`ln -sf /etc/nginx/sites-available/${confFile} /etc/nginx/sites-enabled/${confFile}`)
+                ).catch(
+                    (error) => Promise.reject(new cli.errors.ProcessError(error))
                 );
             }
         }, {
@@ -229,6 +239,7 @@ class NginxExtension extends cli.Extension {
                 ]).catch(
                     (error) => Promise.reject(new cli.errors.CliError({
                         message: `Nginx config file link could not be removed, you will need to do this manually for ${confFile}.`,
+                        help: `Try running 'rm -f /etc/nginx/sites-available/${confFile} && rm -f /etc/nginx/sites-enabled/${confFile}'`,
                         err: error
                     }))
                 )
@@ -244,6 +255,7 @@ class NginxExtension extends cli.Extension {
                 ]).catch(
                     (error) => Promise.reject(new cli.errors.CliError({
                         message: `SSL config file link could not be removed, you will need to do this manually for ${sslConfFile}.`,
+                        help: `Try running 'rm -f /etc/nginx/sites-available/${sslConfFile} && rm -f /etc/nginx/sites-enabled/${sslConfFile}'`,
                         err: error
                     }))
                 )
@@ -259,7 +271,10 @@ class NginxExtension extends cli.Extension {
 
     restartNginx() {
         return this.ui.sudo('nginx -s reload')
-            .catch((error) => Promise.reject(new cli.errors.ProcessError(error)));
+            .catch((error) => Promise.reject(new cli.errors.CliError({
+                message: 'Failed to restart Nginx.',
+                err: error
+            })));
     }
 
     isSupported() {

extensions/nginx/test/extension-spec.js

@@ -222,6 +222,38 @@ describe('Unit: Extensions > Nginx', function () {
             });
         });
 
+        it('passes the error if it\'s already a CliError', function () {
+            const name = 'ghost.dev.conf';
+            const lnExp = new RegExp(`(?=^ln -sf)(?=.*available/${name})(?=.*enabled/${name}$)`);
+            ctx.instance.dir = dir;
+            ctx.instance.template = sinon.stub().resolves();
+            const loadStub = sinon.stub().returns('nginx config file');
+            const templateStub = sinon.stub().returns(loadStub);
+            const ext = proxyNginx({
+                'fs-extra': {
+                    existsSync: () => false,
+                    readFileSync: () => 'hello'
+                },
+                'lodash/template': templateStub
+            });
+            const sudo = sinon.stub().resolves()
+            ext.ui.sudo = sudo;
+            ext.restartNginx = sinon.stub().rejects(new errors.CliError('Did not restart'));
+
+            return ext.setupNginx(null, ctx, task).then(() => {
+                expect(false, 'Promise should have rejected').to.be.true
+            }).catch((error) => {
+                expect(error).to.exist;
+                expect(error).to.be.an.instanceof(errors.CliError);
+                expect(error.message).to.be.equal('Did not restart');
+                expect(templateStub.calledOnce).to.be.true;
+                expect(loadStub.calledOnce).to.be.true;
+                expect(sudo.calledOnce).to.be.true;
+                expect(sudo.args[0][0]).to.match(lnExp);
+                expect(ext.restartNginx.calledOnce).to.be.true;
+            });
+        });
+
         it('returns a ProcessError when symlink command fails', function () {
             const name = 'ghost.dev.conf';
             const lnExp = new RegExp(`(?=^ln -sf)(?=.*available/${name})(?=.*enabled/${name}$)`);
@@ -589,6 +621,22 @@ describe('Unit: Extensions > Nginx', function () {
                     expect(stubs.templatify.args[0][0]).to.deep.equal(expectedTemplate);
                 });
             });
+
+            it('rejects with error if configuration fails', function () {
+                const ext = proxyNginx(proxy);
+                const tasks = getTasks(ext);
+                const sudo = sinon.stub().rejects({stderr: 'oh no!'});
+                ext.ui.sudo = sudo;
+
+                return tasks[6].task(ctx).then(() => {
+                    expect(false, 'Promise should have been rejected').to.be.true;
+                }).catch((err) => {
+                    expect(stubs.template.calledTwice).to.be.true;
+                    expect(stubs.templatify.calledOnce).to.be.true;
+                    expect(ext.ui.sudo.calledOnce).to.be.true;
+                    expect(err.options.stderr).to.equal('oh no!');
+                });
+            })
         });
         describe('Restart', function () {
             it('Restarts Nginx', function () {
@@ -687,7 +735,7 @@ describe('Unit: Extensions > Nginx', function () {
                 expect(sudo.calledOnce).to.be.true;
                 expect(sudo.args[0][0]).to.match(/nginx -s reload/);
                 expect(err).to.be.ok;
-                expect(err).to.be.instanceof(errors.ProcessError);
+                expect(err).to.be.instanceof(errors.CliError);
             });
         });
     });

extensions/systemd/get-uid.js

@@ -23,7 +23,9 @@ module.exports = function getUid(dir) {
         return uid;
     } catch (e) {
         if (!e.message.match(/no such user/i)) {
-            throw e;
+            const errors = require('../../lib').errors;
+            // throw the error here so it'll be logged
+            throw new errors.ProcessError(e);
         }
 
         return null;

extensions/systemd/index.js

@@ -17,7 +17,15 @@ class SystemdExtension extends cli.Extension {
     }
 
     _setup(argv, ctx, task) {
-        const uid = getUid(ctx.instance.dir);
+        let uid;
+
+        // getUid returns either the uid or null, but can also throw an error
+        try {
+            uid = getUid(ctx.instance.dir);
+        } catch (e) {
+            this.ui.log('The "ghost" user has not been created, please run `ghost setup linux-user` first', 'yellow');
+            return task.skip();
+        }
 
         if (!uid) {
             this.ui.log('The "ghost" user has not been created, please run `ghost setup linux-user` first', 'yellow');
@@ -41,7 +49,9 @@ class SystemdExtension extends cli.Extension {
             ghost_exec_path: process.argv.slice(0,2).join(' ')
         }), 'systemd service', serviceFilename, '/lib/systemd/system').then(
             () => this.ui.sudo('systemctl daemon-reload')
-        );
+        ).catch((error) => {
+            return Promise.reject(new cli.errors.ProcessError(error));
+        });
     }
 
     uninstall(instance) {

extensions/systemd/systemd.js

@@ -89,14 +89,24 @@ class SystemdProcessManager extends cli.ProcessManager {
                 // Systemd prints out "inactive" if service isn't running
                 // or "activating" if service hasn't completely started yet
                 if (!error.message.match(/inactive|activating/)) {
-                    return Promise.reject(error);
+                    return Promise.reject(new cli.errors.ProcessError(error));
                 }
                 return Promise.resolve(false);
             });
     }
 
     _precheck() {
-        const uid = getUid(this.instance.dir);
+        let uid;
+
+        // getUid returns either the uid or null, but can also throw an error
+        try {
+            uid = getUid(this.instance.dir);
+        } catch (e) {
+            // getuid throws a ProcessError, we add a message and help and just throw it
+            e.message = 'Systemd process manager has not been set up or is corrupted.';
+            e.options.help = `Run ${chalk.green('ghost setup linux-user systemd')} and try again.`
+            throw e;
+        }
 
         if (!uid) {
             throw new cli.errors.SystemError({

extensions/systemd/test/extension-spec.js

@@ -53,6 +53,25 @@ describe('Unit: Systemd > Extension', function () {
     });
 
     describe('setup stage', function () {
+        it('can handle getUid error', function () {
+            const uidStub = sinon.stub().throws(new errors.ProcessError({stderr: 'something went wrong'}));
+
+            const SystemdExtension = proxyquire(modulePath, {
+                './get-uid': uidStub
+            });
+
+            const logStub = sinon.stub();
+            const skipStub = sinon.stub();
+            const testInstance = new SystemdExtension({log: logStub}, {}, {}, path.join(__dirname, '..'));
+
+            testInstance._setup({}, {instance: {dir: '/some/dir'}}, {skip: skipStub});
+            expect(uidStub.calledOnce).to.be.true;
+            expect(uidStub.calledWithExactly('/some/dir')).to.be.true;
+            expect(logStub.calledOnce).to.be.true;
+            expect(logStub.args[0][0]).to.match(/"ghost" user has not been created/);
+            expect(skipStub.calledOnce).to.be.true;
+        });
+
         it('skips stage if ghost user hasn\'t been set up', function () {
             const uidStub = sinon.stub().returns(false);
 
@@ -153,6 +172,44 @@ describe('Unit: Systemd > Extension', function () {
                 expect(skipStub.called).to.be.false;
             });
         });
+
+        it('can handle error when template method fails', function () {
+            const uidStub = sinon.stub().returns(true);
+            const existsStub = sinon.stub().returns(false);
+            const readFileSyncStub = sinon.stub().returns('SOME TEMPLATE CONTENTS');
+
+            const SystemdExtension = proxyquire(modulePath, {
+                './get-uid': uidStub,
+                'fs-extra': {existsSync: existsStub, readFileSync: readFileSyncStub}
+            });
+
+            const logStub = sinon.stub();
+            const sudoStub = sinon.stub().rejects({stderr: 'something went wrong'});
+            const skipStub = sinon.stub();
+            const getStub = sinon.stub().returns(false);
+            const templateStub = sinon.stub().resolves();
+            const testInstance = new SystemdExtension({log: logStub, sudo: sudoStub}, {}, {}, path.join(__dirname, '..'));
+            const instance = {dir: '/some/dir', cliConfig: {get: getStub}, name: 'test', template: templateStub};
+
+            return testInstance._setup({}, {instance: instance}, {skip: skipStub}).then(() => {
+                expect(false, 'Promise should have rejected').to.be.true
+            }).catch((error) => {
+                expect(error).to.exist;
+                expect(error).to.be.an.instanceof(errors.ProcessError);
+                expect(error.options.stderr).to.be.equal('something went wrong');
+                expect(uidStub.calledOnce).to.be.true;
+                expect(uidStub.calledWithExactly('/some/dir')).to.be.true;
+                expect(getStub.calledOnce).to.be.true;
+                expect(existsStub.calledOnce).to.be.true;
+                expect(readFileSyncStub.calledOnce).to.be.true;
+                expect(templateStub.calledOnce).to.be.true;
+                expect(templateStub.calledWith('SOME TEMPLATE CONTENTS')).to.be.true;
+                expect(sudoStub.calledOnce).to.be.true;
+                expect(sudoStub.calledWithExactly('systemctl daemon-reload')).to.be.true;
+                expect(logStub.called).to.be.false;
+                expect(skipStub.called).to.be.false;
+            });
+        });
     });
 
     describe('uninstall hook', function () {

extensions/systemd/test/get-uid-spec.js

@@ -6,7 +6,7 @@ const proxyquire = require('proxyquire').noCallThru();
 const modulePath = '../get-uid';
 
 describe('Unit: Systemd > get-uid util', function () {
-    it('throws error if execa error is not an expected one', function () {
+    it('throws ProcessError if execa error is not an expected one, but is stderr', function () {
         const shellStub = sinon.stub().throws(new Error('some error'));
         const getUid = proxyquire(modulePath, {
             execa: {shellSync: shellStub}
@@ -16,7 +16,8 @@ describe('Unit: Systemd > get-uid util', function () {
             getUid('/some/dir');
             expect(false, 'error should have been thrown').to.be.true;
         } catch (e) {
-            expect(e).to.be.an.instanceof(Error);
+            const errors = require('../../../lib/errors');
+            expect(e).to.be.an.instanceof(errors.ProcessError);
             expect(e.message).to.equal('some error');
             expect(shellStub.calledOnce).to.be.true;
         }

extensions/systemd/test/systemd-spec.js

@@ -260,7 +260,7 @@ describe('Unit: Systemd > Process Manager', function () {
             });
         });
 
-        it('Passes bad errors through', function () {
+        it('Throws ProcessError for bad errors', function () {
             const ui = {sudo: sinon.stub().rejects(new Error('unknown'))};
             const ext = makeSystemd(null, ui);
             const expectedCmd = 'systemctl is-active ghost_ghost_org';
@@ -269,6 +269,7 @@ describe('Unit: Systemd > Process Manager', function () {
                 expect(false, 'An error should have been thrown').to.be.true;
             }).catch((error) => {
                 expect(error.message).to.equal('unknown');
+                expect(error).to.be.an.instanceof(errors.ProcessError);
                 expect(ui.sudo.calledOnce).to.be.true;
                 expect(ui.sudo.args[0][0]).to.equal(expectedCmd);
             });
@@ -294,6 +295,22 @@ describe('Unit: Systemd > Process Manager', function () {
             proxyOpts = {'./get-uid': sinon.stub().returns(true)};
         });
 
+        it('can handle when uid throws error', function () {
+            proxyOpts['./get-uid'] = sinon.stub().throws(new errors.ProcessError({stderr: 'something went wrong'}));
+            const ext = makeSystemd(proxyOpts);
+            try {
+                ext._precheck();
+                expect(false, 'An error should have been thrown').to.be.true;
+            } catch (error) {
+                expect(proxyOpts['./get-uid'].calledOnce).to.be.true;
+                expect(error).to.be.ok;
+                expect(error).to.be.instanceOf(errors.ProcessError);
+                expect(error.message).to.match(/Systemd process manager has not been set up or is corrupted./);
+                expect(error.options.help).to.match(/ghost setup linux-user systemd/);
+                expect(error.options.stderr).to.match(/something went wrong/);
+            }
+        });
+
         it('Errors if uid doesn\'t been set', function () {
             proxyOpts['./get-uid'] = sinon.stub().returns(null);
             const ext = makeSystemd(proxyOpts);