Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Log errors for messages that exceed the maximum size #143

Merged
merged 4 commits into from
Jan 11, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions lib/segment/analytics/defaults.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ module Queue
MAX_SIZE = 10000
end

module Message
MAX_BYTES = 32768 # 32Kb
end

module BackoffPolicy
MIN_TIMEOUT_MS = 100
MAX_TIMEOUT_MS = 10000
Expand Down
22 changes: 22 additions & 0 deletions lib/segment/analytics/message.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
require 'segment/analytics/defaults'

module Segment
class Analytics
# Represents a message to be sent to the API
class Message
def initialize(hash)
@hash = hash
end

def too_big?
to_json.bytesize > Defaults::Message::MAX_BYTES
end

# Since the hash is expected to not be modified (set at initialization),
# the JSON version can be cached after the first computation.
def to_json(*args)
@json ||= @hash.to_json(*args)
end
end
end
end
28 changes: 28 additions & 0 deletions lib/segment/analytics/message_batch.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
require 'segment/analytics/logging'

module Segment
class Analytics
# A batch of `Message`s to be sent to the API
class MessageBatch
extend Forwardable
include Segment::Analytics::Logging

def initialize
@messages = []
end

def <<(message)
if message.too_big?
logger.error('a message exceeded the maximum allowed size')
else
@messages << message
end
end

def_delegators :@messages, :to_json
def_delegators :@messages, :clear
def_delegators :@messages, :empty?
def_delegators :@messages, :length
end
end
end
9 changes: 5 additions & 4 deletions lib/segment/analytics/worker.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
require 'segment/analytics/defaults'
require 'segment/analytics/utils'
require 'segment/analytics/defaults'
require 'segment/analytics/message'
require 'segment/analytics/message_batch'
require 'segment/analytics/request'
require 'segment/analytics/utils'

module Segment
class Analytics
Expand All @@ -26,7 +27,7 @@ def initialize(queue, write_key, options = {})
@write_key = write_key
@batch_size = options[:batch_size] || Queue::BATCH_SIZE
@on_error = options[:on_error] || proc { |status, error| }
@batch = []
@batch = MessageBatch.new
@lock = Mutex.new
end

Expand All @@ -38,7 +39,7 @@ def run

@lock.synchronize do
until @batch.length >= @batch_size || @queue.empty?
@batch << @queue.pop
@batch << Message.new(@queue.pop)
end
end

Expand Down
25 changes: 25 additions & 0 deletions spec/segment/analytics/message_batch_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
require 'spec_helper'

module Segment
class Analytics
describe MessageBatch do
describe '#<<' do
subject { described_class.new }

it 'appends messages' do
subject << Message.new('a' => 'b')
expect(subject.length).to eq(1)
end

it 'rejects messages that exceed the maximum allowed size' do
max_bytes = Segment::Analytics::Defaults::Message::MAX_BYTES
hash = { 'a' => 'b' * max_bytes }
message = Message.new(hash)

subject << message
expect(subject.length).to eq(0)
end
end
end
end
end
35 changes: 35 additions & 0 deletions spec/segment/analytics/message_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
require 'spec_helper'

module Segment
class Analytics
describe Message do
describe '#to_json' do
it 'caches JSON conversions' do
# Keeps track of the number of times to_json was called
nested_obj = Class.new do
attr_reader :to_json_call_count

def initialize
@to_json_call_count = 0
end

def to_json(*_)
@to_json_call_count += 1
'{}'
end
end.new

message = Message.new('some_key' => nested_obj)
expect(nested_obj.to_json_call_count).to eq(0)

message.to_json
expect(nested_obj.to_json_call_count).to eq(1)

# When called a second time, the call count shouldn't increase
message.to_json
expect(nested_obj.to_json_call_count).to eq(1)
end
end
end
end
end