Adding parsing error handling and improving error messages (#282)

* Adding parsing erorr handling and improving error messages on missing or nil tokens field

* Adding in quotes for the string

* Responding to comments

* running rubocop

* Adding in line that I removed

* Changing helpful messages to new errors and updating tests

* Fix broken tests and adding in a new one

Co-authored-by: Manny Jimenez <mannyjimenez@google.com>

Author: manny-jimenez

lib/fastlane/plugin/firebase_app_distribution/helper/firebase_app_distribution_auth_client.rb

@@ -48,20 +48,27 @@ def fetch_auth_token(google_service_path, firebase_cli_token, debug = false)
       private
 
       def refresh_token_from_firebase_tools
-        if ENV["XDG_CONFIG_HOME"].nil? || ENV["XDG_CONFIG_HOME"].empty?
-          config_path = File.expand_path(".config/configstore/firebase-tools.json", "~")
-        else
-          config_path = File.expand_path("configstore/firebase-tools.json", ENV["XDG_CONFIG_HOME"])
-        end
-
+        config_path = format_config_path
         if File.exist?(config_path)
           begin
-            refresh_token = JSON.parse(File.read(config_path))['tokens']['refresh_token']
-            refresh_token unless refresh_token.nil? || refresh_token.empty?
-          # TODO: Catch parser errors, improve error handling here
-          # Returns nil when there is an empty "tokens" field in the firebase-tools json
-          rescue NoMethodError
+            firebase_tools_tokens = JSON.parse(File.read(config_path))['tokens']
+            if firebase_tools_tokens.nil?
+              UI.user_error!(ErrorMessage::EMPTY_TOKENS_FIELD)
+              return
+            end
+            refresh_token = firebase_tools_tokens['refresh_token']
+          rescue JSON::ParserError
+            UI.user_error!(ErrorMessage::PARSE_FIREBASE_TOOLS_JSON_ERROR)
           end
+          refresh_token unless refresh_token.nil? || refresh_token.empty?
+        end
+      end
+
+      def format_config_path
+        if ENV["XDG_CONFIG_HOME"].nil? || ENV["XDG_CONFIG_HOME"].empty?
+          File.expand_path(".config/configstore/firebase-tools.json", "~")
+        else
+          File.expand_path("configstore/firebase-tools.json", ENV["XDG_CONFIG_HOME"])
         end
       end
 

lib/fastlane/plugin/firebase_app_distribution/helper/firebase_app_distribution_error_message.rb

@@ -3,6 +3,7 @@ module ErrorMessage
   MISSING_APP_ID = "Missing app id. Please check that the app parameter is set and try again"
   SERVICE_CREDENTIALS_NOT_FOUND = "Service credentials file does not exist. Please check the service credentials path and try again"
   PARSE_SERVICE_CREDENTIALS_ERROR = "Failed to extract service account information from the service credentials file"
+  PARSE_FIREBASE_TOOLS_JSON_ERROR = "Encountered error parsing json file. Ensure the firebase-tools.json file is formatted correctly"
   UPLOAD_RELEASE_NOTES_ERROR = "App Distribution halted because it had a problem uploading release notes"
   UPLOAD_TESTERS_ERROR = "App Distribution halted because it had a problem adding testers/groups"
   GET_RELEASE_TIMEOUT = "App Distribution failed to fetch release information"
@@ -20,6 +21,7 @@ module ErrorMessage
   PLAY_IAS_TERMS_NOT_ACCEPTED = "You must accept the Play Internal App Sharing (IAS) terms to upload AABs."
   INVALID_EMAIL_ADDRESS = "You passed an invalid email address."
   TESTER_LIMIT_VIOLATION = "Creating testers would exceed tester limit"
+  EMPTY_TOKENS_FIELD = "Unable to find \"tokens\" field in the firebase-tools.json file. Ensure that the file has a tokens field and try again"
 
   def self.aab_upload_error(aab_state)
     "Failed to process the AAB: #{aab_state}"

lib/fastlane/plugin/firebase_app_distribution/version.rb

@@ -1,5 +1,5 @@
 module Fastlane
   module FirebaseAppDistribution
-    VERSION = "0.3.6"
+    VERSION = "0.3.7"
   end
 end

spec/firebase_app_distribution_auth_client_spec.rb

@@ -7,6 +7,7 @@
   let(:fake_firebase_tools_contents) { "{\"tokens\": {\"refresh_token\": \"refresh_token\"} }" }
   let(:fake_firebase_tools_contents_no_tokens_field) { "{}" }
   let(:fake_firebase_tools_contents_no_refresh_field) { "{\"tokens\": \"empty\"}" }
+  let(:fake_firebase_tools_contents_invalid_json) { "\"tokens\": \"empty\"}" }
   let(:fake_service_creds) { double("service_account_creds") }
   let(:fake_oauth_client) { double("oauth_client") }
   let(:payload) { { "access_token" => "service_fake_auth_token" } }
@@ -138,7 +139,7 @@
             .and_return(fake_firebase_tools_contents_no_tokens_field)
           expect(File).to receive(:exist?).and_return(true)
           expect { auth_client.fetch_auth_token(empty_val, empty_val) }
-            .to raise_error(ErrorMessage::MISSING_CREDENTIALS)
+            .to raise_error(ErrorMessage::EMPTY_TOKENS_FIELD)
         end
 
         it 'crashes if the firebase tools json has no refresh_token field' do
@@ -148,6 +149,14 @@
           expect { auth_client.fetch_auth_token(empty_val, empty_val) }
             .to raise_error(ErrorMessage::MISSING_CREDENTIALS)
         end
+
+        it 'crashes if the firebase tools json fails to parse' do
+          allow(File).to receive(:read)
+            .and_return(fake_firebase_tools_contents_invalid_json)
+          expect(File).to receive(:exist?).and_return(true)
+          expect { auth_client.fetch_auth_token(empty_val, empty_val) }
+            .to raise_error(ErrorMessage::PARSE_FIREBASE_TOOLS_JSON_ERROR)
+        end
       end
     end
   end