From f37b8c9e3e68389fb87dec8441c165d2f0913793 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Tue, 10 Sep 2024 17:12:31 -0300 Subject: [PATCH] Remove check for msg_serial when converting protocol message to JSON MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- lib/ably/models/protocol_message.rb | 1 - spec/unit/models/protocol_message_spec.rb | 8 -------- 2 files changed, 9 deletions(-) diff --git a/lib/ably/models/protocol_message.rb b/lib/ably/models/protocol_message.rb index 0088fded..19ac1662 100644 --- a/lib/ably/models/protocol_message.rb +++ b/lib/ably/models/protocol_message.rb @@ -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 diff --git a/spec/unit/models/protocol_message_spec.rb b/spec/unit/models/protocol_message_spec.rb index b279b3c5..32c74cbc 100644 --- a/spec/unit/models/protocol_message_spec.rb +++ b/spec/unit/models/protocol_message_spec.rb @@ -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) }) }