Skip to content

Commit

Permalink
chore(systemd): ignore error in get-uid and return null
Browse files Browse the repository at this point in the history
refs TryGhost#587

Instead of returning an error in the `get-uid` util we can simplify it and just return null.

Explanation:

Possible reasons for error:
- Command is not known by OS or
- User doesn't exist

The first case is not very likely to happen, as systemd is never setup on a Windows. And the second case would exactly be the result we want. false if the user doesn't exist.
  • Loading branch information
aileen committed Mar 16, 2018
1 parent a168368 commit ee51d3d
Show file tree
Hide file tree
Showing 6 changed files with 15 additions and 72 deletions.
10 changes: 4 additions & 6 deletions extensions/systemd/get-uid.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,10 @@ module.exports = function getUid(dir) {

return uid;
} catch (e) {
if (!e.message.match(/no such user/i)) {
const errors = require('../../lib').errors;
// throw the error here so it'll be logged
throw new errors.ProcessError(e);
}

// CASE: the ghost user doesn't exist, hence can't be used
// We just return null and not doing anything with the error,
// as it would either mean, that the user doesn't exist (this
// is exactly what we want to know), or the command is not by the OS
return null;
}
};
11 changes: 2 additions & 9 deletions extensions/systemd/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,9 @@ class SystemdExtension extends cli.Extension {
}

_setup(argv, ctx, task) {
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();
}
const uid = getUid(ctx.instance.dir);

// getUid returns either the uid or null
if (!uid) {
this.ui.log('The "ghost" user has not been created, please run `ghost setup linux-user` first', 'yellow');
return task.skip();
Expand Down
15 changes: 4 additions & 11 deletions extensions/systemd/systemd.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ class SystemdProcessManager extends cli.ProcessManager {

isEnabled() {
try {
// TODO: this should probably also run as sudo
execa.shellSync(`systemctl is-enabled ${this.systemdName}`);
return true;
} catch (e) {
Expand All @@ -85,6 +86,7 @@ class SystemdProcessManager extends cli.ProcessManager {

isRunning() {
try {
// TODO: this should probably also run as sudo
execa.shellSync(`systemctl is-active ${this.systemdName}`);
return true;
} catch (e) {
Expand All @@ -99,18 +101,9 @@ class SystemdProcessManager extends cli.ProcessManager {
}

_precheck() {
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;
}
const uid = getUid(this.instance.dir);

// getUid returns either the uid or null
if (!uid) {
throw new cli.errors.SystemError({
message: 'Systemd process manager has not been set up or is corrupted.',
Expand Down
19 changes: 0 additions & 19 deletions extensions/systemd/test/extension-spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,25 +53,6 @@ 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);

Expand Down
14 changes: 4 additions & 10 deletions extensions/systemd/test/get-uid-spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,21 +6,15 @@ const proxyquire = require('proxyquire').noCallThru();
const modulePath = '../get-uid';

describe('Unit: Systemd > get-uid util', function () {
it('throws ProcessError if execa error is not an expected one, but is stderr', function () {
it('catches error but returns null', function () {
const shellStub = sinon.stub().throws(new Error('some error'));
const getUid = proxyquire(modulePath, {
execa: {shellSync: shellStub}
});

try {
getUid('/some/dir');
expect(false, 'error should have been thrown').to.be.true;
} catch (e) {
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;
}
const result = getUid('/some/dir');
expect(result).to.be.null;
expect(shellStub.calledOnce).to.be.true;
});

it('returns null if ghost user doesn\'t exist', function () {
Expand Down
18 changes: 1 addition & 17 deletions extensions/systemd/test/systemd-spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -301,23 +301,7 @@ 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 () {
it('Errors if uid hasn\'t been set', function () {
proxyOpts['./get-uid'] = sinon.stub().returns(null);
const ext = makeSystemd(proxyOpts);
try {
Expand Down

0 comments on commit ee51d3d

Please sign in to comment.