Repository: ViacomInc/data-point (js)

Issue:
Assigning an Invalid Transform does not yield enough information
When assigning an Invalid TransformExpression the error does not give enough information to know which is the Entity with the bad Transform. 

Current Error message is: 

```bash
Error: Transform [object Object] is not valid type (Array|String|Function)
```

Pull request:
feat(parsing): improves errors parse messages
closes #32

Patch:
diff --git a/packages/data-point/lib/reducer-entity/factory.js b/packages/data-point/lib/reducer-entity/factory.js
index d7cc472..29a9682 100644
--- a/packages/data-point/lib/reducer-entity/factory.js
+++ b/packages/data-point/lib/reducer-entity/factory.js
@@ -1,5 +1,5 @@
 'use strict'
-
+const _ = require('lodash')
 const REDUCER_ENTITY = (module.exports.type = 'ReducerEntity')
 
 /**
@@ -19,7 +19,9 @@ function ReducerEntity () {
 module.exports.ReducerEntity = ReducerEntity
 
 function isEntity (source) {
-  return source.match(/^[^$]([\w.]+):?([\w.]+)(\[])?/) !== null
+  return (
+    _.isString(source) && source.match(/^[^$]([\w.]+):?([\w.]+)(\[])?/) !== null
+  )
 }
 
 module.exports.isEntity = isEntity
diff --git a/packages/data-point/lib/reducer-function/factory.js b/packages/data-point/lib/reducer-function/factory.js
index 3895dc4..8d771fd 100644
--- a/packages/data-point/lib/reducer-function/factory.js
+++ b/packages/data-point/lib/reducer-function/factory.js
@@ -30,7 +30,10 @@ function isValidFunction (source) {
 module.exports.isValidFunction = isValidFunction
 
 function isFunction (source) {
-  return isValidFunction(source) || source.match(/^(\w[\w.]*)\((.*)\)/) !== null
+  return (
+    isValidFunction(source) ||
+    (_.isString(source) && source.match(/^(\w[\w.]*)\((.*)\)/) !== null)
+  )
 }
 
 module.exports.isFunction = isFunction
diff --git a/packages/data-point/lib/reducer/factory.js b/packages/data-point/lib/reducer/factory.js
index 37f321a..b73ecce 100644
--- a/packages/data-point/lib/reducer/factory.js
+++ b/packages/data-point/lib/reducer/factory.js
@@ -5,6 +5,7 @@ const _ = require('lodash')
 const ReducerPath = require('../reducer-path')
 const ReducerFunction = require('../reducer-function')
 const ReducerEntity = require('../reducer-entity')
+const util = require('util')
 
 const reducerTypes = [ReducerPath, ReducerFunction, ReducerEntity]
 
@@ -17,7 +18,15 @@ function create (source) {
   const reducer = _.find(reducerTypes, r => r.isType(source))
 
   if (_.isUndefined(reducer)) {
-    return new Error(`Invalid reducer type: ${source}`)
+    const message = [
+      'Invalid reducer type.',
+      ' Could not find a matching reducer type while parsing the value:\n ',
+      _.attempt(util.inspect, source),
+      '\nFor a list of supported types visit:\n',
+      'https://github.com/ViacomInc/data-point/tree/master/packages/data-point#reducers\n'
+    ].join('')
+
+    throw new Error(message)
   }
 
   return reducer.create(source)
diff --git a/packages/data-point/lib/stores/store-manager.js b/packages/data-point/lib/stores/store-manager.js
index 297ac69..c8ac8ab 100644
--- a/packages/data-point/lib/stores/store-manager.js
+++ b/packages/data-point/lib/stores/store-manager.js
@@ -59,7 +59,17 @@ function add (manager, errorInfoCb, factory, id, spec) {
   }
 
   const objSpec = utils.set(spec, 'id', id)
-  const item = factory(objSpec, id)
+  const item = _.attempt(factory, objSpec, id)
+
+  if (item instanceof Error) {
+    item.entityId = id
+    item.message = `${item.message}\nEntity "${id}": ${JSON.stringify(
+      spec,
+      null,
+      2
+    )}`
+    throw item
+  }
 
   manager.store.set(id, item)
 
diff --git a/packages/data-point/lib/transform-expression/factory.js b/packages/data-point/lib/transform-expression/factory.js
index 8530b3f..2c66732 100644
--- a/packages/data-point/lib/transform-expression/factory.js
+++ b/packages/data-point/lib/transform-expression/factory.js
@@ -2,6 +2,7 @@
 
 const _ = require('lodash')
 const reducerFactory = require('../reducer/factory')
+const util = require('util')
 
 /**
  * Describes the transform parts used to reduce a context
@@ -47,22 +48,41 @@ function parse (src) {
     source = [src]
   }
 
-  if (_.isArray(source)) {
-    return parseFromArray(source)
-  }
-
-  throw new Error(
-    `Transform ${source.toString()} is not valid type (Array|String|Function)`
-  )
+  return parseFromArray(source)
 }
+
 module.exports.parse = parse
 
+function isValid (source) {
+  const type = typeof source
+  return type === 'string' || type === 'function' || source instanceof Array
+}
+
+module.exports.isValid = isValid
+
+function validate (source) {
+  const isValidTransform = isValid(source)
+  if (isValidTransform) {
+    return true
+  }
+  const message = [
+    `Could not parse a TransformExpression. The TransformExpression:\n `,
+    _.attempt(util.inspect, source),
+    '\nis not using a valid type, try using an Array, String or a Function.',
+    '\nMore info: https://github.com/ViacomInc/data-point/tree/master/packages/data-point#transform-expression\n'
+  ].join('')
+  throw new Error(message)
+}
+
+module.exports.validate = validate
+
 /**
  * parses a raw transform
  * @param  {string} transformRaw raw value path to be parsed
  * @return {Transform}
  */
