fix(errors): add extra data to internal validation errors

docs/api.md

@@ -343,6 +343,7 @@ include additional response properties:
 * `errno: 201`: retryAfter
 * `errno: 202`: retryAfter
 * `errno: 203`: service, operation
+* `errno: 998`: op, data
 
 #### Responses from intermediary servers
 <!--begin-responses-from-intermediary-servers-->

lib/backendService.js

@@ -132,7 +132,7 @@ module.exports = function createBackendServiceAPI(log, config, serviceName, meth
             message: err.message,
             value
           })
-          reject(error.internalValidationError())
+          reject(error.internalValidationError(fullMethodName, { location, value }))
         })
       })
     }

lib/error.js

@@ -886,12 +886,15 @@ AppError.backendServiceFailure = (service, operation) => {
   })
 }
 
-AppError.internalValidationError = () => {
+AppError.internalValidationError = (op, data) => {
   return new AppError({
     code: 500,
     error: 'Internal Server Error',
     errno: ERRNO.INTERNAL_VALIDATION_ERROR,
     message: 'An internal validation check failed.'
+  }, {
+    op,
+    data
   })
 }
 

lib/safe-url.js

@@ -5,7 +5,7 @@
 // This module exports a safe URL-builder interface, ensuring that no
 // unsafe input can leak into generated URLs.
 //
-// It takes the approach of throwing error.unexpectedError() when unsafe
+// It takes the approach of throwing error.internalValidationError() when unsafe
 // input is encountered, for extra visibility. An alternative approach
 // would be to use encodeURIComponent instead to convert unsafe input on
 // the fly. However, we have no valid use case for encoding weird data
@@ -21,9 +21,9 @@
 //   url.render({ uid: 'foo' })               // returns '/account/foo/sessions'
 //   url.render({ uid: 'bar' })               // returns '/account/bar/sessions'
 //   url.render({ uid: 'bar' }, {foo: 'baz'}) // returns '/account/bar/sessions?foo=baz'
-//   url.render({ uid: 'foo\n' })             // throws error.unexpectedError()
-//   url.render({})                           // throws error.unexpectedError()
-//   url.render({ uid: 'foo', id: 'bar' })    // throws error.unexpectedError()
+//   url.render({ uid: 'foo\n' })             // throws error.internalValidationError()
+//   url.render({})                           // throws error.internalValidationError()
+//   url.render({ uid: 'foo', id: 'bar' })    // throws error.internalValidationError()
 
 'use strict'
 
@@ -87,6 +87,6 @@ module.exports = log => class SafeUrl {
 
   _fail (op, data) {
     log.error(Object.assign({ op, caller: this._caller }, data))
-    throw error.internalValidationError()
+    throw error.internalValidationError(op, data)
   }
 }

test/local/backendService.js

@@ -123,6 +123,13 @@ describe('createBackendServiceAPI', () => {
       assert.fail('should have thrown')
     } catch (err) {
       assert.equal(err.errno, error.ERRNO.INTERNAL_VALIDATION_ERROR)
+      assert.equal(err.output.payload.op, 'mock-service.testSimplePost')
+      assert.deepEqual(err.output.payload.data, {
+        location: 'request',
+        value: {
+          foo: 123
+        }
+      })
       assert.equal(log.error.callCount, 1, 'an error was logged')
       assert.equal(log.error.getCall(0).args[0].op, 'mock-service.testSimplePost')
       assert.equal(log.error.getCall(0).args[0].error, 'request schema validation failed')
@@ -136,6 +143,14 @@ describe('createBackendServiceAPI', () => {
       assert.fail('should have thrown')
     } catch (err) {
       assert.equal(err.errno, error.ERRNO.INTERNAL_VALIDATION_ERROR)
+      assert.equal(err.output.payload.op, 'mock-service.testGetWithValidation')
+      assert.deepEqual(err.output.payload.data, {
+        location: 'params',
+        value: {
+          first: 'ABC',
+          second: '123'
+        }
+      })
       assert.equal(log.error.callCount, 1, 'an error was logged')
       assert.equal(log.error.getCall(0).args[0].op, 'mock-service.testGetWithValidation')
       assert.equal(log.error.getCall(0).args[0].error, 'params schema validation failed')
@@ -172,6 +187,13 @@ describe('createBackendServiceAPI', () => {
       assert.fail('should have thrown')
     } catch (err) {
       assert.equal(err.errno, error.ERRNO.INTERNAL_VALIDATION_ERROR)
+      assert.equal(err.output.payload.op, 'mock-service.testGetWithValidation')
+      assert.deepEqual(err.output.payload.data, {
+        location: 'query',
+        value: {
+          foo: 123
+        }
+      })
       assert.equal(log.error.callCount, 1, 'an error was logged')
       assert.equal(log.error.getCall(0).args[0].op, 'mock-service.testGetWithValidation')
       assert.equal(log.error.getCall(0).args[0].error, 'query schema validation failed')

test/local/safe-url.js

@@ -46,7 +46,18 @@ describe('require:', () => {
     })
 
     it('logs an error and throws when param is missing', () => {
-      assert.throws(() => safeUrl.render({}))
+      let threw = false
+      try {
+        safeUrl.render({})
+      } catch (err) {
+        threw = true
+        assert.equal(err.output.payload.op, 'safeUrl.params.mismatch')
+        assert.deepEqual(err.output.payload.data, {
+          expected: [ 'bar' ],
+          keys: []
+        })
+      }
+      assert.equal(threw, true)
       assert.equal(log.error.callCount, 1)
       assert.deepEqual(log.error.args[0][0], {
         op: 'safeUrl.params.mismatch',
@@ -68,7 +79,19 @@ describe('require:', () => {
     })
 
     it('logs an error and throws when param is empty string', () => {
-      assert.throws(() => safeUrl.render({ bar: '' }))
+      let threw = false
+      try {
+        safeUrl.render({ bar: '' })
+      } catch (err) {
+        threw = true
+        assert.equal(err.output.payload.op, 'safeUrl.bad')
+        assert.deepEqual(err.output.payload.data, {
+          location: 'paramVal',
+          key: 'bar',
+          value: ''
+        })
+      }
+      assert.equal(threw, true)
       assert.equal(log.error.callCount, 1)
       assert.deepEqual(log.error.args[0][0], {
         location: 'paramVal',
@@ -106,7 +129,19 @@ describe('require:', () => {
     })
 
     it('logs an error and throws for bad query keys', () => {
-      assert.throws(() => safeUrl.render({ bar: 'baz' }, {'💩': 'bar'}))
+      let threw = false
+      try {
+        safeUrl.render({ bar: 'baz' }, {'💩': 'bar'})
+      } catch (err) {
+        threw = true
+        assert.equal(err.output.payload.op, 'safeUrl.unsafe')
+        assert.deepEqual(err.output.payload.data, {
+          location: 'queryKey',
+          key: '💩',
+          value: '💩'
+        })
+      }
+      assert.equal(threw, true)
       assert.equal(log.error.callCount, 1)
       assert.deepEqual(log.error.args[0][0], {
         location: 'queryKey',