Skip to content

Commit

Permalink
Merge pull request #103 from cablett/cablett-everything-structured-log
Browse files Browse the repository at this point in the history
Catch fatal errors and output as structured
  • Loading branch information
tpitale committed Feb 11, 2020
2 parents 7f5d4be + 8eec3ac commit 135a794
Show file tree
Hide file tree
Showing 6 changed files with 101 additions and 3 deletions.
1 change: 1 addition & 0 deletions lib/mail_room.rb
Expand Up @@ -13,3 +13,4 @@ module MailRoom
require "mail_room/coordinator"
require "mail_room/cli"
require 'mail_room/logger/structured'
require 'mail_room/crash_handler'
11 changes: 9 additions & 2 deletions lib/mail_room/cli.rb
Expand Up @@ -2,14 +2,14 @@ module MailRoom
# The CLI parses ARGV into configuration to start the coordinator with.
# @author Tony Pitale
class CLI
attr_accessor :configuration, :coordinator
attr_accessor :configuration, :coordinator, :options

# Initialize a new CLI instance to handle option parsing from arguments
# into configuration to start the coordinator running on all mailboxes
#
# @param args [Array] `ARGV` passed from `bin/mail_room`
def initialize(args)
options = {}
@options = {}

OptionParser.new do |parser|
parser.banner = [
Expand All @@ -25,6 +25,10 @@ def initialize(args)
options[:quiet] = true
end

parser.on('--log-exit-as') do |format|
options[:exit_error_format] = 'json' unless format.nil?
end

# parser.on("-l", "--log FILE") do |path|
# options[:log_path] = path
# end
Expand All @@ -50,6 +54,9 @@ def start
end

coordinator.run
rescue Exception => e # not just Errors, but includes lower-level Exceptions
CrashHandler.new(error: e, format: @options[:exit_error_format]).handle
exit
end
end
end
29 changes: 29 additions & 0 deletions lib/mail_room/crash_handler.rb
@@ -0,0 +1,29 @@

module MailRoom
class CrashHandler

attr_reader :error, :format

SUPPORTED_FORMATS = %w[json none]

def initialize(error:, format:)
@error = error
@format = format
end

def handle
if format == 'json'
puts json
return
end

raise error
end

private

def json
{ time: Time.now, severity: :fatal, message: error.message, backtrace: error.backtrace }.to_json
end
end
end
20 changes: 20 additions & 0 deletions spec/lib/cli_spec.rb
Expand Up @@ -37,5 +37,25 @@

expect(coordinator).to have_received(:run)
end

context 'on error' do
let(:error_message) { "oh noes!" }
let(:coordinator) { OpenStruct.new(run: true, quit: true) }

before do
cli.instance_variable_set(:@options, {exit_error_format: error_format})
coordinator.stubs(:run).raises(RuntimeError, error_message)
end

context 'json format provided' do
let(:error_format) { 'json' }

it 'passes onto CrashHandler' do
cli.start

expect(MailRoom::CrashHandler).to have_received(:new).with a_hash_including({format: error_format})
end
end
end
end
end
41 changes: 41 additions & 0 deletions spec/lib/crash_handler_spec.rb
@@ -0,0 +1,41 @@
require 'spec_helper'

describe MailRoom::CrashHandler do

let(:error_message) { "oh noes!" }
let(:error) { RuntimeError.new(error_message) }

describe '#handle' do

subject{ described_class.new(error: error, format: format) }

context 'when given a json format' do
let(:format) { 'json' }
let(:fake_json) do
{ message: error_message }.to_json
end

it 'outputs the result of json to stdout' do
subject.stubs(:json).returns(fake_json)

expect{ subject.handle }.to output(/\"message\":\"#{error_message}\"/).to_stdout
end
end

context 'when given a blank format' do
let(:format) { "" }

it 'raises an error as designed' do
expect{ subject.handle }.to raise_error(error.class, error_message)
end
end

context 'when given a nonexistent format' do
let(:format) { "nonsense" }

it 'raises an error as designed' do
expect{ subject.handle }.to raise_error(error.class, error_message)
end
end
end
end
2 changes: 1 addition & 1 deletion spec/lib/logger/structured_spec.rb
Expand Up @@ -21,7 +21,7 @@
expect { subject.unknown(message) }.to output(json_matching("ANY", message)).to_stdout_from_any_process
end

it 'only accepts strings' do
it 'only accepts hashes' do
expect { subject.unknown("just a string!") }.to raise_error(ArgumentError, /must be a Hash/)
end

Expand Down

0 comments on commit 135a794

Please sign in to comment.