-function create (source) {
+function create (source = []) {
+  validate(source)
   const tokens = parse(source)
 
   const transformBase = new TransformExpression()


Test patch:
diff --git a/packages/data-point/lib/reducer/__snapshots__/factory.test.js.snap b/packages/data-point/lib/reducer/__snapshots__/factory.test.js.snap
new file mode 100644
index 0000000..6b6b84d
--- /dev/null
+++ b/packages/data-point/lib/reducer/__snapshots__/factory.test.js.snap
@@ -0,0 +1,9 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`reducer#create  detect invalid 1`] = `
+"Invalid reducer type. Could not find a matching reducer type while parsing the value:
+ 'a'
+For a list of supported types visit:
+https://github.com/ViacomInc/data-point/tree/master/packages/data-point#reducers
+"
+`;
diff --git a/packages/data-point/lib/reducer/factory.test.js b/packages/data-point/lib/reducer/factory.test.js
index 2b273ed..e68894c 100644
--- a/packages/data-point/lib/reducer/factory.test.js
+++ b/packages/data-point/lib/reducer/factory.test.js
@@ -30,7 +30,8 @@ describe('reducer#create', () => {
   })
 
   test(' detect invalid', () => {
-    const reducer = Factory.create('a')
-    expect(reducer).toBeInstanceOf(Error)
+    expect(() => {
+      Factory.create('a')
+    }).toThrowErrorMatchingSnapshot()
   })
 })
