Skip to content

Commit ca68049

Browse files
acburdineacburdine
committed
feat(doctor): run all checks and print summary of errors
no issue - add task property to all errors output by the various tasks - fix tests - add isDoctorCommand property to doctor context
1 parent c4a40a0 commit ca68049

13 files changed

+116
-77
lines changed

‎lib/commands/doctor/checks/check-directory.js

+6-3
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ const isRoot = require('path-is-root');
66

77
const errors = require('../../../errors');
88

9-
module.exports = function checkDirectoryAndAbove(dir, extra) {
9+
module.exports = function checkDirectoryAndAbove(dir, extra, task) {
1010
if (isRoot(dir)) {
1111
return Promise.resolve();
1212
}
@@ -15,8 +15,11 @@ module.exports = function checkDirectoryAndAbove(dir, extra) {
1515
const mode = new Mode(stats);
1616

1717
if (!mode.others.read) {
18-
return Promise.reject(new errors.SystemError(`The path ${dir} is not readable by other users on the system.
19-
This can cause issues with the CLI, please either make this directory readable by others or ${extra} in another location.`));
18+
return Promise.reject(new errors.SystemError({
19+
message: `The path ${dir} is not readable by other users on the system.
20+
This can cause issues with the CLI, please either make this directory readable by others or ${extra} in another location.`,
21+
task: task
22+
}));
2023
}
2124

2225
return checkDirectoryAndAbove(path.join(dir, '../'), extra);

‎lib/commands/doctor/checks/check-permissions.js

+7-3
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ const chalk = require('chalk');
55

66
const errors = require('../../../errors');
77

8-
module.exports = function checkPermissions(type) {
8+
module.exports = function checkPermissions(type, task) {
99
let errMsg;
1010

1111
// fall back to check the owner permission of the content folder if nothing specified
@@ -41,7 +41,10 @@ module.exports = function checkPermissions(type) {
4141

4242
errMsg += checkTypes[type].help
4343

44-
return Promise.reject(new errors.SystemError(errMsg));
44+
return Promise.reject(new errors.SystemError({
45+
message: errMsg,
46+
task: task
47+
}));
4548
}).catch((error) => {
4649
if (error instanceof errors.SystemError) {
4750
return Promise.reject(error);
@@ -54,9 +57,10 @@ module.exports = function checkPermissions(type) {
5457
errMsg = `Ghost can't access some files or directories to check for correct permissions.
5558
${checkTypes.folder.help}`;
5659

57-
return Promise.reject(new errors.SystemError({message: errMsg, err: error}));
60+
return Promise.reject(new errors.SystemError({message: errMsg, err: error, task: task}));
5861
}
5962

63+
error.task = task;
6064
return Promise.reject(new errors.ProcessError(error));
6165
});
6266
}

‎lib/commands/doctor/checks/content-folder.js

+4-2
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,11 @@ const path = require('path');
55
const checkPermissions = require('./check-permissions');
66
const shouldUseGhostUser = require('../../../utils/use-ghost-user');
77

8+
const taskTitle = 'Checking content folder ownership';
9+
810
module.exports = {
9-
title: 'Checking content folder ownership',
11+
title: taskTitle,
1012
enabled: () => shouldUseGhostUser(path.join(process.cwd(), 'content')),
11-
task: () => checkPermissions('owner'),
13+
task: () => checkPermissions('owner', taskTitle),
1214
category: ['start', 'update']
1315
};

‎lib/commands/doctor/checks/file-permissions.js

+4-2
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,16 @@
22

33
const checkPermissions = require('./check-permissions');
44

5+
const taskTitle = 'Checking file permissions';
6+
57
module.exports = {
6-
title: 'Checking file permissions',
8+
title: taskTitle,
79
enabled: (ctx) => {
810
const instance = ctx.system.getInstance();
911
const isLocal = instance.process.name === 'local' ? true : false;
1012

1113
return !isLocal;
1214
},
13-
task: () => checkPermissions('files'),
15+
task: () => checkPermissions('files', taskTitle),
1416
category: ['start', 'update']
1517
};

‎lib/commands/doctor/checks/folder-permissions.js

+4-2
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,16 @@
22

33
const checkPermissions = require('./check-permissions');
44

5+
const taskTitle = 'Checking folder permissions';
6+
57
module.exports = {
6-
title: 'Checking folder permissions',
8+
title: taskTitle,
79
enabled: (ctx) => {
810
const instance = ctx.system.getInstance();
911
const isLocal = instance.process.name === 'local' ? true : false;
1012

1113
return !isLocal;
1214
},
13-
task: () => checkPermissions('folder'),
15+
task: () => checkPermissions('folder', taskTitle),
1416
category: ['start', 'update']
1517
};

‎lib/commands/doctor/checks/install-folder-permissions.js

+9-4
Original file line numberDiff line numberDiff line change
@@ -5,21 +5,26 @@ const constants = require('constants');
55
const errors = require('../../../errors');
66
const checkDirectoryAndAbove = require('./check-directory');
77

8+
const taskTitle = 'Checking current folder permissions';
9+
810
function installFolderPermissions(ctx) {
911
return fs.access(process.cwd(), constants.R_OK | constants.W_OK).catch(() => {
10-
return Promise.reject(new errors.SystemError(`The current directory is not writable.
11-
Please fix your directory permissions.`));
12+
return Promise.reject(new errors.SystemError({
13+
message: `The current directory is not writable.
14+
Please fix your directory permissions.`,
15+
task: taskTitle
16+
}));
1217
}).then(() => {
1318
if (ctx.local || !ctx.system.platform.linux || (ctx.argv && ctx.argv['setup-linux-user'] === false)) {
1419
return Promise.resolve();
1520
}
1621

17-
return checkDirectoryAndAbove(process.cwd(), 'run `ghost install`');
22+
return checkDirectoryAndAbove(process.cwd(), 'run `ghost install`', taskTitle);
1823
});
1924
}
2025

2126
module.exports = {
22-
title: 'Checking current folder permissions',
27+
title: taskTitle,
2328
task: installFolderPermissions,
2429
category: ['install', 'update']
2530
};

‎lib/commands/doctor/checks/mysql.js

+7-2
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ const includes = require('lodash/includes');
55

66
const errors = require('../../../errors');
77

8+
const taskTitle = 'Checking for a MySQL installation';
9+
810
function mysqlCheck(ctx, task) {
911
// On ubuntu, mysqld is in `/usr/sbin` but it's not automatically in the PATH of non-root users
1012
// So, we modify the path env var to make things work
@@ -29,13 +31,16 @@ ${chalk.blue('c)')} run ${chalk.cyan('`ghost install local`')} to get a developm
2931
return Promise.resolve();
3032
}
3133

32-
return Promise.reject(new errors.SystemError('MySQL check failed.'));
34+
return Promise.reject(new errors.SystemError({
35+
message: 'MySQL check failed.',
36+
task: taskTitle
37+
}));
3338
});
3439
});
3540
}
3641

