Skip to content

Commit

Permalink
Remove check for msg_serial when converting protocol message to JSON
Browse files Browse the repository at this point in the history
This check is currently failing when calling as_json on an incoming
MESSAGE protocol message — for example, in the case where you receive a
message whilst using debug-level logging.

I’m not sure what was the intention of this check, which dates back to
523a4b1 (there, the error message is "(…) cannot generate valid JSON for
ProtocolMessage"). It’s not clear to me why a msgSerial would be
required in order to serialise a protocol message; I can only guess it
was intended as some sort of a business logic sense check on outgoing
messages.

Perhaps, back then, it was the case that incoming MESSAGE protocol
messages contained a msgSerial, hence causing this check to succeed, but
this is certainly no longer the case (even on protocol v1), nor can I
see any good reason why it should be.

Until aaa6211, this check could be (and was) satisfied by the presence
of the connectionSerial attribute in the protocol message. That commit
removed all references to connectionSerial and hence this check started
failing.

Given that there doesn’t seem to be a good reason for this check, remove
it and hope that it wasn’t doing anything important.

Resolves #436.
  • Loading branch information
lawrence-forooghian committed Sep 10, 2024
1 parent 5a9e7bc commit f37b8c9
Show file tree
Hide file tree
Showing 2 changed files with 0 additions and 9 deletions.
1 change: 0 additions & 1 deletion lib/ably/models/protocol_message.rb
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,6 @@ def attributes
# Return a JSON ready object from the underlying #attributes using Ably naming conventions for keys
def as_json(*args)
raise TypeError, ':action is missing, cannot generate a valid Hash for ProtocolMessage' unless action
raise TypeError, ':msg_serial is missing, cannot generate a valid Hash for ProtocolMessage' if ack_required? && !has_message_serial?

attributes.dup.tap do |hash_object|
hash_object['action'] = action.to_i
Expand Down
8 changes: 0 additions & 8 deletions spec/unit/models/protocol_message_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -397,14 +397,6 @@ def new_protocol_message(options)
end
end

context 'with missing msg_serial for ack message' do
let(:model) { new_protocol_message({ :action => message_action }) }

it 'it raises an exception' do
expect { model.to_json }.to raise_error TypeError, /msg_serial.*missing/
end
end

context 'is aliased by #to_s' do
let(:model) { new_protocol_message({ :action => attached_action, :channelSerial => 'unique', messages: [message1, message2, message3], :timestamp => as_since_epoch(Time.now) }) }

Expand Down

0 comments on commit f37b8c9

Please sign in to comment.