Can't modify frozen NilClass

[estiens]

I often run into an issue with this like in dirty.rb (106)

original.instance_variable_set "@value_before_type_cast", record["previous_attributes"][key]

if I am doing active dev work and the database attributes get out of sync with the model, would be nice to have some kind of rescue in the dirty tracking that doesn't throw an exception

FrozenError: can't modify frozen NilClass: nil
from /Users/estiens/.asdf/installs/ruby/3.1.1/lib/ruby/gems/3.1.0/bundler/gems/all_futures-ecec2e88d6d6/lib/all_futures/dirty.rb:106:in `instance_variable_set

[leastbad]

Hey Eric, this is valuable feedback. I would like to make this better for you and others, but I need to more fully understand the scenario that you're describing. What do you mean by active dev work? I mean I understand that you're trying to build some functionality on top of AF - cool! - but why are things out of sync? I'm having a hard time seeing the bigger picture.

Also, I'd love to see suggestions on an API for how you'd handle such a rescue. As always, I am trying hard to emulate what Active Record already does, or extrapolate if necessary. Generally, things only get frozen if they are conceptually supposed to be (eg. deleted records) but I'm eager to hear about places where this logic falls down since you can no longer unfreeze things in Ruby as of 2.7.

[estiens]

I mean, actively working on an app and, for example, changing an attribute name, adding/deleting an attribute - really you could argue that if you did that with a relational database you'd have to do an entire migration, and a simple Kredis.clear_all does the trick, but perhaps even a more descriptive error message would be okay?

just odd to hunt in the dirty code and realize what was throwing an exception was changing an attribute name - not nec a bug though!

[leastbad]

It took me a long time to figure out what I think you mean, and to be honest, I'm still not 100% sure.

If you attempt to use Active Record to access a model that doesn't have a table created in the database, it will raise: ActiveRecord::StatementInvalid: PG::UndefinedTable: ERROR: relation "messages" does not exist LINE 8: WHERE a.attrelid = '"messages"'::regclass

If you attempt to use Active Record to access an attribute on a model that doesn't exist, it raises: NoMethodError: undefined method frank' for #User:0x0000556404323b48`

Therefore, it seems entirely reasonable to me that if you attempt to load an All Futures record with a different schema that it's going to raise an exception.

Unfortunately, bringing dirty.rb into the conversation was a bit of a red herring, as the real issue is that at this time AF doesn't have any concept of a migration. I do agree that some explanation in the documentation to reinforce this could be helpful.

However, calling instance_variable_set on @value_before_type_cast is not even close to public API... so I'm sort of having a "what the heck does this guy want me to actually do" moment. In my last reply I specifically asked for suggestions on how any of the changes that you'd like to see could be implemented. That stands!

What I can try to do is raise an exception when a model loads that has a schema that doesn't match. That's probably as close to what you have in mind as we can get without diverging wildly from how Active Record would handle an un-migrated schema.

[leastbad]

So, as an update, I've identified the issue on 106 of dirty.rb and added a check to make sure that the attribute exists before setting it. This will take care of the case where you add an attribute after a record has been persisted; it will just remain nil with no exceptions raised. I think that this is a positive change - thanks for the nudge.

Now, the more difficult case is what happens if you rename or remove an attribute for an element that is saved. Right now, it will raise: ActiveModel::UnknownAttributeError: unknown attribute 'bob' for Government. I might override that with an AF-specific exception, but my two questions to you would be:

1. Is this a reasonable outcome, given that there is no migration concept in AF? I sort of feel like this might just have to do for v2.
2. What if there was a class method you could call to suppress those errors? I'm not committing to this, because I suspect it would be a major PITA to implement... but it deserves discussion. I'd almost rather just say that it makes more sense to apply that same effort to a migration system in v3.