3742
module.exports = {
38-
title: 'Checking for a MySQL installation',
43+
title: taskTitle,
3944
task: mysqlCheck,
4045
// Disable this check if:
4146
// a) local install OR

‎lib/commands/doctor/checks/node-version.js

+9-4
Original file line numberDiff line numberDiff line change
@@ -6,23 +6,28 @@ const errors = require('../../../errors');
66
const cliPackage = require('../../../../package');
77
const checkDirectoryAndAbove = require('./check-directory');
88

9+
const taskTitle = 'Checking system Node.js version';
10+
911
function nodeVersion(ctx) {
1012
if (process.env.GHOST_NODE_VERSION_CHECK !== 'false' && !semver.satisfies(process.versions.node, cliPackage.engines.node)) {
11-
return Promise.reject(new errors.SystemError(`${chalk.red('The version of Node.js you are using is not supported.')}
13+
return Promise.reject(new errors.SystemError({
14+
message: `${chalk.red('The version of Node.js you are using is not supported.')}
1215
${chalk.gray('Supported: ')}${cliPackage.engines.node}
1316
${chalk.gray('Installed: ')}${process.versions.node}
14-
See ${chalk.underline.blue('https://docs.ghost.org/v1/docs/supported-node-versions')} for more information`));
17+
See ${chalk.underline.blue('https://docs.ghost.org/v1/docs/supported-node-versions')} for more information`,
18+
task: taskTitle
19+
}));
1520
}
1621

1722
if (ctx.local || !ctx.system.platform.linux || (ctx.argv && ctx.argv['setup-linux-user'] === false)) {
1823
return Promise.resolve();
1924
}
2025

21-
return checkDirectoryAndAbove(process.argv[0], 'install node and Ghost-CLI');
26+
return checkDirectoryAndAbove(process.argv[0], 'install node and Ghost-CLI', taskTitle);
2227
}
2328

