Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(ssl) improve error messages to log original error #595

Merged
merged 1 commit into from
Feb 2, 2018

Conversation

aileen
Copy link
Member

@aileen aileen commented Jan 24, 2018

refs #587

  • fixes an issue, where failed requests with got wouldn't get catched properly
  • differentiates between ProcessError for stderr and CliError for everything else
  • CliError can be passed the original error and will log the message and stack to the log file, or log in the UI when in verbose mode
  • Adds err property with original error whereever we have it so it'll be passed along and printed in the log file

@aileen aileen force-pushed the improve-errorhandling branch from a2051c1 to f0486aa Compare January 24, 2018 09:37
return Promise.reject(new cli.errors.SystemError(
'Your domain name is not pointing to the correct IP address of your server, please update it and run `ghost setup ssl` again'
));
return Promise.reject(new cli.errors.SystemError({

This comment was marked as abuse.

@@ -27,10 +27,6 @@ function install(ui, task) {
ui.logVerbose('ssl: downloading acme.sh to temporary directory', 'green');
return fs.emptyDir(acmeTmpDir)
}).then(() => got(acmeApiUrl)).then((response) => {
if (response.statusCode !== 200) {

This comment was marked as abuse.

This comment was marked as abuse.

lib/errors.js Outdated
@@ -23,7 +23,10 @@ class CliError extends Error {
Error.captureStackTrace(this, this.constructor);

this.context = options.context || {};
// TODO: Do something with the original error message which we can receive now as the `err` property
this.err = options.err || {};

This comment was marked as abuse.

This comment was marked as abuse.

@aileen aileen force-pushed the improve-errorhandling branch 17 times, most recently from 0cc6821 to 83da6b2 Compare January 25, 2018 12:56
@@ -83,11 +114,16 @@ class ProcessError extends CliError {
output += chalk.yellow(`Exit code: ${this.options.code}\n\n`);
}

if (verbose && this.options.stdout) {

This comment was marked as abuse.

@aileen aileen changed the title [WIP] feat(ssl) improve error messages for acme feat(ssl) improve error messages to log original error Jan 25, 2018
@aileen aileen force-pushed the improve-errorhandling branch from 83da6b2 to 597f67d Compare January 25, 2018 13:05
@TryGhost TryGhost deleted a comment from coveralls Jan 25, 2018
@aileen aileen force-pushed the improve-errorhandling branch from 597f67d to 154c4c3 Compare January 25, 2018 13:15
@aileen aileen requested a review from acburdine January 25, 2018 13:17
@aileen
Copy link
Member Author

aileen commented Jan 25, 2018

@kirrg001 @acburdine I think this one is ready for review!

@@ -10,6 +10,8 @@ const chalk = require('chalk');
*/
class CliError extends Error {
constructor(options) {
const originalError = {};

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

Copy link
Member

@acburdine acburdine left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good from a code standpoint 👍

@kirrg001
Copy link
Contributor

@acburdine Thanks for code review. I'll test that PR later 👍

@@ -53,7 +53,8 @@ module.exports = function resolveVersion(version, update) {
} catch (e) {
return Promise.reject(new errors.CliError({
message: 'Ghost-CLI was unable to load versions from Yarn.',
log: false
log: false,
err: e

This comment was marked as abuse.

refs TryGhost#587

- fixes an issue, where failed requests with `got` wouldn't get catched properly
- differentiates between `ProcessError` for `stderr` and `CliError` for everything else
- `CliError` can be passed the original error and will log the `message` and `stack` to the log file, or log in the UI when in verbose mode
- Adds `err` property with original error whereever we have it so it'll be passed along and printed in the log file
@aileen aileen force-pushed the improve-errorhandling branch from 154c4c3 to aecb1c9 Compare February 2, 2018 09:26
@acburdine acburdine merged commit 509aa5a into TryGhost:master Feb 2, 2018
@aileen aileen deleted the improve-errorhandling branch May 10, 2018 01:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants