Faker 3.3.0 generating invalid phone numbers

[theycallmeswift]

Describe the bug

Starting with version 3.3.0, Faker is generating invalid US phone numbers.

To Reproduce

Use the reproduction script below to reproduce the issue. If you swap the library version back to 3.2.3, the tests will always pass successfully.

# frozen_string_literal: true

require 'bundler/inline'

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

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

  gem 'faker', '3.3.0' # fails
  # gem 'faker', '3.2.3' # passes
  gem 'phonelib'
  gem 'minitest'
end

require 'minitest/autorun'

Faker::Config.locale = 'en-US'

class BugTest < Minitest::Test
  def test_fail_on_first_bad_number
    @tester = Faker::PhoneNumber

    100.times do
      number = @tester.cell_phone_in_e164

      assert Phonelib.valid?(number), "Expected #{number} to be a valid phone number"
    end
  end

  def test_100_numbers
    @tester = Faker::PhoneNumber

    results = Array.new(100) { Phonelib.valid?(@tester.cell_phone_in_e164) }
    tally = results.tally

    assert results.all? == true, "Expected 100 valid phone numbers, but only #{tally[true]} of #{tally[false]} valid"
  end
end

[thdaraujo]

Hey @theycallmeswift , thanks for reporting this.

Faker does not guarantee that a generated phone number will be valid via Phonelib. If you need to generate a specific format, please take a look at these discussions:
#2898 (comment)
#1104 (comment)

[theycallmeswift]

@thdaraujo makes sense. Can you help me understand what changed that caused this though? I've been banging my head with Git bisect and haven't been able to trace this to anything in the commit history.

[theycallmeswift]

Also, in case it's helpful to anyone in the future, this is an easy thing to add to your spec/support folder to add valid_ versions of all PhoneNumber methods:

# frozen_string_literal: true

module Faker
  class PhoneNumber < Base
    class << self
      def respond_to_missing?(method_name, include_private = false)
        method_name.to_s.start_with?('valid_') || super
      end

      def method_missing(method, *args, &block)
        super unless method.to_s.starts_with?('valid_')

        # In a sample of 100,000 attempts, it takes an average of 3.6 tries and
        # more than 50 is exceptional (0.001%)
        max_attempts = 50
        attempts = 0

        loop do
          number = send(method.to_s.gsub('valid_', '').to_sym, *args, &block)
          return number if Phonelib.valid? number

          attempts += 1
          break if attempts >= max_attempts
        end

        raise "Faker unable to generate valid phone number using #{method} in #{attempts} tries."
      end
    end
  end
end

[aprescott]

I just encountered this same problem. @theycallmeswift thanks for opening this issue.

I think what's actually happening here is the result of a legitimate bug with cell phone number generation and this issue shouldn't be closed. There was a change to the file structure of en-US.yml in 31d99d1. PhoneNumber expects the cell phone formats to be in faker.cell_phone.formats, but 31d99d1 moved faker.cell_phone to faker.phone_number.cell_phone, so the faker.cell_phone.formats entry isn't found, causing a fallback to en.yml's generic faker.cell_phone.formats.

Spot checking the other .yml files, en-US.yml appears to be the only one that has the wrong position of its cell_phone key after 31d99d1.

Further notes from my debugging below.

Faker maintains US-specific area codes en-US locale file along with non-generic format definitions. It just stopped using it in 3.3.0 for methods like cell_phone_in_e164, which can now produce an invalid US number like +11466926150. So while there may be no guarantee that numbers are valid, there are area code definitions that are ostensibly expected to be used.

You can trace a change in behavior around US numbers by inspecting the translation within a console. Here it is working on the released gem 3.2.3 within an app I maintain:

# on the released gem 3.2.3 (not the v3.2.3 git tag)
Faker::Config.locale = "en-US"
Faker::PhoneNumber.translate("faker.cell_phone.formats")
# => 
# ["\#{PhoneNumber.area_code}-\#{PhoneNumber.exchange_code}-\#{PhoneNumber.subscriber_number}",
#  "(\#{PhoneNumber.area_code}) \#{PhoneNumber.exchange_code}-\#{PhoneNumber.subscriber_number}",
#  "\#{PhoneNumber.area_code}-\#{PhoneNumber.exchange_code}-\#{PhoneNumber.subscriber_number}",
#  "\#{PhoneNumber.area_code}.\#{PhoneNumber.exchange_code}.\#{PhoneNumber.subscriber_number}",
#  "\#{PhoneNumber.area_code}-\#{PhoneNumber.exchange_code}-\#{PhoneNumber.subscriber_number}",
#  "(\#{PhoneNumber.area_code}) \#{PhoneNumber.exchange_code}-\#{PhoneNumber.subscriber_number}",
#  "\#{PhoneNumber.area_code}-\#{PhoneNumber.exchange_code}-\#{PhoneNumber.subscriber_number}",
#  "\#{PhoneNumber.area_code}.\#{PhoneNumber.exchange_code}.\#{PhoneNumber.subscriber_number}"]

Here it is broken on the 3.3.0 release and on the v3.2.3 git tag for this repo:

# on 3.3.0 (and the v3.2.3 git tag)
Faker::Config.locale = "en-US"
Faker::PhoneNumber.translate("faker.cell_phone.formats")
# => ["###-###-####", "(###) ###-####", "###.###.####", "### ### ####"]

# can take any number from 1 to 9, hence invalid US numbers like +11466926150.

Note that there's a discrepancy between the v3.2.3 git tag and the released gem version 3.2.3. From #2900 it seems like there was an issue with the 3.2.3 release where changes weren't included, which would explain why I couldn't reproduce the problem on the v3.2.3 git tag.

If you git bisect (ignoring the v3.2.3 tag) the first commit that starts generating invalid US numbers is 31d99d1, which refactored the translation files. (Presumably those translation files didn't make it into the 3.2.3 release but are incorrectly part of the v3.2.3 tag.)

In 31d99d1, the structure of the en-US.yml file was changed.

Here's the before:

And the after:

The faker.cell_phone key is now faker.phone_number.cell_phone. But that isn't the key that PhoneNumber expects, even on the current main.

def cell_phone
  parse('cell_phone.formats')
end

That means 3.3.0 changed PhoneNumber.cell_phone a result of the .yml changes in 31d99d1, along with any other method that calls cell_phone, like cell_phone_in_e164.

Spot checking other locale files indicates that cell_phone.formats is expected, not phone_number.cell_phone.formats, so I think a revision to en-US.yml will fix this bug.

[aprescott]

I've opened #2924 which should address this.

[aprescott]

In case it helps anyone, see #2924 (comment) for a short-term workaround that doesn't involve generating US numbers with retries.

[thdaraujo]

oh interesting, thanks for investigating @aprescott ! That makes sense, and you're right, the refactored yaml commit was only added on v3.3.

[stefannibrasil]

thank you @aprescott releasing the fix in a patch release in a bit.