rails: New ActiveModel::Error raises FrozenError without deprecation on 6.1

At GitHub we’re seeing that the new ActiveModel::Error (which I think is a great improvement overall) is raising FrozenError in our app. In particular we see it when we call errors[:some_attribute].clear.

Migrating away from calling clear is warranted but right now the errors prevent us from testing with Rails 6.1 and a deprecation warning is probably more appropriate for 6.1 than a hard failure.

More Info

ActiveModel::Error was added to Rails 6.1 in https://github.com/rails/rails/pull/32313. Part of the change was the addition of DeprecationHandlingMessageArray which serves as a delegate for the [] method:

https://github.com/rails/rails/blob/ba38b40e83302cb9da17c95086d93e5071426668/activemodel/lib/active_model/errors.rb#L211

DeprecationHandlingMessageArray handles calls to << with a deprecation warning, but does not handle calls to other array methods which mutate the array such as errors[:base].clear. In our case, we’ve run into issues with .clear so far but other array methods could conceivably be an issue as well.

Here is a reproduction script which fails under 6.1 and passed under 6.0. It’s a pretty contrived example (simplified from our actual use) but I think it serves to highlight the issue. The script will use 6.1 (master) by default and 6.0 can be run by: RAILS_6_0=1 ruby ./reproduction.rb

# frozen_string_literal: true

require "bundler/inline"

# Run for Rails 6.1 (master) - Failures
# ruby /Users/cpruitt/Desktop/activerecord_errors_repro.rb
#
# Run for Rails 6.0 - Passes
# RAILS_6_0=1 ruby /Users/cpruitt/Desktop/activerecord_errors_repro.rb

gemfile(true) do
  source "https://rubygems.org"

  git_source(:github) { |repo| "https://github.com/#{repo}.git" }

  if ENV["RAILS_6_0"] == "1"
    gem "activerecord", "6.0.0"
  else
    gem "rails", github: "rails/rails"
  end
  gem "sqlite3"
end

require "active_record"
require "minitest/autorun"
require "logger"

# This connection will do for database-independent bug reports.
ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:")
ActiveRecord::Base.logger = Logger.new(STDOUT)

ActiveRecord::Schema.define do
  create_table :users, force: true do |t|
    t.string :username
    t.datetime :suspended_at
  end
end

class User < ActiveRecord::Base
  validate :must_be_verbose

  def suspend(reason)
    # This clear fails under Rails 6.1 with
    # FrozenError: can't modify frozen Array
    errors[:base].clear

    errors.add(:base, "You need to specify a reason for the suspension.") if reason.blank?
    update_attribute(:suspended_at, Time.now)
  end

  private

    # Usernames must be really long
    def must_be_verbose
      return if username.to_s.length > 100
      errors.add(:base, "You need to be more witty with your username.")
    end
end

class BugTest < Minitest::Test
  def test_email_validation_exists_on_save
    short_username_from_before_length_validation = 'firstuser'
    user = User.new(username: short_username_from_before_length_validation)
    user.save!(validate: false)

    user.save
    user.errors[:base].any?
    assert_equal ["You need to be more witty with your username."], user.errors[:base]
  end

  def test_user_can_be_suspended_even_if_other_validation_fails
    short_username_from_before_length_validation = 'firstuser'
    user = User.new(username: short_username_from_before_length_validation)
    user.save!(validate: false)

    user.save
    user.suspend("Violated TOS")
    assert user.errors.empty?
  end

  def test_email_validation_is_cleared_on_failed_suspend
    short_username_from_before_length_validation = 'firstuser'
    user = User.new(username: short_username_from_before_length_validation)
    user.save!(validate: false)

    # This will add "You need to be more witty with your username." to base errors
    user.save

    # No reason given so will fail and will have error on base but will clear base error added above
    user.suspend("")

    # Only the suspension error should appear here
    assert_equal ["You need to specify a reason for the suspension."], user.errors[:base]
  end
end

CC @lulalala, @eileencodes