[Scripts] Handle Configuration Definition Error (#1919)

* [Scripts] Handle configuration definition error from script service GraphQL

* [Scripts] Refactor error messages so that config file name is not hard-coded

* Modify configuration definition error message

lib/project_types/script/layers/domain/errors.rb

@@ -15,10 +15,11 @@ def initialize(type)
         end
 
         class MissingScriptConfigFieldError < ScriptProjectError
-          attr_reader :field
-          def initialize(field)
+          attr_reader :field, :filename
+          def initialize(field:, filename:)
             super()
             @field = field
+            @filename = filename
           end
         end
 

lib/project_types/script/layers/domain/script_config.rb

@@ -4,13 +4,13 @@ module Script
   module Layers
     module Domain
       class ScriptConfig
-        attr_reader :content, :version, :title, :description, :configuration_ui, :configuration
+        attr_reader :content, :version, :title, :description, :configuration_ui, :configuration, :filename
 
         REQUIRED_FIELDS = %w(version title)
 
-        def initialize(content:)
+        def initialize(content:, filename:)
+          @filename = filename
           validate_content!(content)
-
           @content = content
           @version = @content["version"].to_s
           @title = @content["title"]
@@ -23,7 +23,9 @@ def initialize(content:)
 
         def validate_content!(content)
           REQUIRED_FIELDS.each do |field|
-            raise Errors::MissingScriptConfigFieldError, field if content[field].nil?
+            if content[field].nil?
+              raise Errors::MissingScriptConfigFieldError.new(field: field, filename: filename)
+            end
           end
         end
       end

lib/project_types/script/layers/infrastructure/errors.rb

@@ -5,65 +5,78 @@ module Layers
     module Infrastructure
       module Errors
         class BuildError < ScriptProjectError; end
-        class ScriptConfigSyntaxError < ScriptProjectError; end
+
+        class ScriptConfigurationDefinitionError < ScriptProjectError
+          attr_reader :filename
+          def initialize(message:, filename:)
+            @filename = filename
+            super(message)
+          end
+        end
+
+        class ScriptConfigSyntaxError < ScriptProjectError
+          attr_reader :filename
+          def initialize(filename)
+            @filename = filename
+            super()
+          end
+        end
 
         class ScriptConfigMissingKeysError < ScriptProjectError
-          attr_reader :missing_keys
-          def initialize(missing_keys)
+          attr_reader :missing_keys, :filename
+          def initialize(missing_keys:, filename:)
             super()
             @missing_keys = missing_keys
+            @filename = filename
           end
         end
 
         class ScriptConfigInvalidValueError < ScriptProjectError
-          attr_reader :valid_input_modes
-          def initialize(valid_input_modes)
+          attr_reader :valid_input_modes, :filename
+          def initialize(valid_input_modes:, filename:)
             super()
             @valid_input_modes = valid_input_modes
+            @filename = filename
           end
         end
 
         class ScriptConfigFieldsMissingKeysError < ScriptProjectError
-          attr_reader :missing_keys
-          def initialize(missing_keys)
+          attr_reader :missing_keys, :filename
+          def initialize(missing_keys:, filename:)
             super()
             @missing_keys = missing_keys
+            @filename = filename
           end
         end
 
         class ScriptConfigFieldsInvalidValueError < ScriptProjectError
-          attr_reader :valid_types
-          def initialize(valid_types)
+          attr_reader :valid_types, :filename
+          def initialize(valid_types:, filename:)
             super()
             @valid_types = valid_types
+            @filename = filename
           end
         end
 
         class ScriptEnvAppNotConnectedError < ScriptProjectError; end
 
-        class InvalidScriptConfigYmlDefinitionError < ScriptProjectError; end
-
-        class InvalidScriptJsonDefinitionError < ScriptProjectError; end
-
-        class MissingScriptConfigYmlFieldError < ScriptProjectError
-          attr_reader :field
-          def initialize(field)
+        class ScriptConfigParseError < ScriptProjectError
+          attr_reader :filename, :serialization_format
+          def initialize(filename:, serialization_format:)
             super()
-            @field = field
+            @filename = filename
+            @serialization_format = serialization_format
           end
         end
 
-        class MissingScriptJsonFieldError < ScriptProjectError
-          attr_reader :field
-          def initialize(field)
+        class NoScriptConfigFileError < ScriptProjectError
+          attr_reader :filename
+          def initialize(filename)
             super()
-            @field = field
+            @filename = filename
           end
         end
 
-        class NoScriptConfigYmlFileError < ScriptProjectError; end
-        class NoScriptConfigFileError < ScriptProjectError; end
-
         class APILibraryNotFoundError < ScriptProjectError
           attr_reader :library_name
           def initialize(library_name)

lib/project_types/script/layers/infrastructure/script_project_repository.rb

@@ -160,12 +160,15 @@ def validate_metadata!(extension_point_type, language)
 
         def script_config_repository
           @script_config_repository ||= begin
+            script_config_yml_repo = ScriptConfigYmlRepository.new(ctx: ctx)
             supported_repos = [
-              ScriptConfigYmlRepository.new(ctx: ctx),
+              script_config_yml_repo,
               ScriptJsonRepository.new(ctx: ctx),
             ]
             repo = supported_repos.find(&:active?)
-            raise Infrastructure::Errors::NoScriptConfigYmlFileError if repo.nil?
+            if repo.nil?
+              raise Infrastructure::Errors::NoScriptConfigFileError, script_config_yml_repo.filename
+            end
             repo
           end
         end
@@ -179,7 +182,7 @@ def active?
           end
 
           def get!
-            raise Infrastructure::Errors::NoScriptConfigFileError unless active?
+            raise Infrastructure::Errors::NoScriptConfigFileError, filename unless active?
 
             content = ctx.read(filename)
             hash = file_content_to_hash(content)
@@ -196,6 +199,10 @@ def update!(title:)
             from_h(hash)
           end
 
+          def filename
+            raise NotImplementedError
+          end
+
           private
 
           def update_hash(hash:, title:)
@@ -204,14 +211,7 @@ def update_hash(hash:, title:)
           end
 
           def from_h(hash)
-            Domain::ScriptConfig.new(content: hash)
-          rescue Domain::Errors::MissingScriptConfigFieldError => e
-            raise missing_field_error, e.field
-          end
-
-          # to be implemented by subclasses
-          def filename
-            raise NotImplementedError
+            Domain::ScriptConfig.new(content: hash, filename: filename)
           end
 
           def file_content_to_hash(file_content)
@@ -221,57 +221,57 @@ def file_content_to_hash(file_content)
           def hash_to_file_content(hash)
             raise NotImplementedError
           end
-
-          def missing_field_error
-            raise NotImplementedError
-          end
         end
 
         class ScriptConfigYmlRepository < ScriptConfigRepository
-          private
-
           def filename
             "script.config.yml"
           end
 
+          private
+
           def file_content_to_hash(file_content)
             begin
               hash = YAML.load(file_content)
             rescue Psych::SyntaxError
-              raise Errors::InvalidScriptConfigYmlDefinitionError
+              raise parse_error
             end
-            raise Errors::InvalidScriptConfigYmlDefinitionError unless hash.is_a?(Hash)
+            raise parse_error unless hash.is_a?(Hash)
             hash
           end
 
           def hash_to_file_content(hash)
             YAML.dump(hash)
           end
 
-          def missing_field_error
-            Errors::MissingScriptConfigYmlFieldError
+          def parse_error
+            Errors::ScriptConfigParseError.new(filename: filename, serialization_format: "YAML")
           end
         end
 
         class ScriptJsonRepository < ScriptConfigRepository
-          private
-
           def filename
             "script.json"
           end
 
+          private
+
           def file_content_to_hash(file_content)
-            JSON.parse(file_content)
-          rescue JSON::ParserError
-            raise Errors::InvalidScriptJsonDefinitionError
+            begin
+              hash = JSON.parse(file_content)
+            rescue JSON::ParserError
+              raise parse_error
+            end
+            raise parse_error unless hash.is_a?(Hash)
+            hash
           end
 
           def hash_to_file_content(hash)
             JSON.pretty_generate(hash)
           end
 
-          def missing_field_error
-            Errors::MissingScriptJsonFieldError
+          def parse_error
+            Errors::ScriptConfigParseError.new(filename: filename, serialization_format: "JSON")
           end
         end
       end

lib/project_types/script/layers/infrastructure/script_service.rb

@@ -46,20 +46,37 @@ def set_app_script(
 
           if user_errors.any? { |e| e["tag"] == "already_exists_error" }
             raise Errors::ScriptRepushError, uuid
+          elsif (e = user_errors.find { |err| err["tag"] == "configuration_definition_error" })
+            raise Errors::ScriptConfigurationDefinitionError.new(
+              message: e["message"],
+              filename: script_config.filename,
+            )
           elsif (e = user_errors.any? { |err| err["tag"] == "configuration_definition_syntax_error" })
-            raise Errors::ScriptConfigSyntaxError
+            raise Errors::ScriptConfigSyntaxError, script_config.filename
           elsif (e = user_errors.find { |err| err["tag"] == "configuration_definition_missing_keys_error" })
-            raise Errors::ScriptConfigMissingKeysError, e["message"]
+            raise Errors::ScriptConfigMissingKeysError.new(
+              missing_keys: e["message"],
+              filename: script_config.filename,
+            )
           elsif (e = user_errors.find { |err| err["tag"] == "configuration_definition_invalid_value_error" })
-            raise Errors::ScriptConfigInvalidValueError, e["message"]
+            raise Errors::ScriptConfigInvalidValueError.new(
+              valid_input_modes: e["message"],
+              filename: script_config.filename,
+            )
           elsif (e = user_errors.find do |err|
                    err["tag"] == "configuration_definition_schema_field_missing_keys_error"
                  end)
-            raise Errors::ScriptConfigFieldsMissingKeysError, e["message"]
+            raise Errors::ScriptConfigFieldsMissingKeysError.new(
+              missing_keys: e["message"],
+              filename: script_config.filename,
+            )
           elsif (e = user_errors.find do |err|
                    err["tag"] == "configuration_definition_schema_field_invalid_value_error"
                  end)
-            raise Errors::ScriptConfigFieldsInvalidValueError, e["message"]
+            raise Errors::ScriptConfigFieldsInvalidValueError.new(
+              valid_types: e["message"],
+              filename: script_config.filename,
+            )
           elsif user_errors.find { |err| %w(not_use_msgpack_error schema_version_argument_error).include?(err["tag"]) }
             raise Domain::Errors::MetadataValidationError
           else

lib/project_types/script/messages/messages.rb

@@ -47,41 +47,39 @@ module Messages
           invalid_language_cause: "Invalid language %s.",
           invalid_language_help: "Allowed values: %s.",
 
-          missing_script_config_yml_field_cause: "The script.config.yml file is missing the required %s field.",
-          missing_script_config_yml_field_help: "Add the field and try again.",
+          missing_script_config_field_cause: "The %{filename} file is missing the required %{field} field.",
+          missing_script_config_field_help: "Add the field and try again.",
 
-          missing_script_json_field_cause: "The script.json file is missing the required %s field.",
-          missing_script_json_field_help: "Add the field and try again.",
+          script_config_parse_error_cause: "The %{filename} file contains invalid %{serialization_format}.",
+          script_config_parse_error_help: "Fix the errors and try again.",
 
-          invalid_script_json_definition_cause: "The script.json file contains invalid JSON.",
-          invalid_script_json_definition_help: "Fix the errors and try again.",
-
-          invalid_script_config_yml_definition_cause: "The script.config.yml file contains invalid YAML.",
-          invalid_script_config_yml_definition_help: "Fix the errors and try again.",
-
-          no_script_config_yml_file_cause: "The script.config.yml file is missing.",
-          no_script_config_yml_file_help: "Create this file and try again.",
+          no_script_config_file_cause: "The %{filename} file is missing.",
+          no_script_config_file_help: "Create this file and try again.",
 
           app_not_connected_cause: "Script is not connected to an app.",
           app_not_connected_help: "Run shopify connect or enter fields for api-key and api-secret.",
 
-          configuration_syntax_error_cause: "The script.json is not formatted properly.",
+          configuration_definition_error_cause: "In the %{filename} file, there was a problem with the "\
+                                                "configuration. %{message}",
+          configuration_definition_error_help: "Fix the error and try again.",
+
+          configuration_syntax_error_cause: "The %{filename} is not formatted properly.",
           configuration_syntax_error_help: "Fix the errors and try again.",
 
-          configuration_missing_keys_error_cause: "The script.json file is missing required keys: "\
+          configuration_missing_keys_error_cause: "The %{filename} file is missing required keys: "\
                                               "%{missing_keys}.",
           configuration_missing_keys_error_help: "Add the keys and try again.",
 
-          configuration_invalid_value_error_cause: "The script.json configuration only accepts "\
+          configuration_invalid_value_error_cause: "The %{filename} configuration only accepts "\
                                                    "one of the following types(s): %{valid_input_modes}.",
           configuration_invalid_value_error_help: "Change the type and try again.",
 
-          configuration_schema_field_missing_keys_error_cause: "A configuration entry in the script.json file "\
+          configuration_schema_field_missing_keys_error_cause: "A configuration entry in the %{filename} file "\
                                                      "is missing required keys: %{missing_keys}.",
           configuration_definition_schema_field_missing_keys_error_help: "Add the keys and try again.",
 
           configuration_schema_field_invalid_value_error_cause: "The configuration entries in the "\
-                                                     "script.json file only accept one of the following "\
+                                                     "%{filename} file only accept one of the following "\
                                                      "type(s): %{valid_types}.",
           configuration_schema_field_invalid_value_error_help: "Change the types and try again.",