Move Logger#add_exception to the Buffer and forward from the Logger

lib/rackstash/buffer.rb

@@ -84,6 +84,36 @@ module Rackstash
       clear
     end
 
+    # Extract useful data from an exception and add it to fields of the buffer
+    # for structured logging. The following fields will be set:
+    #
+    #  * `error` - The class name of the exception
+    #  * `error_message` - The exception's message
+    #  * `error_trace` - The backtrace of the exception, one frame per line
+    #
+    # The exception will not be added to the buffer's `message` field.
+    # Log it manually with {#add} if desired.
+    #
+    # By default, the details of subsequent exceptions will overwrite those of
+    # older exceptions in the current buffer. Only by the `force` argument to
+    # `false`, we will preserve existing exceptions.
+    #
+    # @param exception [Exception] an Exception object as catched by `rescue`
+    # @param force [Boolean] set to `false` to preserve the details of an
+    #   existing exception in the current buffer's fields, set to `true` to
+    #   overwrite them.
+    # @return [Exception] the passed `exception`
+    def add_exception(exception, force: true)
+      return exception unless force || @fields[FIELD_ERROR].nil?
+
+      @fields.merge!(
+        FIELD_ERROR => exception.class.name,
+        FIELD_ERROR_MESSAGE => exception.message,
+        FIELD_ERROR_TRACE => (exception.backtrace || []).join("\n")
+      )
+      exception
+    end
+
     # Add a new message to the buffer. This will mark the current buffer as
     # {pending?} and will result in the eventual flush of the logged data.
     #

lib/rackstash/logger.rb

@@ -337,35 +337,9 @@ module Rackstash
     end
     alias log add
 
-    # Extract useful data from an exception and add it to fields of the buffer
+    # (see Buffer#add_exception)
-    # for structured logging. The following fields will be set:
-    #
-    #  * `error` - The class name of the exception
-    #  * `error_message` - The exception's message
-    #  * `error_trace` - The backtrace of the exception, one frame per line
-    #
-    # The exception will not be added to the buffer's `message` field.
-    # Log it manually with {#add} if desired.
-    #
-    # By default, the details of subsequent exceptions will overwrite those of
-    # older exceptions in the current buffer. Only by the `force` argument to
-    # `false`, we will preserve existing exceptions.
-    #
-    # @param exception [Exception] an Exception object as catched by `rescue`
-    # @param force [Boolean] set to `false` to preserve the details of an
-    #   existing exception in the current buffer's fields, set to `true` to
-    #   overwrite them.
-    # @return [Exception] the passed `exception`
     def add_exception(exception, force: true)
-      return exception if !force && buffer.fields[FIELD_ERROR]
+      buffer.add_exception(exception, force: force)
-
-      exception_fields = {
-        FIELD_ERROR => exception.class.name,
-        FIELD_ERROR_MESSAGE => exception.message,
-        FIELD_ERROR_TRACE => (exception.backtrace || []).join("\n")
-      }
-      buffer.fields.merge!(exception_fields)
-      exception
     end
 
     # Create a new buffering {Buffer} and puts in on the {BufferStack} for the

spec/rackstash/buffer_spec.rb

@@ -20,6 +20,64 @@ describe Rackstash::Buffer do
     end
   end
 
