ruby-hyperloop / hyper-mesh

The project has moved to Hyperstack!! - Synchronization of active record models across multiple clients using Pusher, ActionCable, or Polling
https://hyperstack.org/
MIT License
22 stars 12 forks source link

Hypermesh sometimes nullifies STI type on save #67

Closed sfcgeorge closed 6 years ago

sfcgeorge commented 6 years ago

Eek. Just looked like hyper-mesh deleted a record, but actually the record just became invisible to Rails. For some reason Hyper-mesh/react must have thought the type field changed to nil and saved as such. Rails automatically adds to all queries like WHERE type = 'Candidate' so the record effectively dissappears. It is also of course a data loss bug as you no longer know what the record's STI type is, but it's not such a bad data loss bug, everything else is still there.

This is the first time I've seen it happen, and I've been using Hyperloop a fair while now so it's not so easy to reproduce.

I'm going to ensure it doesn't happen in our app again by adding a not null DB constraint (which it should already have had).

The save that blanked the type

{"operation":"ReactiveRecord::Operations::Save", "params":{"models":[{"id":5512, "model":"Candidate", "attributes":{"id":24, "first_name":"kjkljklj", "dob":"1968-01-01", "phone_number":"07778888888", "address":"ljk", "city":";lj", "postcode":"klj", "type":null, "force_validate":true}, "vector":["Candidate", ["find_by_id", 24]]}], "associations":[], "validate":true}}
{"response":{"success":true,"saved_models":[[5512,"Candidate",{"id":24,"email":"testerman@fancysite.net","encrypted_password":"$2a$10$EZ4wKqttdlclWtqgSjiGGOqBwo99MZ5dOL/gkk9BFcqvTyZP172RG","reset_password_token":null,"reset_password_sent_at":null,"remember_created_at":null,"sign_in_count":7,"current_sign_in_at":"2018-02-16T23:57:40.346+00:00","last_sign_in_at":"2018-02-16T23:57:40.052+00:00","current_sign_in_ip":"127.0.0.1","last_sign_in_ip":"127.0.0.1","confirmation_token":"wvx8phk9sB6o1FRDYNCV","confirmed_at":null,"confirmation_sent_at":"2018-02-15T16:14:37.628+00:00","unconfirmed_email":null,"created_at":"2018-02-15T16:14:37.627+00:00","updated_at":"2018-02-20T14:03:48.336+00:00","first_name":"kjkljklj","last_name":"Poultersmith","sex":null,"phone_number":"+447774568888","address":"ljk","address2":null,"city":";lj","postcode":"klj","status":"inactive","profile_step":"add_cv","dob":"1968-01-01","type":null,"original_id":null,"info_sources":null,"stripe_id":null,"utm_medium":"Job_Posting","utm_source":"Uni_Job_Board","utm_campaign":"First+Base+Employment+Ltd","utm_content":"Software+Development+Engineers","utm_term":"University+of+Leicester","controller_whitelist":""},null]]}}
catmando commented 6 years ago

@sfcgeorge sounds like you have figured out more about this... at some point can we have a conversation about what you know... so we can fix.

sfcgeorge commented 6 years ago

[from chat] Sometimes the type field gets set to nil, so Hyperloop thinks it has changed (it shouldn't have) so is sent on the wire, and once in Rails saves fail, queries fail etc.

I added a NOT NULL constraint on the DB to protect the data, but that obviously doesn't stop the crashed. Then I monkyepatched Hyperloop to not change type to nil. It gets loaded fine in the first place, then can become nil for whatever reason, so the patch prevents that and seems to have mitigated the issue. Adding a puts shows type is set to nil several times around fetches.

  module ReactiveRecord
    class Base
      def dont_update_attribute?(attribute, value)
        return true if attribute == :type && value.blank? # <-- patch
        return false if attributes[attribute].is_a?(DummyValue)
        return false unless attributes.key?(attribute)
        return false if attributes[attribute] != value
        true
      end
    end

Note: this is treating the symptom not the cause, I don't know why Hyperloop thinks the type has changed to nil in the first place, I'm just preventing that change from being stored.

adamcreekroad commented 6 years ago

Well I believe this might be the culprit (or at least just where the nilling is happening)

# lib/reactive_record/active_record/instance_methods.rb
module ActiveRecord
  module InstanceRecords
    def type
      @backing_record.reactive_get!(:type, nil)
    end

    def type=(val)
      @backing_record.reactive_set!(:type, backing_record.convert(:type, val))
    end
  end
end

Commenting out this code gets rid of the problem, but what are these two methods actually doing @catmando ?

Edit: Commenting out that code only somehow made it happen less frequently? So nevermind...

adamcreekroad commented 6 years ago

Actually it looks like this is the cause:

# lib/reactive-record/active-record/class_methods.rb
module ActiveRecord
  module ClassMethods
    def _react_param_conversion(param, opt = nil)
      ...

      if value
        [assoc.attribute, {id: [value], type: [nil]}]
      else
        [assoc.attribute, [nil]]
      end
    end
  end
end

I think is happening when updated records are pushed to the client. Removing the type: [nil] seems to fix the problem, but I don't know why that is there and what it is intending to do.

catmando commented 6 years ago

I think its fixed! ab8637bc2b3a250f1e74544245375cb9806a7442