Add opt-in failures for translation downloads

CHANGELOG.md

@@ -10,7 +10,7 @@ _None_
 
 ### New Features
 
-_None_
+- Add an opt-in `fail_on_error` mode to translation download actions so GlotPress request and response errors fail CI jobs. [#771]
 
 ### Bug Fixes
 

lib/fastlane/plugin/wpmreleasetoolkit/actions/android/android_download_translations_action.rb

@@ -1,5 +1,7 @@
 # frozen_string_literal: true
 
+require_relative '../../helper/config_item_helper'
+
 # This action is the new version of android_update_metadata (AndroidUpdateMetadataAction) and should now be used instead of that one
 
 module Fastlane
@@ -19,7 +21,8 @@ def self.run(params)
           res_dir: res_dir,
           glotpress_project_url: params[:glotpress_url],
           glotpress_filters: params[:status_filter].map { |s| { status: s } },
-          locales_map: params[:locales]
+          locales_map: params[:locales],
+          fail_on_error: params[:fail_on_error]
         )
 
         # Update submodules then lint translations
@@ -99,6 +102,7 @@ def self.available_options
             type: Boolean,
             default_value: false
           ),
+          Fastlane::Helper::ConfigItemHelper.opt_in_fail_on_error_config_item,
         ]
       end
 

lib/fastlane/plugin/wpmreleasetoolkit/actions/common/get_prs_between_tags.rb

@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 
 require 'fastlane/action'
+require_relative '../../helper/config_item_helper'
 require_relative '../../helper/github_helper'
 
 module Fastlane
@@ -104,15 +105,7 @@ def self.available_options
                                                     'See https://docs.github.com/en/repositories/releasing-projects-on-github/automatically-generated-release-notes#configuration-options',
                                        optional: true,
                                        type: String),
-          FastlaneCore::ConfigItem.new(key: :fail_on_error,
-                                       description: 'Whether to fail the lane if the changelog cannot be computed. ' \
-                                                    'When `false` (the default), the error message is returned as the changelog itself, ' \
-                                                    'so that it ends up visible in the GitHub Release body. ' \
-                                                    'Set this to `true` if the caller publishes the release only after this action succeeds, ' \
-                                                    'and would rather stop than publish a release whose notes are an error message',
-                                       optional: true,
-                                       default_value: false,
-                                       type: Boolean),
+          Fastlane::Helper::ConfigItemHelper.opt_in_fail_on_error_config_item,
           Fastlane::Helper::GithubHelper.github_token_config_item,
         ]
       end

lib/fastlane/plugin/wpmreleasetoolkit/actions/common/gp_downloadmetadata_action.rb

@@ -2,6 +2,7 @@
 
 require 'fastlane/action'
 require_relative '../../helper/metadata_download_helper'
+require_relative '../../helper/config_item_helper'
 
 module Fastlane
   module Actions
@@ -13,12 +14,13 @@ def self.run(params)
         UI.message "Source locale: #{params[:source_locale].nil? ? '-' : params[:source_locale]}"
         UI.message "Path: #{params[:download_path]}"
         UI.message "Auto-retry: #{params[:auto_retry]}"
+        UI.message "Fail on error: #{params[:fail_on_error]}"
 
         # Check download path
         FileUtils.mkdir_p(params[:download_path])
 
         # Download
-        downloader = Fastlane::Helper::MetadataDownloader.new(params[:download_path], params[:target_files], params[:auto_retry])
+        downloader = Fastlane::Helper::MetadataDownloader.new(params[:download_path], params[:target_files], params[:auto_retry], fail_on_error: params[:fail_on_error])
 
         params[:locales].each do |loc|
           if loc.is_a?(Array)
@@ -77,6 +79,7 @@ def self.available_options
                                        type: FastlaneCore::Boolean,
                                        optional: true,
                                        default_value: true),
+          Fastlane::Helper::ConfigItemHelper.opt_in_fail_on_error_config_item,
         ]
       end
 

lib/fastlane/plugin/wpmreleasetoolkit/actions/ios/ios_download_strings_files_from_glotpress.rb

@@ -1,5 +1,8 @@
 # frozen_string_literal: true
 
+require 'tempfile'
+require_relative '../../helper/config_item_helper'
+
 module Fastlane
   module Actions
     class IosDownloadStringsFilesFromGlotpressAction < Action
@@ -17,33 +20,91 @@ def self.run(params)
           destination = File.join(lproj_dir, "#{params[:table_basename]}.strings")
           FileUtils.mkdir_p(lproj_dir)
 