+  describe '#add_exception' do
+    it 'adds the exception fields' do
+      begin
+        raise 'My Error'
+      rescue => e
+        buffer.add_exception(e)
+      end
+
+      expect(buffer.fields['error']).to eql 'RuntimeError'
+      expect(buffer.fields['error_message']).to eql 'My Error'
+      expect(buffer.fields['error_trace']).to match %r{\A#{__FILE__}:#{__LINE__ - 7}:in}
+    end
+
+    it 'does not require a backtrace' do
+      buffer.add_exception(StandardError.new('Error'))
+
+      expect(buffer.fields['error']).to eql 'StandardError'
+      expect(buffer.fields['error_message']).to eql 'Error'
+      expect(buffer.fields['error_trace']).to eql ''
+    end
+
+    context 'with force: true' do
+      it 'overwrites exceptions' do
+        begin
+          raise 'Error'
+        rescue => first
+          buffer.add_exception(first, force: true)
+        end
+
+        begin
+          raise TypeError, 'Another Error'
+        rescue => second
+          buffer.add_exception(second, force: true)
+        end
+
+        expect(buffer.fields['error']).to eql 'TypeError'
+        expect(buffer.fields['error_message']).to eql 'Another Error'
+        expect(buffer.fields['error_trace']).to match %r{\A#{__FILE__}:#{__LINE__ - 7}:in}
+      end
+    end
+
+    context 'with force: false' do
+      it 'does not overwrite exceptions' do
+        buffer.fields['error'] = 'Something is wrong'
+
+        begin
+          raise TypeError, 'Error'
+        rescue => second
+          buffer.add_exception(second, force: false)
+        end
+
+        expect(buffer.fields['error']).to eql 'Something is wrong'
+        expect(buffer.fields['error_message']).to be_nil
+        expect(buffer.fields['error_trace']).to be_nil
+      end
+    end
+  end
+
   describe '#add_message' do
     it 'adds a message to the buffer' do
       msg = double(message: 'Hello World', time: Time.now)

spec/rackstash/logger_spec.rb

@@ -507,68 +507,17 @@ describe Rackstash::Logger do
   end
 
   describe '#add_exception' do
-    let(:fields) { Rackstash::Fields::Hash.new }
+    it 'forwards to the buffer' do
-
-    before(:each) do
       buffer = instance_double('Rackstash::Buffer')
-      allow(buffer).to receive(:fields).and_return(fields)
+      expect(logger).to receive(:buffer).and_return(buffer)
-      allow(logger).to receive(:buffer).and_return(buffer)
+      expect(buffer).to receive(:add_exception)
+
+      logger.add_exception(RuntimeError.new)
     end
 
-    it 'adds the exception fields' do
+    it 'implements the same method signature as the Buffer' do
-      begin
+      expect(Rackstash::Buffer.instance_method(:add_exception).parameters)
-        raise 'My Error'
+        .to eql logger.method(:add_exception).parameters
-      rescue => e
-        logger.add_exception(e)
-      end
-
-      expect(fields['error']).to eql 'RuntimeError'
-      expect(fields['error_message']).to eql 'My Error'
-      expect(fields['error_trace']).to match %r{\A#{__FILE__}:#{__LINE__ - 7}:in}
-    end
-
-    it 'does not require a backtrace' do
-      logger.add_exception(StandardError.new('Error'))
-
-      expect(fields['error']).to eql 'StandardError'
-      expect(fields['error_message']).to eql 'Error'
-      expect(fields['error_trace']).to eql ''
-    end
-
-    context 'with force: true' do
-      it 'overwrites exceptions' do
-        begin
-          raise 'Error'
-        rescue => first
-          logger.add_exception(first, force: true)
-        end
-
-        begin
-          raise TypeError, 'Another Error'
-        rescue => second
-          logger.add_exception(second, force: true)
-        end
-
-        expect(fields['error']).to eql 'TypeError'
-        expect(fields['error_message']).to eql 'Another Error'
-        expect(fields['error_trace']).to match %r{\A#{__FILE__}:#{__LINE__ - 7}:in}
-      end
-    end
-
-    context 'with force: false' do
-      it 'does not overwrite exceptions' do
-        fields['error'] = 'Something is wrong'
-
-        begin
-          raise TypeError, 'Error'
-        rescue => second
-          logger.add_exception(second, force: false)
-        end
-
-        expect(fields['error']).to eql 'Something is wrong'
-        expect(fields['error_message']).to be_nil
-        expect(fields['error_trace']).to be_nil
-      end
     end
   end