Merge pull request #1633 from dg-ratiodata/feature/preven_crash_due_to_invalid_data_file

Refactor Data Loader to prevent middleman from crashing due to invalid…

middleman-core/features/data.feature

@@ -51,3 +51,31 @@ Feature: Local Data API
     Then I should see "title1:Hello"
     Then I should see "title2:More"
     Then I should see "title3:Stuff"
+
+  Scenario: Invalid YAML
+    Given a fixture app "basic-data-app"
+    And the default aruba timeout is 30 seconds
+    And a file named "data/test.yml" with:
+      """
+      'ASDSFDa:
+       -asdf asdf
+      """
+    When I run `middleman build`
+    Then the output should contain:
+    """
+    failed due to an error:
+    """
+
+  Scenario: Invalid JSON
+    Given a fixture app "basic-data-app"
+    And the default aruba timeout is 30 seconds
+    And a file named "data/test.json" with:
+      """
+      'ASDSFDa:
+       -asdf asdf
+      """
+    When I run `middleman build`
+    Then the output should contain:
+    """
+    failed due to an error:
+    """

middleman-core/lib/middleman-core/core_extensions/data.rb

@@ -7,9 +7,7 @@ module Middleman
       class << self
         # @private
         def registered(app)
-          # Data formats
-          require 'yaml'
-          require 'json'
+          require 'middleman-core/core_extensions/data/file_loader'
 
           app.config.define_setting :data_dir, 'data', 'The directory data files are stored in'
           app.send :include, InstanceMethods
@@ -95,11 +93,9 @@ module Middleman
 
           data_path = full_path.relative_path_from(root + @app.config[:data_dir])
 
-          if %w(.yaml .yml).include?(extension)
-            data = YAML.load_file(full_path)
-          elsif extension == '.json'
-            data = JSON.parse(full_path.read)
-          else
+          begin
+            data = FileLoader.new.load(full_path)
+          rescue FileLoader::NoFileLoaderFoundError
             return
           end
 

middleman-core/lib/middleman-core/core_extensions/data/file_loader.rb

@@ -0,0 +1,71 @@
+require 'yaml'
+require 'json'
+
+module Middleman
+  module CoreExtensions
+    module Data
+      # Load data files
+      class FileLoader
+        # No parser available
+        class NoFileLoaderFoundError < StandardError; end
+
+        # Load yaml files
+        class YamlFileLoader
+          def match?(file)
+            %w(.yaml .yml).include? File.extname(file)
+          end
+
+          # @param [Pathname] file
+          def load(file)
+            YAML.load_file(file)
+          rescue Psych::SyntaxError, StandardError => e
+            $stderr.puts %(Loading data file "#{file}" failed due to an error: #{e.message})
+            {}
+          end
+        end
+
+        # Load json files
+        class JsonFileLoader
+          def match?(file)
+            '.json' == File.extname(file)
+          end
+
+          # @param [Pathname] file
+          def load(file)
+            JSON.parse(file.read)
+          rescue => e
+            $stderr.puts %(Loading data file "#{file}" failed due to an error: #{e.message})
+            {}
+          end
+        end
+
+        # Default loader
+        #
+        # Always fails
+        class NilFileLoader
+          def match?(file)
+            raise NoFileLoaderFoundError
+          end
+        end
+
+        private
+
+        attr_reader :loaders
+
+        public
+
+        def initialize
+          @loaders = []
+          @loaders << YamlFileLoader.new
+          @loaders << JsonFileLoader.new
+          @loaders << NilFileLoader.new
+        end
+
+        # Load file using loader
+        def load(file)
+          loaders.find { |l| l.match? file }.load(file)
+        end
+      end
+    end
+  end
+end