RefererParser::InvalidUriError (issue for builds from 3.4.0-beta1 to 3.5.0-beta1)

[GesJeremie]

Edit: We probably want to label this not as a bug but more as enhancement (my bad)

Describe the bug
Sentry is reporting an error for one of our build:

Investigation

• RefererParser is a dependency in workarea-core: s.add_dependency 'referer-parser', '~> 0.3.0' (https://rubygems.org/gems/referer-parser/versions/0.3.0) and seems to have been introduced in workarea 3.4.0-beta1

• Some users of this gem are experiencing similar problems: Handling Android app referrers like "com.google.android.googlequicksearchbox"? snowplow-referer-parser/referer-parser#131. The gist being that RefererParser::Parser.new.parse() is not able to parse referrer "urls" starting with android-app:// (ie. The referrer being an android app such as slack, facebook, twitter, ...).

• Scott Bullard (can't link his github profile) provided a fix for this bug: snowplow-referer-parser/referer-parser@737a892, however it has never been officially released (merged on develop but not on master)

• Looking at the different versions of Workarea, it seems that 3.5.0 mitigates the bug by rescue-ing the parse result: https://github.com/workarea-commerce/workarea/blob/v3.5.0/core/lib/workarea/visit.rb#L69. However that's not the case for older versions starting from 3.4.0-beta1 to 3.5.0-beta1: https://github.com/workarea-commerce/workarea/blob/v3.4.12/core/app/controllers/workarea/current_referrer.rb#L10 (I couldn't fetch the source code from 3.4.0-beta1 ¯\_(ツ)_/¯ )

• It's worth noting that recent builds (>= v3.5.0) will basically not track the android-app referrers since the gem RefererParser will raise and therefore will be rescued to an empty hash. It's probably worth fixing since it's a good metric to have (that's what triggered this issue / discussion).

Bonus

If anyone is running into the same issue and are not able to upgrade at the moment, here's my fix (v3.4.32):

# lib/workarea/ext/freedom_patches/referer_parser.rb
module RefererParser
  class Parser
    #
    # Patched to take in account the new scheme `android-app`.
    #
    # See: https://github.com/snowplow-referer-parser/referer-parser/pull/145/commits/737a892f628347a610e5633d62153c8d6518f4e5
    #
    # The PR above has never been officialy released, therefore the patch.
    #
    def parse(obj)
      url = obj.is_a?(URI) ? obj : URI.parse(obj.to_s)
      
      # This section changed
      if !['android-app', 'http', 'https'].include?(url.scheme)
        raise InvalidUriError.new("Only Android-App, HTTP and HTTPS schemes are supported -- #{url.scheme}")
      end

      data = { :known => false, :uri => url.to_s }

      domain, name_key = domain_and_name_key_for(url)
      if domain and name_key
        referer_data = @name_hash[name_key]
        data[:known] = true
        data[:source] = referer_data[:source]
        data[:medium] = referer_data[:medium]
        data[:domain] = domain

        # Parse parameters if the referer uses them
        if url.query and referer_data[:parameters]
          query_params = CGI.parse(url.query)
          referer_data[:parameters].each do |param|
            # If there is a matching parameter, get the first non-blank value
            if !(values = query_params[param]).empty?
              data[:term] = values.select { |v| v.strip != "" }.first
              break if data[:term]
            end
          end
        end
      end

      data
    rescue URI::InvalidURIError
      raise InvalidUriError.new("Unable to parse URI, not a URI? -- #{obj.inspect}", $!)
    end
  end
end

# test/lib/workarea/ext/freedom_patches/referer_parser_test.rb
# A silly test to sleep well at night
require 'test_helper'

module RefererParser
  class ParserTest < Workarea::TestCase
    def test_parse_android_app_referrers
      referrers = [
        'android-app://com.linkedin.android',
        'android-app://org.telegram.plus',
        'android-app://com.twitter.android'
      ]

      assert_nothing_raised do
        referrers.each do |referrer|
          Parser.new.parse(referrer)
        end
      end
    end
  end
end

# You probably want to override Workarea::CurrentReferrer as well to backport the rescue
# app/controllers/workarea/current_referrer.rb
module Workarea
  module CurrentReferrer
    #
    # Base implementation (workarea-core v3.4.32) is prone
    # to raise an error for `RefererParser::Parser.new.parse(referrer)`
    # which will block the user to be able to checkout properly.
    #
    # @source https://github.com/workarea-commerce/workarea/blob/v3.4.32/core/app/controllers/workarea/current_referrer.rb
    #
    # The latest implementation (workarea-core 3.5.18) mitigates the problem
    # by rescue-ing the attributes.
    #
    # @source https://github.com/workarea-commerce/workarea/blob/v3.5.18/core/lib/workarea/visit.rb#L72
    #
    # I took inspiration from the latest implementation and applied the changes here.
    #
    def current_referrer
      return @current_referrer if defined?(@current_referrer)

      referrer = cookies['workarea_referrer']
      return unless referrer.present?
     
      # This rescue is gold, you don't want the user to reach a 404 during checkout ===> -$$$$
      attributes = RefererParser::Parser.new.parse(referrer) rescue {}

      @current_referrer ||= TrafficReferrer.new(
        attributes.slice(:source, :medium, :uri)
      )
    end
  end
end

[GesJeremie]

After more thinking, we probably want to close this ticket:

• It's actually not the job of Workarea to fix RefererParser:Parser.new.parse to take in account android-app:// schemes -- Either way fixing it actually just set TrafficReferrer#uri to something like "android-app://org.telegram.plus". Still useful but you know ...

• New versions behave correctly thanks to the rescue introduced in 3.5.0

• The only thing worth fixing would be to add the rescue in CurrentReferrer for 3.4 in a patch since it affects Checkout (current tests are not able to surface this bug).

Anyway, sorry for all the noise :)

[bencrouse]

I agree that if this is interrupting checkout, it's worth fixing for v3.4. I've made a note for our next sprint. Thanks for bringing it up!