Fix bad error messaging when JS package ownership fails (#17)

* Improve error messaging when JS package ownership fails

* bump version

* add error

* fix tests

Author: Alex Evanczuk

Gemfile.lock

@@ -1,7 +1,7 @@
 PATH
   remote: .
   specs:
-    code_ownership (1.28.0)
+    code_ownership (1.28.1)
       code_teams (~> 1.0)
       parse_packwerk
       sorbet-runtime
@@ -15,7 +15,7 @@ GEM
     coderay (1.1.3)
     diff-lcs (1.4.4)
     method_source (1.0.0)
-    parse_packwerk (0.10.1)
+    parse_packwerk (0.11.0)
       sorbet-runtime
     parser (3.1.2.0)
       ast (~> 2.4.1)

README.md

@@ -34,6 +34,29 @@ File annotations are a last resort if there is no clear home for your code. File
 ```ruby
 # @team MyTeam
 ```
+
+### Javascript Package Ownership
+Javascript package based ownership allows you to specify an owenrship key in a `package.json`. To use this, configure your `package.json` like this:
+
+```json
+{
+  // other keys
+  "metadata": {
+    "owner": "My Team"
+  }
+  // other keys
+}
+```
+
+You can also tell `code_ownership` where to find JS packages in the configuration, like this:
+```yml
+js_package_paths:
+  - frontend/javascripts/packages/*
+  - frontend/other_location_for_packages/*
+```
+
+This defaults `**/`, which makes it look for `package.json` files across your application.
+
 ## Usage: Reading CodeOwnership
 ### `for_file`
 `CodeOwnership.for_file`, given a relative path to a file returns a `CodeTeams::Team` if there is a team that owns the file, `nil` otherwise.

code_ownership.gemspec

@@ -1,6 +1,6 @@
 Gem::Specification.new do |spec|
   spec.name          = "code_ownership"
-  spec.version       = '1.28.0'
+  spec.version       = '1.28.1'
   spec.authors       = ['Gusto Engineers']
   spec.email         = ['[email protected]']
   spec.summary       = 'A gem to help engineering teams declare ownership of code'

lib/code_ownership/private/parse_js_packages.rb

@@ -32,6 +32,16 @@ def self.from(pathname)
             name: package_name,
             metadata: package_loaded_json[METADATA] || {}
           )
+        rescue JSON::ParserError => e
+          error_message = <<~MESSAGE
+            #{e.inspect}
+
+            #{pathname} has invalid JSON, so code ownership cannot be determined.
+
+            Please either make the JSON in that file valid or specify `js_package_paths` in config/code_ownership.yml.
+          MESSAGE
+
+          raise InvalidCodeOwnershipConfigurationError.new(error_message)
         end
 
         sig { returns(Pathname) }

spec/lib/code_ownership_spec.rb

@@ -634,6 +634,29 @@
         end
       end
     end
+
+    context 'application has invalid JSON in package' do
+      before do
+        write_file('config/code_ownership.yml', {}.to_yaml)
+
+        write_file('frontend/javascripts/my_package/package.json', <<~CONTENTS)
+          { syntax error!!!
+            "metadata": {
+              "owner": "Foo"
+            }
+          }
+        CONTENTS
+      end
+
+      it 'lets the user know the their package JSON is invalid' do
+        expect { CodeOwnership.validate! }.to raise_error do |e|
+          expect(e).to be_a CodeOwnership::InvalidCodeOwnershipConfigurationError
+          expect(e.message).to match /JSON::ParserError.*?unexpected token/
+          expect(e.message).to include 'frontend/javascripts/my_package/package.json has invalid JSON, so code ownership cannot be determined.'
+          expect(e.message).to include 'Please either make the JSON in that file valid or specify `js_package_paths` in config/code_ownership.yml.'
+        end
+      end
+    end
   end
 
   describe '.for_file' do