-          Fastlane::Helper::Ios::L10nHelper.download_glotpress_export_file(
+          download_and_replace_strings_file(
             project_url: params[:project_url],
             locale: glotpress_locale,
             filters: params[:filters],
-            destination: destination
+            destination: destination,
+            table_basename: params[:table_basename],
+            skip_file_validation: params[:skip_file_validation],
+            fail_on_error: params[:fail_on_error]
           )
-          # Do a quick check of the downloaded `.strings` file to ensure it looks valid
-          validate_strings_file(destination) unless params[:skip_file_validation]
         end
       end
 
-      # Validate that a `.strings` file downloaded from GlotPress seems valid and does not contain empty translations
-      def self.validate_strings_file(destination)
-        return unless File.exist?(destination) # If the file failed to download, don't try to validate an non-existing file. We'd already have a separate error for the download failure anyway.
+      def self.download_and_replace_strings_file(project_url:, locale:, filters:, destination:, table_basename:, skip_file_validation:, fail_on_error:)
+        if File.exist?(destination) && !File.file?(destination)
+          report_error("The destination `#{destination}` exists but is not a regular file", fail_on_error: fail_on_error)
+          return false
+        end
 
-        translations = Fastlane::Helper::Ios::L10nHelper.read_strings_file_as_hash(path: destination)
-        empty_keys = translations.select { |_, value| value.nil? || value.empty? }.keys.sort
-        unless empty_keys.empty?
-          UI.error(
-            "Found empty translations in `#{destination}` for the following keys: #{empty_keys.inspect}.\n" \
-              + "This is likely a GlotPress bug, and will lead to copies replaced by empty text in the UI.\n" \
-              + 'Please report this to the GlotPress team, and fix the file locally before continuing.'
+        destination_mode = File.exist?(destination) ? File.stat(destination).mode & 0o7777 : 0o644
+        Tempfile.create([table_basename, '.strings'], File.dirname(destination)) do |temporary_file|
+          downloaded = Fastlane::Helper::Ios::L10nHelper.download_glotpress_export_file(
+            project_url: project_url,
+            locale: locale,
+            filters: filters,
+            destination: temporary_file,
+            fail_on_error: fail_on_error
           )
+          return false unless downloaded
+
+          temporary_file.flush
+          # Do a quick check of the downloaded `.strings` file to ensure it looks valid
+          validate_strings_file(temporary_file.path, display_path: destination, fail_on_error: fail_on_error) unless skip_file_validation
+          File.chmod(destination_mode, temporary_file.path)
+          temporary_file.close
+          File.rename(temporary_file.path, destination)
         end
+        true
+      rescue FastlaneCore::Interface::FastlaneError
+        raise
       rescue StandardError => e
-        UI.error("Error while validating the file exported from GlotPress (`#{destination}`) - #{e.message.chomp}")
+        report_error("Error writing downloaded locale `#{locale}` — #{e.message} (#{destination})", fail_on_error: fail_on_error)
+        false
+      end
+      private_class_method :download_and_replace_strings_file
+
+      # Validate that a `.strings` file downloaded from GlotPress seems valid and does not contain empty translations
+      def self.validate_strings_file(path, display_path: path, fail_on_error: false)
+        unless File.exist?(path)
+          report_error("The file exported from GlotPress was not created (`#{display_path}`)", fail_on_error: fail_on_error) if fail_on_error
+          return
+        end
+
+        if File.empty?(path)
+          report_error("The file exported from GlotPress is empty (`#{display_path}`)", fail_on_error: fail_on_error) if fail_on_error
+          return
+        end
+
+        translations = nil
+        begin
+          translations = Fastlane::Helper::Ios::L10nHelper.read_strings_file_as_hash(path: path)
+        rescue StandardError => e
+          report_error("Error while validating the file exported from GlotPress (`#{display_path}`) - #{e.message.chomp}", fail_on_error: fail_on_error)
+          return
+        end
+
+        unless translations.is_a?(Hash) && translations.all? { |key, value| key.is_a?(String) && value.is_a?(String) }
+          report_error("The file exported from GlotPress is not a string-to-string dictionary (`#{display_path}`)", fail_on_error: fail_on_error)
+          return
+        end
+
+        empty_keys = translations.select { |_, value| value.nil? || value.empty? }.keys.sort
+        return if empty_keys.empty?
+
+        report_error(
+          "Found empty translations in `#{display_path}` for the following keys: #{empty_keys.inspect}.\n" \
+            + "This is likely a GlotPress bug, and will lead to copies replaced by empty text in the UI.\n" \
+            + 'Please report this to the GlotPress team, and fix the file locally before continuing.',
+          fail_on_error: fail_on_error
+        )
+      end
+
+      def self.report_error(message, fail_on_error:)
+        fail_on_error ? UI.user_error!(message) : UI.error(message)
       end
+      private_class_method :report_error
 
       #####################################################
       # @!group Documentation
@@ -92,6 +153,7 @@ def self.available_options
                                        type: Fastlane::Boolean,
                                        optional: true,
                                        default_value: false),
+          Fastlane::Helper::ConfigItemHelper.opt_in_fail_on_error_config_item,
         ]
       end
 

