fix: log errors when validators throw (#41)

Allows validators to provide custom error messages
eeveebank · May 20, 2024 · 8c91ba5 · 8c91ba5
2 parents d5c5399 + cf92712
commit 8c91ba5
Show file tree

Hide file tree

Showing 3 changed files with 87 additions and 17 deletions.
diff --git a/lib/mergeDeep.js b/lib/mergeDeep.js
@@ -6,7 +6,7 @@ const NAME_USERNAME_PROPERTY = item => NAME_FIELDS.find(prop => Object.prototype
 const GET_NAME_USERNAME_PROPERTY = item => { if (NAME_USERNAME_PROPERTY(item)) return item[NAME_USERNAME_PROPERTY(item)] }
 
 class MergeDeep {
-  constructor (log, github, ignorableFields, configvalidators = {}, overridevalidators = {}) {
+  constructor (log, github, ignorableFields) {
     this.log = log
     this.github = github
     this.ignorableFields = ignorableFields
@@ -269,20 +269,30 @@ class MergeDeep {
       return
     }
     if (this.overridevalidators[key]) {
-      // this.log.debug(`Calling overridevalidator for key ${key} `)
-      if (!this.overridevalidators[key].canOverride(baseconfig, overrideconfig, this.github)) {
-        this.log.error(`Error in calling overridevalidator for key ${key} ${this.overridevalidators[key].error}`)
-        throw new Error(this.overridevalidators[key].error)
+      this.log.debug(`MergeDeep calling overridevalidator for key ${key} `)
+      try {
+        const canOverride = this.overridevalidators[key].canOverride(baseconfig, overrideconfig, this.github)
+        if (!canOverride) {
+          throw new Error(this.overridevalidators[key].error)
+        }
+      } catch (error) {
+        this.log.error(`MergeDeep Error in calling overridevalidator for key ${key} ${error}`)
+        throw error
       }
     }
   }
 
   validateConfig (key, baseconfig) {
     if (this.configvalidators[key]) {
-      // this.log.debug(`Calling configvalidator for key ${key} `)
-      if (!this.configvalidators[key].isValid(baseconfig, this.github)) {
-        this.log.error(`Error in calling configvalidator for key ${key} ${this.configvalidators[key].error}`)
-        throw new Error(this.configvalidators[key].error)
+      this.log.debug(`MergeDeep calling configvalidator for key ${key} `)
+      try {
+        const isValid = this.configvalidators[key].isValid(baseconfig, this.github)
+        if (!isValid) {
+          throw new Error(this.configvalidators[key].error)
+        }
+      } catch (error) {
+        this.log.error(`MergeDeep Error in calling configvalidator for key ${key} ${error}`)
+        throw error
       }
     }
   }

diff --git a/lib/settings.js b/lib/settings.js
@@ -443,17 +443,27 @@ ${stripAllWhitespace(JSON.stringify(quietResults, null, 2))}
     const configValidator = this.configvalidators[section]
     if (configValidator) {
       this.log.debug(`Calling configvalidator for key ${section} `)
-      if (!configValidator.isValid(overrideConfig, this.github)) {
-        this.log.error(`Error in calling configvalidator for key ${section} ${configValidator.error}`)
-        throw new Error(configValidator.error)
+      try {
+        const isValid = configValidator.isValid(overrideConfig, this.github)
+        if (!isValid) {
+          throw new Error(configValidator.error)
+        }
+      } catch (error) {
+        this.log.error(`Error in calling configvalidator for key ${section} ${error}`)
+        throw error
       }
     }
     const overridevalidator = this.overridevalidators[section]
     if (overridevalidator) {
       this.log.debug(`Calling overridevalidator for key ${section} `)
-      if (!overridevalidator.canOverride(baseConfig, overrideConfig, this.github)) {
-        this.log.error(`Error in calling overridevalidator for key ${section} ${overridevalidator.error}`)
-        throw new Error(overridevalidator.error)
+      try {
+        const canOverride = overridevalidator.canOverride(baseConfig, overrideConfig, this.github)
+        if (!canOverride) {
+          throw new Error(overridevalidator.error)
+        }
+      } catch (error) {
+        this.log.error(`Error in calling overridevalidator for key ${section} ${error}`)
+        throw error
       }
     }
   }

diff --git a/test/unit/lib/validator.test.js b/test/unit/lib/validator.test.js
@@ -2,9 +2,14 @@
 const DeploymentConfig = require('../../../lib/deploymentConfig')
 const MergeDeep = require('../../../lib/mergeDeep')
 const YAML = require('js-yaml')
-const log = require('pino')('test.log')
+
+const log = { debug: jest.fn(), error: jest.fn() }
 
 describe('Validator Tests', () => {
+  beforeEach(() => {
+    jest.clearAllMocks()
+  })
+
   it('Branch override validator test', () => {
     const overrideMock = jest.fn((baseconfig, overrideconfig) => {
       if (baseconfig.protection.required_pull_request_reviews.required_approving_review_count && overrideconfig.protection.required_pull_request_reviews.required_approving_review_count) {
@@ -113,7 +118,7 @@ describe('Validator Tests', () => {
 
     try {
       const ignorableFields = []
-      const mergeDeep = new MergeDeep(log, ignorableFields)
+      const mergeDeep = new MergeDeep(log, {}, ignorableFields)
       mergeDeep.mergeDeep(baseconfig, overrideconfig)
     } catch (err) {
       expect(err).toBeDefined()
@@ -176,4 +181,49 @@ describe('Validator Tests', () => {
     }
     expect(configMock.mock.calls.length).toBe(1)
   })
+
+  it('override validator supports custom messages', () => {
+    const overrideMock = jest.fn(() => {
+      throw new Error('Custom message')
+    })
+
+    const configMock = jest.fn((baseconfig) => {
+      console.log(`Branch config validator, baseconfig ${baseconfig}`)
+      return false
+    })
+    DeploymentConfig.overridevalidators = { branches: { canOverride: overrideMock, error: 'Branch overrideValidators.error' } }
+    DeploymentConfig.configvalidators = { branches: { isValid: configMock, error: 'Branch configValidators.error' } }
+
+    const overrideconfig = {
+      branches: {}
+    }
+    const baseconfig = {
+      branches: {}
+    }
+
+    const mergeDeep = new MergeDeep(log, {}, [])
+    expect(() => mergeDeep.mergeDeep(baseconfig, overrideconfig)).toThrow('Custom message')
+    expect(log.error).toHaveBeenCalledWith(`MergeDeep Error in calling overridevalidator for key branches Error: Custom message`)
+  })
+
+  it('config validator supports custom messages', () => {
+    const overrideMock = jest.fn(() => true)
+
+    const configMock = jest.fn(() => {
+      throw new Error('Custom message')
+    })
+    DeploymentConfig.overridevalidators = { branches: { canOverride: overrideMock, error: 'Branch overrideValidators.error' } }
+    DeploymentConfig.configvalidators = { branches: { isValid: configMock, error: 'Branch configValidators.error' } }
+
+    const overrideconfig = {
+      branches: {}
+    }
+    const baseconfig = {
+      branches: {}
+    }
+
+    const mergeDeep = new MergeDeep(log, {}, [])
+    expect(() => mergeDeep.mergeDeep(baseconfig, overrideconfig)).toThrow('Custom message')
+    expect(log.error).toHaveBeenCalledWith(`MergeDeep Error in calling configvalidator for key branches Error: Custom message`)
+  })
 })