fix(errors): include request data on unexpected errors

docs/api.md

@@ -312,8 +312,6 @@ for `code` and `errno` are:
   A backend service request failed.
 * `code: 500, errno: 998`:
   An internal validation check failed.
-* `code: 500, errno: 999`:
-  Unspecified error
 
 The following errors
 include additional response properties:

lib/error.js

@@ -86,7 +86,7 @@ var ERRNO = {
   UNEXPECTED_ERROR: 999
 }
 
-var DEFAULTS = {
+const DEFAULTS = {
   code: 500,
   error: 'Internal Server Error',
   errno: ERRNO.UNEXPECTED_ERROR,
@@ -94,9 +94,9 @@ var DEFAULTS = {
   info: 'https://github.com/mozilla/fxa-auth-server/blob/master/docs/api.md#response-format'
 }
 
-var TOO_LARGE = /^Payload (?:content length|size) greater than maximum allowed/
+const TOO_LARGE = /^Payload (?:content length|size) greater than maximum allowed/
 
-var BAD_SIGNATURE_ERRORS = [
+const BAD_SIGNATURE_ERRORS = [
   'Bad mac',
   'Unknown algorithm',
   'Missing required payload hash',
@@ -144,7 +144,7 @@ AppError.prototype.backtrace = function (traced) {
 /**
   Translates an error from Hapi format to our format
 */
-AppError.translate = function (response) {
+AppError.translate = function (request, response) {
   var error
   if (response instanceof AppError) {
     return response
@@ -152,7 +152,7 @@ AppError.translate = function (response) {
   var payload = response.output.payload
   var reason = response.reason
   if (! payload) {
-    error = new AppError({})
+    error = AppError.unexpectedError(request)
   } else if (payload.statusCode === 500 && /(socket hang up|ECONNREFUSED)/.test(reason)) {
     // A connection to a remote service either was not made or timed out.
     error = AppError.backendServiceFailure()
@@ -194,6 +194,9 @@ AppError.translate = function (response) {
       info: payload.info,
       stack: response.stack
     })
+    if (payload.statusCode >= 500) {
+      decorateErrorWithRequest(error, request)
+    }
   }
   return error
 }
@@ -898,9 +901,27 @@ AppError.internalValidationError = (op, data) => {
   })
 }
 
-AppError.unexpectedError = () => {
-  return new AppError({})
+AppError.unexpectedError = request => {
+  const error = new AppError({})
+  decorateErrorWithRequest(error, request)
+  return error
 }
 
 module.exports = AppError
 module.exports.ERRNO = ERRNO
+
+function decorateErrorWithRequest (error, request) {
+  if (request) {
+    error.output.payload.request = {
+      // request.app.devices and request.app.metricsContext are async, so can't be included here
+      acceptLanguage: request.app.acceptLanguage,
+      locale: request.app.locale,
+      userAgent: request.app.ua,
+      method: request.method,
+      path: request.path,
+      query: request.query,
+      payload: request.payload,
+      headers: request.headers
+    }
+  }
+}

lib/server.js

@@ -250,7 +250,7 @@ async function create (log, error, config, routes, db, oauthdb, translator) {
     let response = request.response
     if (response.isBoom) {
       logEndpointErrors(response, log)
-      response = error.translate(response)
+      response = error.translate(request, response)
       if (config.env !== 'prod') {
         response.backtrace(request.app.traced)
       }

test/local/error.js

@@ -5,8 +5,9 @@
 'use strict'
 
 const { assert } = require('chai')
-var messages = require('joi/lib/language')
+const messages = require('joi/lib/language')
 const AppError = require('../../lib/error')
+const P = require('../../lib/promise')
 
 describe('AppErrors', () => {
 
@@ -24,7 +25,7 @@ describe('AppErrors', () => {
       assert.equal(typeof AppError, 'function')
       assert.equal(AppError.length, 3)
       assert.equal(typeof AppError.translate, 'function')
-      assert.equal(AppError.translate.length, 1)
+      assert.lengthOf(AppError.translate, 2)
       assert.equal(typeof AppError.invalidRequestParameter, 'function')
       assert.equal(AppError.invalidRequestParameter.length, 1)
       assert.equal(typeof AppError.missingRequestParameter, 'function')
@@ -35,7 +36,7 @@ describe('AppErrors', () => {
   it(
     'should translate with missing required parameters',
     () => {
-      var result = AppError.translate({
+      var result = AppError.translate(null, {
         output: {
           payload: {
             message: 'foo' + messages.errors.any.required,
@@ -59,7 +60,7 @@ describe('AppErrors', () => {
   it(
     'should translate with invalid parameter',
     () => {
-      var result = AppError.translate({
+      var result = AppError.translate(null, {
         output: {
           payload: {
             validation: 'foo'
@@ -80,7 +81,7 @@ describe('AppErrors', () => {
   it(
     'should translate with missing payload',
     () => {
-      var result = AppError.translate({
+      var result = AppError.translate(null, {
         output: {}
       })
       assert.ok(result instanceof AppError, 'instanceof AppError')
@@ -112,10 +113,76 @@ describe('AppErrors', () => {
     }
   )
 
+  it('unexpectedError without request data', () => {
+    const err = AppError.unexpectedError()
+    assert.instanceOf(err, AppError)
+    assert.instanceOf(err, Error)
+    assert.equal(err.errno, 999)
+    assert.equal(err.message, 'Unspecified error')
+    assert.equal(err.output.statusCode, 500)
+    assert.equal(err.output.payload.error, 'Internal Server Error')
+    assert.isUndefined(err.output.payload.request)
+  })
+
+  it('unexpectedError with request data', () => {
+    const err = AppError.unexpectedError({
+      app: {
+        acceptLanguage: 'en, fr',
+        locale: 'en',
+        geo: {
+          city: 'Mountain View',
+          state: 'California'
+        },
+        ua: {
+          os: 'Android',
+          osVersion: '9'
+        },
+        devices: P.resolve([ { id: 1 } ]),
+        metricsContext: P.resolve({
+          service: 'sync'
+        })
+      },
+      method: 'GET',
+      path: '/v1/wibble',
+      query: {
+        foo: 'bar'
+      },
+      payload: {
+        baz: 'qux'
+      },
+      headers: {
+        wibble: 'blee'
+      }
+    })
+    assert.equal(err.errno, 999)
+    assert.equal(err.message, 'Unspecified error')
+    assert.equal(err.output.statusCode, 500)
+    assert.equal(err.output.payload.error, 'Internal Server Error')
+    assert.deepEqual(err.output.payload.request, {
+      acceptLanguage: 'en, fr',
+      locale: 'en',
+      userAgent: {
+        os: 'Android',
+        osVersion: '9'
+      },
+      method: 'GET',
+      path: '/v1/wibble',
+      query: {
+        foo: 'bar'
+      },
+      payload: {
+        baz: 'qux'
+      },
+      headers: {
+        wibble: 'blee'
+      }
+    })
+  })
+
   const reasons = ['socket hang up', 'ECONNREFUSED'];
   reasons.forEach((reason) => {
     it(`converts ${reason} errors to backend service error`, () => {
-      const result = AppError.translate({
+      const result = AppError.translate(null, {
         output: {
           payload: {
             errno: 999,

test/local/server.js

@@ -389,7 +389,7 @@ describe('lib/server', () => {
           })
 
           it('called log.error correctly', () => {
-            assert.equal(log.error.callCount, 1)
+            assert.isAtLeast(log.error.callCount, 1)
             const args = log.error.args[0]
             assert.equal(args.length, 1)
             assert.deepEqual(args[0], {