-
Notifications
You must be signed in to change notification settings - Fork 73
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
Global error handler #73
Changes from 6 commits
9202e26
4d7a0a4
e920ab9
8fe689d
f545388
7a999ff
3784ac1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,5 @@ | ||
require '3scale_toolbox/cli/error_handler' | ||
|
||
module ThreeScaleToolbox::CLI | ||
def self.root_command | ||
ThreeScaleToolbox::Commands::ThreeScaleCommand | ||
|
@@ -11,10 +13,30 @@ def self.load_builtin_commands | |
ThreeScaleToolbox::Commands::BUILTIN_COMMANDS.each(&method(:add_command)) | ||
end | ||
|
||
def self.install_signal_handlers | ||
# Set exit handler | ||
%w[INT TERM].each do |signal| | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is just moving this from one module from another. It does not address the fact that this is global action that is for example not reverted back. IMO this class does not have one reason to change. It does error handling but also installs signal handlers for an interruption. Those are two different concepts that should be separate.
|
||
Signal.trap(signal) do | ||
puts | ||
exit!(0) | ||
end | ||
end | ||
|
||
# Set stack trace dump handler | ||
if !defined?(RUBY_ENGINE) || RUBY_ENGINE != 'jruby' | ||
Signal.trap('USR1') do | ||
puts 'Caught USR1; dumping a stack trace' | ||
puts caller.map { |i| " #{i}" }.join("\n") | ||
end | ||
end | ||
end | ||
|
||
def self.run(args) | ||
load_builtin_commands | ||
ThreeScaleToolbox.load_plugins | ||
# TODO handle error | ||
root_command.build_command.run args | ||
install_signal_handlers | ||
ErrorHandler.error_watchdog do | ||
load_builtin_commands | ||
ThreeScaleToolbox.load_plugins | ||
root_command.build_command.run args | ||
end | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,122 @@ | ||
require 'json' | ||
|
||
module ThreeScaleToolbox | ||
module CLI | ||
class ErrorHandler | ||
def self.error_watchdog | ||
new.error_watchdog { yield } | ||
end | ||
|
||
# Catches errors and prints nice diagnostic messages | ||
def error_watchdog | ||
error = false | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not? def error_watchdog
yield
false
rescue StandardError, ScriptError => err
handle_error err
return err
end |
||
begin | ||
# Run | ||
yield | ||
rescue StandardError, ScriptError => e | ||
handle_error e | ||
error = true | ||
end | ||
error | ||
end | ||
|
||
private | ||
|
||
def handle_error(error) | ||
if expected_error?(error) | ||
warn | ||
warn "\e[1m\e[31mError: #{error.message}\e[0m" | ||
else | ||
print_error(error) | ||
end | ||
end | ||
|
||
def expected_error?(error) | ||
case error | ||
when ThreeScaleToolbox::Error | ||
true | ||
else | ||
false | ||
end | ||
end | ||
|
||
def print_error(error) | ||
write_error(error, $stderr) | ||
|
||
File.open('crash.log', 'w') do |io| | ||
write_verbose_error(error, io) | ||
end | ||
|
||
write_section_header($stderr, 'Detailed information') | ||
warn | ||
warn 'A detailed crash log has been written to ./crash.log.' | ||
end | ||
|
||
def write_error(error, stream) | ||
write_error_message(error, stream) | ||
write_stack_trace(error, stream) | ||
end | ||
|
||
def write_error_message(error, stream) | ||
write_section_header(stream, 'Message') | ||
stream.puts "\e[1m\e[31m#{error.class}: #{error.message}\e[0m" | ||
end | ||
|
||
def write_stack_trace(error, stream) | ||
write_section_header(stream, 'Backtrace') | ||
stream.puts error.backtrace | ||
end | ||
|
||
def write_version_information(stream) | ||
write_section_header(stream, 'Version Information') | ||
stream.puts ThreeScaleToolbox::VERSION | ||
end | ||
|
||
def write_system_information(stream) | ||
write_section_header(stream, 'System Information') | ||
stream.puts Etc.uname.to_json | ||
end | ||
|
||
def write_installed_gems(stream) | ||
write_section_header(stream, 'Installed gems') | ||
gems_and_versions.each do |g| | ||
stream.puts " #{g.first} #{g.last.join(', ')}" | ||
end | ||
end | ||
|
||
def write_load_paths(stream) | ||
write_section_header(stream, 'Load paths') | ||
$LOAD_PATH.each_with_index do |i, index| | ||
stream.puts " #{index}. #{i}" | ||
end | ||
end | ||
|
||
def write_verbose_error(error, stream) | ||
stream.puts "Crashlog created at #{Time.now}" | ||
|
||
write_error_message(error, stream) | ||
write_stack_trace(error, stream) | ||
write_version_information(stream) | ||
write_system_information(stream) | ||
write_installed_gems(stream) | ||
write_load_paths(stream) | ||
end | ||
|
||
def gems_and_versions | ||
gems = {} | ||
Gem::Specification.find_all.sort_by { |s| [s.name, s.version] }.each do |spec| | ||
gems[spec.name] ||= [] | ||
gems[spec.name] << spec.version.to_s | ||
end | ||
gems | ||
end | ||
|
||
def write_section_header(stream, title) | ||
stream.puts | ||
|
||
stream.puts "===== #{title.upcase}:" | ||
stream.puts | ||
end | ||
end | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,55 @@ | ||
require '3scale_toolbox' | ||
|
||
RSpec.describe ThreeScaleToolbox::CLI::ErrorHandler do | ||
include_context :temp_dir | ||
|
||
context '#error_watchdog' do | ||
def raise_runtime_error | ||
raise 'some error' | ||
end | ||
|
||
def raise_toolbox_error | ||
raise ThreeScaleToolbox::Error, 'some error' | ||
end | ||
|
||
context 'raises expected error' do | ||
it 'error is shown on stderr' do | ||
Dir.chdir(tmp_dir) do | ||
expect do | ||
subject.error_watchdog { raise_toolbox_error } | ||
end.to output(/some error/).to_stderr | ||
expect(File).not_to exist('crash.log') | ||
end | ||
end | ||
|
||
it 'returns true' do | ||
expect( | ||
subject.error_watchdog { raise_toolbox_error } | ||
).to be_truthy | ||
end | ||
end | ||
|
||
context 'raises unexpected error' do | ||
it 'crash.log is generated' do | ||
Dir.chdir(tmp_dir) do | ||
expect do | ||
subject.error_watchdog { raise_runtime_error } | ||
end.to output(/some error/).to_stderr | ||
expect(File).to exist('crash.log') | ||
end | ||
end | ||
|
||
it 'returns true' do | ||
expect( | ||
subject.error_watchdog { raise_runtime_error } | ||
).to be_truthy | ||
end | ||
end | ||
|
||
context 'Does not raise error' do | ||
it 'returns true' do | ||
expect(subject.error_watchdog {}).to be_falsey | ||
end | ||
end | ||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems a bit odd to signal error with truthy return value.