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

🐷 Improve config error logs and messages #274

Closed

Conversation

aileen
Copy link
Member

@aileen aileen commented Jul 5, 2017

refs #216

This PR changes the Config.exists() method in a way that rather than simply returning false, we're catching the error and return a ConfigError object that includes the received error object from fs.readJsonSync() as a message.

There are two places, where Config.exists() is being called and was expecting a boolean false to determine if the config is valid. But this can mean, that the config either doesn't exist or has a syntax error. It remained a secret to the user.

I changed this to check for an options property (I'm not very happy with the wording of this, but the CLI uses its own errors and not Ignition and I didn't want to change this within this PR), which than has the message property that includes a better error message which we then can log for the user.

e. g.

A ConfigError occured.

Error detected in the production configuration.

Message: The config.production.json file is missing or not valid JSON
Context: Error: /Users/aileennowak/Code/cli-installs/cli_3/config.production.json: ENOENT: no such file or directory, open '/Users/aileennowak/Code/cli-installs/cli_3/config.production.json'

Debug Information:
    Node Version: v6.11.0
    Ghost-CLI Version: 1.0.0-beta.3
    Environment: production
    Command: 'ghost start'

refs TryGhost#216

This PR changes the `Config.exists()` method in a way that rather than simply returning false, we're catching the error and return a `ConfigError` object that includes the received error object from `fs.readJsonSync()` as a message.

There are two places, where `Config.exists()` is being called and was expecting a boolean `false` to determine if the config is valid. But this can mean, that the config either doesn't exist or has a syntax error. It remained a secret to the user.

I changed this to check for an `options` property (I'm not very happy with the wording of this, but the CLI uses its own errors and not Ignition and I didn't want to change this within this PR), which than has the `message` property that includes a better error message which we then can log for the user.

e. g.

```
A ConfigError occured.

Error detected in the production configuration.

Message: The config.production.json file is missing or not valid JSON
Context: Error: /Users/aileennowak/Code/cli-installs/cli_3/config.production.json: ENOENT: no such file or directory, open '/Users/aileennowak/Code/cli-installs/cli_3/config.production.json'

Debug Information:
    Node Version: v6.11.0
    Ghost-CLI Version: 1.0.0-beta.3
    Environment: production
    Command: 'ghost start'
```
@coveralls
Copy link

Coverage Status

Coverage increased (+0.3%) to 57.457% when pulling 379ac6c on AileenCGN:1.0.0-dev/improve-config-error-message into 3c64dd5 on TryGhost:master.

@kirrg001 kirrg001 assigned acburdine and unassigned kirrg001 Jul 5, 2017
@acburdine
Copy link
Member

Going to close this for now - as per the task in #216 the solution should instead try to point to the specific place in the config where the JSON is invalid.

@acburdine acburdine closed this Jul 13, 2017
@aileen aileen deleted the 1.0.0-dev/improve-config-error-message branch May 10, 2018 01:51
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