2429
module.exports = {
25-
title: 'Checking system Node.js version',
30+
title: taskTitle,
2631
task: nodeVersion,
2732
category: ['install', 'update']
2833
};

‎lib/commands/doctor/checks/system-stack.js

+8-5
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ const execa = require('execa');
44

55
const errors = require('../../../errors');
66

7+
const taskTitle = 'Checking operating system compatibility';
8+
79
function systemStack(ctx, task) {
810
let promise;
911

@@ -52,17 +54,18 @@ For local installs we recommend using \`ghost install local\` instead.`,
5254
return Promise.resolve();
5355
}
5456

55-
return Promise.reject(
56-
new errors.SystemError('System checks failed.')
57-
);
57+
return Promise.reject(new errors.SystemError({
58+
message: `System stack checks failed with message: '${error.message}'`,
59+
task: taskTitle
60+
}));
5861
});
5962
});
6063
}
6164

6265
module.exports = {
63-
title: 'Checking operating system compatibility',
66+
title: taskTitle,
6467
task: systemStack,
6568
enabled: (ctx) => !ctx.local,
66-
skip: (ctx) => ctx.argv && !ctx.argv.stack,
69+
skip: (ctx) => !ctx.isDoctorCommand && ctx.argv && !ctx.argv.stack,
6770
category: ['install']
6871
}

‎lib/commands/doctor/checks/validate-config.js

+7-3
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,16 @@ const errors = require('../../../errors');
88
const Config = require('../../../utils/config');
99
const advancedOptions = require('../../config/advanced');
1010

11+
const taskTitle = 'Validating config';
12+
1113
function validateConfig(ctx) {
1214
const config = Config.exists(path.join(process.cwd(), `config.${ctx.system.environment}.json`));
1315

1416
if (config === false) {
1517
return Promise.reject(new errors.ConfigError({
1618
environment: ctx.system.environment,
17-
message: 'Config file is not valid JSON'
19+
message: 'Config file is not valid JSON',
20+
task: taskTitle
1821
}));
1922
}
2023

@@ -35,15 +38,16 @@ function validateConfig(ctx) {
3538
[key]: value
3639
},
3740
message: validated,
38-
environment: ctx.system.environment
41+
environment: ctx.system.environment,
42+
task: taskTitle
3943
}));
4044
}
4145
});
4246
});
4347
}
4448

4549
module.exports = {
46-
title: 'Validating config',
50+
title: taskTitle,
4751
task: validateConfig,
4852
category: ['start']
4953
}

‎lib/commands/doctor/index.js

+10-12
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ const Command = require('../../command');
33

