From e4a006cfb2a2ecbab2f742b6e9f9c8e9b8958430 Mon Sep 17 00:00:00 2001
From: Jan Krutisch <jan@krutisch.de>
Date: Thu, 19 Oct 2023 23:35:43 +0200
Subject: [PATCH] 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                        |  1 +
 lib/honeybadger/agent.rb            | 11 +++++----
 lib/honeybadger/singleton.rb        |  4 ++--
 spec/unit/honeybadger/agent_spec.rb | 37 ++++++++++++++++++++++++++++-
 4 files changed, 45 insertions(+), 8 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 10a76ff8..88d7367b 100644
--- a/CHANGELOG.md
+++ b/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)
diff --git a/lib/honeybadger/agent.rb b/lib/honeybadger/agent.rb
index 5700c61e..fd2e08cf 100644
--- a/lib/honeybadger/agent.rb
+++ b/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
 
diff --git a/lib/honeybadger/singleton.rb b/lib/honeybadger/singleton.rb
index 53a57a60..dc4f0161 100644
--- a/lib/honeybadger/singleton.rb
+++ b/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
diff --git a/spec/unit/honeybadger/agent_spec.rb b/spec/unit/honeybadger/agent_spec.rb
index da18a88a..6f78f29c 100644
--- a/spec/unit/honeybadger/agent_spec.rb
+++ b/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