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

Support Verbose Errors and Logs #322

Merged
merged 8 commits into from
Mar 26, 2019

Conversation

charlespierce
Copy link
Contributor

@charlespierce charlespierce commented Mar 25, 2019

Info

  • As part of improving errors, we want to support the --verbose flag to print additional error details.
  • We also want to log those additional details, when we have them, regardless of the existence of the --verbose flag, so that users can access the more involved error details.

Changes

  • Added an ErrorReporter type, responsible for composing the error message, additional details, and error log.
  • Restructured the style module to only be responsible for the styling of the error messages, not the composition of various pieces.
  • Updated the two binaries (main.rs and shim.rs) to use ErrorReporter for reporting any errors that are encountered.

Tested

  • Added acceptance tests for the verbose flag and error logs.
  • Manually tested that the verbose flag outputs as expected, and the error logs are written.

Notes

  • The ErrorReporter is configured to treat the NOTION_DEV environment variable the same as the --verbose flag.
  • Currently, when an error log is written, only the file name is outputted to the console, not the full path. All of the logs are written to NOTION_HOME/log/, so we will need to document that fact for users, or switch to outputting the entire path (could be long, especially on windows).
  • The error log is only written if there is an underlying error, so that informational errors thrown directly from within Notion don't cause an unnecessary error log to be created.

Screenshots

Concise error with underlying cause

Concise error with underlying cause

Verbose error with underlying cause

Verbose error with underlying cause

Copy link
Collaborator

@dherman dherman left a comment

Choose a reason for hiding this comment

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

Looks like a nice approach! I made a few small style suggestions, which are up to you. There's one correctness issue but you can file a followup issue.

crates/notion-core/src/error/reporter.rs Show resolved Hide resolved
src/main.rs Outdated Show resolved Hide resolved
crates/notion-core/Cargo.toml Outdated Show resolved Hide resolved
@charlespierce charlespierce merged commit dbaddee into volta-cli:master Mar 26, 2019
@charlespierce charlespierce deleted the verbose_errors branch March 26, 2019 15:28
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.

2 participants