FIX: Make notify work with proper ruby keyword arguments (#498)

* FIX: Make notify work with proper ruby keyword arguments

Fixes #426

* Add tests for the various permutations of hashes and kwargs

* Fix docs

Removed the hash braces in the second example to clarify that kwargs are fine

* Merge kwargs earlier

---------

Co-authored-by: Joshua Wood <josh@joshuawood.net>

CHANGELOG.md

@@ -11,6 +11,7 @@ adheres to [Semantic Versioning](http://semver.org/).
 
 ### Fixed
 - `Honeybadger::Config#respond_to?` would always return true (#490)
+- `Honeybadger::Agent#notify` takes keyword arguments instead of an options hash now (#498)
 
 ### Changed
 - Accept three arguments for the Sidekiq error handler (#495)

lib/honeybadger/agent.rb

@@ -91,15 +91,16 @@ def initialize(opts = {})
     #   end
     #
     #   # Custom notification:
-    #   Honeybadger.notify('Something went wrong.', {
+    #   Honeybadger.notify('Something went wrong.',
     #     error_class: 'MyClass',
     #     context: {my_data: 'value'}
-    #   }) # => '06220c5a-b471-41e5-baeb-de247da45a56'
+    #   ) # => '06220c5a-b471-41e5-baeb-de247da45a56'
     #
     # @param [Exception, Hash, Object] exception_or_opts An Exception object,
     #   or a Hash of options which is used to build the notice. All other types
     #   of objects will be converted to a String and used as the :error_message.
     # @param [Hash] opts The options Hash when the first argument is an Exception.
+    # @param [Hash] kwargs options as keyword args.
     #
     # @option opts [String]    :error_message The error message.
     # @option opts [String]    :error_class ('Notice') The class name of the error.
@@ -118,17 +119,17 @@ def initialize(opts = {})
     #
     # @return [String] UUID reference to the notice within Honeybadger.
     # @return [false] when ignored.
-    def notify(exception_or_opts, opts = {})
+    def notify(exception_or_opts = nil, opts = {}, **kwargs)
       opts = opts.dup
+      opts.merge!(kwargs)
 
       if exception_or_opts.is_a?(Exception)
         already_reported_notice_id = exception_or_opts.instance_variable_get(:@__hb_notice_id)
         return already_reported_notice_id if already_reported_notice_id
-
         opts[:exception] = exception_or_opts
       elsif exception_or_opts.respond_to?(:to_hash)
         opts.merge!(exception_or_opts.to_hash)
-      else
+      elsif !exception_or_opts.nil?
         opts[:error_message] = exception_or_opts.to_s
       end
 

lib/honeybadger/singleton.rb

@@ -51,10 +51,10 @@ module Honeybadger
   # @!method notify(...)
   # Forwards to {Agent.instance}.
   # @see Agent#notify
-  def notify(exception_or_opts, opts = {})
+  def notify(exception_or_opts=nil, opts = {}, **kwargs)
     # Note this is defined directly (instead of via forwardable) so that
     # generated stack traces work as expected.
-    Agent.instance.notify(exception_or_opts, opts)
+    Agent.instance.notify(exception_or_opts, opts, **kwargs)
   end
 
   # @api private

spec/unit/honeybadger/agent_spec.rb

@@ -102,10 +102,45 @@
       opts = {error_message: 'test'}
       prev = opts.dup
       instance = described_class.new(Honeybadger::Config.new(api_key: "fake api key", logger: NULL_LOGGER))
-      instance.notify("test", opts)
+      instance.notify('test', opts)
       expect(prev).to eq(opts)
     end
 
+    it "does take keyword arguments" do
+      opts = {error_message: 'test'}
+      config = Honeybadger::Config.new(api_key:'fake api key', logger: NULL_LOGGER)
+      instance = described_class.new(config)
+
+      expect(instance.worker).to receive(:push) do |notice|
+        expect(notice.error_message).to match('test')
+      end
+      instance.notify(**opts)
+    end
+
+    it "does take keyword arguments as second argument" do
+      opts = {tags: 'testing, kwargs'}
+      config = Honeybadger::Config.new(api_key:'fake api key', logger: NULL_LOGGER)
+      instance = described_class.new(config)
+
+      expect(instance.worker).to receive(:push) do |notice|
+        expect(notice.error_message).to match('test')
+        expect(notice.tags).to eq(['testing', 'kwargs'])
+      end
+      instance.notify('test', **opts)
+    end
+
+    it "does take explicit hash as second argument" do
+      opts = {tags: 'testing, hash'}
+      config = Honeybadger::Config.new(api_key:'fake api key', logger: NULL_LOGGER)
+      instance = described_class.new(config)
+
+      expect(instance.worker).to receive(:push) do |notice|
+        expect(notice.error_message).to match('test')
+        expect(notice.tags).to eq(['testing', 'hash'])
+      end
+      instance.notify('test', opts)
+    end
+
     it "does not report an already reported exception" do
       instance = described_class.new(Honeybadger::Config.new(api_key: "fake api key", logger: NULL_LOGGER))
       exception = RuntimeError.new