lib/fastlane/plugin/wpmreleasetoolkit/helper/android/android_localize_helper.rb

@@ -243,17 +243,19 @@ def self.create_available_languages_file(res_dir:, locale_codes:)
         # @param [Array<Hash{Symbol=>String}>] locales_map
         #        An array of locales to download. Each item in the array must be a Hash
         #        with keys `:glotpress` and `:android` containing the respective locale codes.
+        # @param [Boolean] fail_on_error Whether to fail on request errors or invalid downloaded XML.
         #
-        def self.download_from_glotpress(res_dir:, glotpress_project_url:, locales_map:, glotpress_filters: { status: 'current' })
+        def self.download_from_glotpress(res_dir:, glotpress_project_url:, locales_map:, glotpress_filters: { status: 'current' }, fail_on_error: false)
           glotpress_filters = [glotpress_filters] unless glotpress_filters.is_a?(Array)
+          UI.user_error!('At least one GlotPress filter is required.') if fail_on_error && glotpress_filters.empty?
 
           orig_file = File.join(res_dir, 'values', 'strings.xml')
           orig_xml = File.open(orig_file) { |f| Nokogiri::XML(f, nil, Encoding::UTF_8.to_s) }
 
           locales_map.each do |lang_codes|
             all_xml_documents = glotpress_filters.map do |filters|
               UI.message "Downloading translations for '#{lang_codes[:android]}' from GlotPress (#{lang_codes[:glotpress]}) [#{filters}]..."
-              download_glotpress_export_file(project_url: glotpress_project_url, locale: lang_codes[:glotpress], filters: filters)
+              download_glotpress_export_file(project_url: glotpress_project_url, locale: lang_codes[:glotpress], filters: filters, fail_on_error: fail_on_error)
             end.compact
             next if all_xml_documents.empty?
 
@@ -282,19 +284,35 @@ def self.download_from_glotpress(res_dir:, glotpress_project_url:, locales_map:,
         # @param [String] locale The GlotPress locale code to download strings for.
         # @param [Hash{Symbol=>String}] filters The hash of filters to apply when exporting from GlotPress.
         #                               Typical examples include `{ status: 'current' }` or `{ status: 'review' }`.
+        # @param [Boolean] fail_on_error Whether to fail on request errors or invalid downloaded XML.
         # @return [Nokogiri::XML::Document] the download XML document, parsed as a Nokogiri::XML object
         #
-        def self.download_glotpress_export_file(project_url:, locale:, filters:)
+        def self.download_glotpress_export_file(project_url:, locale:, filters:, fail_on_error:)
           query_params = filters.transform_keys { |k| "filters[#{k}]" }.merge(format: 'android')
           url = "#{project_url.chomp('/')}/#{locale}/default/export-translations/?#{URI.encode_www_form(query_params)}"
 
           Fastlane::Helper::GlotPressDownloader.download(
             url: url,
             locale: locale,
-            auto_retry: true
+            auto_retry: true,
+            fail_on_error: fail_on_error
           ) do |response_body|
             # Replace tabs with spaces (GlotPress uses tabs, but we prefer spaces)
-            Nokogiri::XML(response_body.gsub("\t", '    '), nil, Encoding::UTF_8.to_s)
+            if fail_on_error
+              begin
+                xml = Nokogiri::XML(response_body.gsub("\t", '    '), nil, Encoding::UTF_8.to_s, &:strict)
+              rescue Nokogiri::XML::SyntaxError => e
+                UI.user_error!("Invalid Android translation export for locale `#{locale}` — #{e.message} (#{url})")
+              end
+
+              unless xml.root&.name == 'resources'
+                UI.user_error!("Invalid Android translation export for locale `#{locale}` — expected a `resources` root element (#{url})")
+              end
+
+              xml
+            else
+              Nokogiri::XML(response_body.gsub("\t", '    '), nil, Encoding::UTF_8.to_s)
+            end
           end
         end
         private_class_method :download_glotpress_export_file

lib/fastlane/plugin/wpmreleasetoolkit/helper/config_item_helper.rb

@@ -0,0 +1,17 @@
+# frozen_string_literal: true
+
+module Fastlane
+  module Helper
+    class ConfigItemHelper
+      def self.opt_in_fail_on_error_config_item
+        FastlaneCore::ConfigItem.new(
+          key: :fail_on_error,
+          description: 'Whether handled errors should fail the lane instead of using the action-specific fallback behavior',
+          type: FastlaneCore::Boolean,
+          optional: true,
+          default_value: false
+        )
+      end
+    end
+  end
+end