From 9cdee8aae3cb7b969583493440469ac0dfea764f Mon Sep 17 00:00:00 2001
From: Tom de Bruijn <tom@tomdebruijn.com>
Date: Tue, 27 Jun 2023 15:57:08 +0200
Subject: [PATCH] Fix issue with invalid breadcrumb metadata (#974)

We had a report of breadcrumbs not working when the metadata was
invalid. This required some manual intervention from us to fix the data.
Avoid this occurring in the future by not allowing any other metadata
than a Hash object.

Closes https://github.com/appsignal/support/issues/269
---
 .../check-argument-type-of-breadcrumb-metadata.md | 15 +++++++++++++++
 lib/appsignal/transaction.rb                      |  6 ++++++
 spec/lib/appsignal/transaction_spec.rb            | 13 +++++++++++++
 3 files changed, 34 insertions(+)
 create mode 100644 .changesets/check-argument-type-of-breadcrumb-metadata.md

diff --git a/.changesets/check-argument-type-of-breadcrumb-metadata.md b/.changesets/check-argument-type-of-breadcrumb-metadata.md
new file mode 100644
index 000000000..501d32b61
--- /dev/null
+++ b/.changesets/check-argument-type-of-breadcrumb-metadata.md
@@ -0,0 +1,15 @@
+---
+bump: "patch"
+type: "fix"
+---
+
+Log error when the argument type of the breadcrumb metadata is invalid. This metadata argument should be a Hash, and other values are not supported. More information can be found in the [Ruby gem breadcrumb documentation](https://docs.appsignal.com/ruby/instrumentation/breadcrumbs.html).
+
+```ruby
+Appsignal.add_breadcrumb(
+  "breadcrumb category",
+  "breadcrumb action",
+  "some message",
+  { :metadata_key => "some value" } # This needs to be a Hash object
+)
+```
diff --git a/lib/appsignal/transaction.rb b/lib/appsignal/transaction.rb
index 402f97648..ff69bf239 100644
--- a/lib/appsignal/transaction.rb
+++ b/lib/appsignal/transaction.rb
@@ -186,6 +186,12 @@ def set_tags(given_tags = {})
     # @see https://docs.appsignal.com/ruby/instrumentation/breadcrumbs.html
     #   Breadcrumb reference
     def add_breadcrumb(category, action, message = "", metadata = {}, time = Time.now.utc)
+      unless metadata.is_a? Hash
+        Appsignal.logger.error "add_breadcrumb: Cannot add breadcrumb. " \
+          "The given metadata argument is not a Hash."
+        return
+      end
+
       @breadcrumbs.push(
         :time => time.to_i,
         :category => category,
diff --git a/spec/lib/appsignal/transaction_spec.rb b/spec/lib/appsignal/transaction_spec.rb
index c825be0f7..c25cd50ee 100644
--- a/spec/lib/appsignal/transaction_spec.rb
+++ b/spec/lib/appsignal/transaction_spec.rb
@@ -432,6 +432,19 @@ def current_transaction
           expect(breadcrumb["metadata"]).to eq({})
         end
       end
+
+      context "with metadata argument that's not a Hash" do
+        it "does not add the breadcrumb and logs and error" do
+          transaction.add_breadcrumb("category", "action", "message", "invalid metadata")
+          transaction.sample_data
+
+          expect(transaction.to_h["sample_data"]["breadcrumbs"]).to be_empty
+          expect(log_contents(log)).to contains_log(
+            :error,
+            "add_breadcrumb: Cannot add breadcrumb. The given metadata argument is not a Hash."
+          )
+        end
+      end
     end
 
     describe "#set_action" do