44
class DoctorCommand extends Command {
55
run(argv) {
6-
const chalk = require('chalk');
76
const includes = require('lodash/includes');
87

98
const checks = require('./checks');
@@ -24,7 +23,10 @@ class DoctorCommand extends Command {
2423

2524
let instance;
2625

27-
if (!argv.skipInstanceCheck) {
26+
if (
27+
!argv.skipInstanceCheck &&
28+
!(argv.categories && argv.categories.length === 1 && argv.categories[0] === 'install')
29+
) {
2830
checkValidInstall('doctor');
2931
instance = this.system.getInstance();
3032
instance.checkEnvironment();
@@ -35,22 +37,18 @@ class DoctorCommand extends Command {
3537
system: this.system,
3638
instance: instance,
3739
ui: this.ui,
38-
local: argv.local || false
40+
local: argv.local || false,
41+
// This is set to true whenever the command is `ghost doctor` itself,
42+
// rather than something like `ghost start` or `ghost update`
43+
isDoctorCommand: Boolean(argv._ && argv._.length && argv._[0] === 'doctor')
3944
};
4045

41-
return this.ui.listr(checksToRun, context).catch((error) => {
42-
if (!argv.quiet) {
43-
this.ui.log('\nChecks failed!', 'red');
44-
this.ui.log(`${chalk.yellow('Message:')} ${error.message}\n`);
45-
}
46-
47-
return Promise.reject(error);
48-
});
46+
return this.ui.listr(checksToRun, context, {exitOnError: false});
4947
}
5048
}
5149

5250
DoctorCommand.description = 'Check the system for any potential hiccups when installing/updating Ghost';
53-
DoctorCommand.longDescription = '$0 doctor [install|startup]\n Run various checks to determine potential problems with your environment.'
51+
DoctorCommand.longDescription = '$0 doctor [categories..]\n Run various checks to determine potential problems with your environment.'
5452
DoctorCommand.params = '[categories..]';
5553
DoctorCommand.global = true;
5654

‎test/unit/commands/doctor/checks/system-stack-spec.js

+6-6
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ describe('Unit: Doctor Checks > systemStack', function () {
3838
expect(false, 'error should have been thrown').to.be.true;
3939
}).catch((error) => {
4040
expect(error).to.be.an.instanceof(errors.SystemError);
41-
expect(error.message).to.equal('System checks failed.');
41+
expect(error.message).to.equal('System stack checks failed with message: \'Operating system is not Linux\'');
4242
expect(execaStub.called).to.be.false;
4343
expect(logStub.calledOnce).to.be.true;
4444
expect(logStub.args[0][0]).to.match(/failed with message/);
@@ -61,7 +61,7 @@ describe('Unit: Doctor Checks > systemStack', function () {
6161
expect(false, 'error should have been thrown').to.be.true;
6262
}).catch((error) => {
6363
expect(error).to.be.an.instanceof(errors.SystemError);
64-
expect(error.message).to.equal('System checks failed.');
64+
expect(error.message).to.equal('System stack checks failed with message: \'Operating system is not Linux\'');
6565
expect(execaStub.called).to.be.false;
6666
expect(logStub.calledOnce).to.be.true;
6767
expect(logStub.args[0][0]).to.match(/failed with message/);
@@ -105,7 +105,7 @@ describe('Unit: Doctor Checks > systemStack', function () {
105105
expect(false, 'error should have been thrown').to.be.true;
106106
}).catch((error) => {
107107
expect(error).to.be.an.instanceof(errors.SystemError);
108-
expect(error.message).to.equal('System checks failed.');
108+
expect(error.message).to.equal('System stack checks failed with message: \'Linux version is not Ubuntu 16\'');
109109
expect(execaStub.calledOnce).to.be.true;
110110
expect(logStub.calledOnce).to.be.true;
111111
expect(logStub.args[0][0]).to.match(/failed with message/);
@@ -128,7 +128,7 @@ describe('Unit: Doctor Checks > systemStack', function () {
128128
expect(false, 'error should have been thrown').to.be.true;
129129
}).catch((error) => {
130130
expect(error).to.be.an.instanceof(errors.SystemError);
131-
expect(error.message).to.equal('System checks failed.');
131+
expect(error.message).to.equal('System stack checks failed with message: \'Linux version is not Ubuntu 16\'');
132132
expect(execaStub.calledOnce).to.be.true;
133133
expect(logStub.calledOnce).to.be.true;
134134
expect(logStub.args[0][0]).to.match(/failed with message/);
@@ -156,7 +156,7 @@ describe('Unit: Doctor Checks > systemStack', function () {
156156
expect(false, 'error should have been thrown').to.be.true;
157157
}).catch((error) => {
158158
expect(error).to.be.an.instanceof(errors.SystemError);
159-
expect(error.message).to.equal('System checks failed.');
159+
expect(error.message).to.equal('System stack checks failed with message: \'Missing package(s): systemd, nginx\'');
160160
expect(execaStub.calledOnce).to.be.true;
161161
expect(listrStub.calledOnce).to.be.true;
162162
expect(listrStub.args[0][2].renderer).to.equal('silent');
@@ -208,7 +208,7 @@ describe('Unit: Doctor Checks > systemStack', function () {
208208
expect(false, 'error should have been thrown').to.be.true;
209209
}).catch((error) => {
210210
expect(error).to.be.an.instanceof(errors.SystemError);
211-
expect(error.message).to.equal('System checks failed.');
211+
expect(error.message).to.equal('System stack checks failed with message: \'Missing package(s): systemd, nginx\'');
212212
expect(execaStub.calledThrice).to.be.true;
213213
expect(logStub.calledOnce).to.be.true;
214214
expect(logStub.args[0][0]).to.match(/failed with message/);

0 commit comments

Comments
 (0)