diff --git a/packages/data-point/lib/stores/__snapshots__/store-manager.test.js.snap b/packages/data-point/lib/stores/__snapshots__/store-manager.test.js.snap
new file mode 100644
index 0000000..9f9fc7a
--- /dev/null
+++ b/packages/data-point/lib/stores/__snapshots__/store-manager.test.js.snap
@@ -0,0 +1,10 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`add It should throw error with information if item already exist 1`] = `[Foo: Bar]`;
+
+exports[`add It should throw error with information if item already exist 2`] = `
+[Error: factory error
+Entity "id1": {
+  "spec": "mySpec"
+}]
+`;
diff --git a/packages/data-point/lib/stores/store-manager.test.js b/packages/data-point/lib/stores/store-manager.test.js
new file mode 100644
index 0000000..a234624
--- /dev/null
+++ b/packages/data-point/lib/stores/store-manager.test.js
@@ -0,0 +1,70 @@
+/* eslint-env jest */
+
+const _ = require('lodash')
+const ObjectStoreManager = require('./store-manager')
+
+function createManager () {
+  return {
+    store: new Map()
+  }
+}
+
+describe('add', () => {
+  test('It should throw error with information if item already exist', () => {
+    const mgr = createManager()
+    const errorInfoCb = () => ({
+      name: 'Foo',
+      message: 'Bar'
+    })
+    const Factory = () => 1
+    ObjectStoreManager.add(mgr, errorInfoCb, Factory, 'id1')
+
+    const error = _.attempt(
+      ObjectStoreManager.add,
+      mgr,
+      errorInfoCb,
+      Factory,
+      'id1'
+    )
+
+    expect(error).toBeInstanceOf(Error)
+    expect(error).toHaveProperty('name', 'Foo')
+    expect(error).toMatchSnapshot()
+  })
+
+  test('It should throw error with information if item already exist', () => {
+    const mgr = createManager()
+    const errorInfoCb = () => ({
+      name: 'Foo',
+      message: 'Bar'
+    })
+    const Factory = () => {
+      throw new Error('factory error')
+    }
+
+    const error = _.attempt(
+      ObjectStoreManager.add,
+      mgr,
+      errorInfoCb,
+      Factory,
+      'id1',
+      { spec: 'mySpec' }
+    )
+
+    expect(error).toBeInstanceOf(Error)
+    expect(error).toHaveProperty('entityId', 'id1')
+    expect(error).toMatchSnapshot()
+  })
+
+  test('It should throw error with information if item already exist', () => {
+    const mgr = createManager()
+    const errorInfoCb = () => ({
+      name: 'Foo',
+      message: 'Bar'
+    })
+    const Factory = (spec, id) => `${id}:${spec.type}`
+
+    ObjectStoreManager.add(mgr, errorInfoCb, Factory, 'id1', { type: 'myType' })
+    expect(mgr.store.get('id1')).toEqual('id1:myType')
+  })
+})
diff --git a/packages/data-point/lib/transform-expression/__snapshots__/factory.test.js.snap b/packages/data-point/lib/transform-expression/__snapshots__/factory.test.js.snap
new file mode 100644
index 0000000..7a381cf
--- /dev/null
+++ b/packages/data-point/lib/transform-expression/__snapshots__/factory.test.js.snap
@@ -0,0 +1,9 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`validate It should throw error if transform is invalid 1`] = `
+"Could not parse a TransformExpression. The TransformExpression:
+ true
+is not using a valid type, try using an Array, String or a Function.
+More info: https://github.com/ViacomInc/data-point/tree/master/packages/data-point#transform-expression
+"
+`;
diff --git a/packages/data-point/lib/transform-expression/factory.test.js b/packages/data-point/lib/transform-expression/factory.test.js
index 536d956..4e927e1 100644
--- a/packages/data-point/lib/transform-expression/factory.test.js
+++ b/packages/data-point/lib/transform-expression/factory.test.js
@@ -4,6 +4,32 @@
 const _ = require('lodash')
 const TransformFactory = require('./factory')
 
+describe('isValid', () => {
+  test('It should return true if type is valid', () => {
+    expect(TransformFactory.isValid('string')).toEqual(true)
+    expect(TransformFactory.isValid(() => 1)).toEqual(true)
+    expect(TransformFactory.isValid([])).toEqual(true)
+  })
+
+  test('should return false if invalid', () => {
+    expect(TransformFactory.isValid(1)).toEqual(false)
+    expect(TransformFactory.isValid(true)).toEqual(false)
+    expect(TransformFactory.isValid({})).toEqual(false)
+  })
+})
+
+describe('validate', () => {
+  test('It should throw error if transform is invalid', () => {
+    expect(() => {
+      TransformFactory.validate(true)
+    }).toThrowErrorMatchingSnapshot()
+  })
+
+  test('it should return true if valid', () => {
+    expect(TransformFactory.validate('$.')).toEqual(true)
+  })
+})
+
 describe('TransformFactory#create', () => {
   test('TransformFactory#create default', () => {
     const result = TransformFactory.create()