Raise on invalid timestamps instead of silently dropping them (#144)

Previously track(), track_anonymous(), and track_delivery_metric() silently discarded invalid timestamps (milliseconds, strings, Time objects). This was inconsistent with track_push_notification_event which raises ParamError. Now all methods raise ParamError with a descriptive message when an invalid timestamp is provided.

Author: hownowstephen

lib/customerio/client.rb

@@ -141,6 +141,7 @@ def track_delivery_metric(metric_name, attributes = {})
       raise ParamError, "delivery_id must be a non-empty string" if empty?(attributes[:delivery_id])
 
       body = { delivery_id: attributes[:delivery_id], metric: metric_name }
+      validate_timestamp!(attributes[:timestamp])
       body[:timestamp] = attributes[:timestamp] if valid_timestamp?(attributes[:timestamp])
       body[:recipient] = attributes[:recipient] unless empty?(attributes[:recipient])
       body[:reason] = attributes[:reason] unless empty?(attributes[:reason])
@@ -271,8 +272,11 @@ def create_pageview_event(customer_id, page, attributes = {})
 
     def create_event(url:, event_name:, anonymous_id: nil, event_type: nil, attributes: {}, id: nil, timestamp: nil) # rubocop:disable Metrics/ParameterLists
       body = { name: event_name, data: attributes }
-      body[:timestamp] = timestamp if valid_timestamp?(timestamp)
-      body[:timestamp] = attributes[:timestamp] if body[:timestamp].nil? && valid_timestamp?(attributes[:timestamp])
+      effective_timestamp = timestamp || attributes[:timestamp]
+      if effective_timestamp
+        validate_timestamp!(effective_timestamp)
+        body[:timestamp] = effective_timestamp
+      end
       body[:anonymous_id] = anonymous_id unless empty?(anonymous_id)
       body[:type] = event_type unless empty?(event_type)
       body[:id] = id unless empty?(id)
@@ -284,6 +288,13 @@ def valid_timestamp?(timestamp)
       timestamp.is_a?(Integer) && timestamp > 999_999_999 && timestamp < 100_000_000_000
     end
 
+    def validate_timestamp!(timestamp)
+      return if timestamp.nil?
+      return if valid_timestamp?(timestamp)
+
+      raise ParamError, "timestamp must be a valid integer (seconds since epoch, 10-digit range)"
+    end
+
     def empty?(val)
       val.nil? || (val.is_a?(String) && val.strip.empty?)
     end

spec/client_spec.rb

@@ -433,49 +433,22 @@ def json(data)
       client.track(5, "purchase", {type: "socks", price: "13.99", timestamp: 1561231234})
     end
 
-    it "doesn't send timestamp if timestamp is in milliseconds" do
-      stub_request(:post, api_uri('/api/v1/customers/5/events')).
-        with(body: json({
-          name: "purchase",
-          data: {
-            type: "socks",
-            price: "13.99"
-          }
-        })).
-        to_return(status: 200, body: "", headers: {})
-
-      client.track(5, "purchase", {type: "socks", price: "13.99"}, timestamp: 1561231234000)
+    it "raises an error if timestamp is in milliseconds" do
+      lambda {
+        client.track(5, "purchase", {type: "socks", price: "13.99"}, timestamp: 1561231234000)
+      }.should raise_error(Customerio::Client::ParamError, /timestamp must be a valid integer/)
     end
 
-    it "doesn't send timestamp if timestamp is a date" do
-      date = Time.now
-
-      stub_request(:post, api_uri('/api/v1/customers/5/events')).
-        with(body: {
-          name: "purchase",
-          data: {
-            type: "socks",
-            price: "13.99"
-          }
-        }).
-        to_return(status: 200, body: "", headers: {})
-
-      client.track(5, "purchase", {type: "socks", price: "13.99"}, timestamp: date)
+    it "raises an error if timestamp is a date" do
+      lambda {
+        client.track(5, "purchase", {type: "socks", price: "13.99"}, timestamp: Time.now)
+      }.should raise_error(Customerio::Client::ParamError, /timestamp must be a valid integer/)
     end
 
-    it "doesn't send timestamp if timestamp isn't an integer" do
-      stub_request(:post, api_uri('/api/v1/customers/5/events')).
-        with(body: json({
-          name: "purchase",
-          data: {
-            type: "socks",
-            price: "13.99"
-          }
-        })).
-
-        to_return(status: 200, body: "", headers: {})
-
-      client.track(5, "purchase", {type: "socks", price: "13.99"}, timestamp: "Hello world")
+    it "raises an error if timestamp isn't an integer" do
+      lambda {
+        client.track(5, "purchase", {type: "socks", price: "13.99"}, timestamp: "Hello world")
+      }.should raise_error(Customerio::Client::ParamError, /timestamp must be a valid integer/)
     end
 
     it "sends an event id for deduplication when provided" do
@@ -845,22 +818,13 @@ def json(data)
       })
     end
 
-    it "omits timestamp when not a valid integer" do
-      stub_request(:post, api_uri("/api/v1/metrics")).
-        with(
-          :body => json({
-            :delivery_id => "abc123",
-            :metric => "delivered"
-          }),
-          :headers => {
-            "Content-Type" => "application/json"
-          }).
-        to_return(:status => 200, :body => "", :headers => {})
-
-      client.track_delivery_metric("delivered", {
-        :delivery_id => "abc123",
-        :timestamp => "not-a-timestamp"
-      })
+    it "raises an error when timestamp is not a valid integer" do
+      lambda {
+        client.track_delivery_metric("delivered", {
+          :delivery_id => "abc123",
+          :timestamp => "not-a-timestamp"
+        })
+      }.should raise_error(Customerio::Client::ParamError, /timestamp must be a valid integer/)
     end
 
     it "should raise if metric_name is invalid" do