Merge pull request #3295 from mozilla/feat/include-backend-error-data

feat(auth-server): add more backend error data

packages/fxa-auth-server/lib/error.js

@@ -218,7 +218,16 @@ AppError.translate = function(request, response) {
     /(socket hang up|ECONNREFUSED)/.test(reason)
   ) {
     // A connection to a remote service either was not made or timed out.
-    error = AppError.backendServiceFailure();
+    if (response instanceof Error) {
+      error = AppError.backendServiceFailure(
+        undefined,
+        undefined,
+        undefined,
+        response
+      );
+    } else {
+      error = AppError.backendServiceFailure();
+    }
   } else if (payload.statusCode === 401) {
     // These are common errors generated by Hawk auth lib.
     if (

packages/fxa-auth-server/lib/sentry.js

@@ -88,6 +88,9 @@ async function configureSentry(server, config) {
       beforeSend(event, hint) {
         return filterSentryEvent(event, hint);
       },
+      integrations: [
+        new Sentry.Integrations.LinkedErrors({ key: 'jse_cause' }),
+      ],
     });
     Sentry.configureScope(scope => {
       scope.setTag('process', 'key_server');
@@ -116,11 +119,6 @@ async function configureSentry(server, config) {
           }
         }
 
-        // If it's a verror replace the stack with the full stack
-        if (err instanceof verror) {
-          err.stack = verror.fullStack(err);
-        }
-
         Sentry.withScope(scope => {
           scope.addEventProcessor(_sentryEvent => {
             const sentryEvent = Sentry.Handlers.parseRequest(
@@ -131,6 +129,33 @@ async function configureSentry(server, config) {
             return sentryEvent;
           });
           scope.setExtra('exception', exception);
+          const cause = verror.cause(err);
+          if (cause && cause.message) {
+            const causeContext = {
+              errorName: cause.name,
+              reason: cause.reason,
+              errorMessage: cause.message,
+            };
+
+            // Poolee EndpointError's have a few other things and oddly don't include
+            // a stack at all. We try and extract a bit more to reflect what actually
+            // happened as 'socket hang up' is somewhat inaccurate when the remote server
+            // throws a 500.
+            const output = cause.output;
+            if (output && output.payload) {
+              for (const key of ['error', 'message', 'statusCode']) {
+                causeContext[key] = output.payload[key];
+              }
+            }
+            const attempt = cause.attempt;
+            if (attempt) {
+              causeContext.method = attempt.method;
+              causeContext.path = attempt.path
+                ? attempt.path.replace(TOKENREGEX, FILTERED)
+                : null;
+            }
+            scope.setContext('cause', causeContext);
+          }
 
           // Merge the request scope into the temp scope
           Hoek.merge(scope, request.sentryScope);

packages/fxa-auth-server/test/local/sentry.js

@@ -5,30 +5,87 @@
 'use strict';
 
 const { assert } = require('chai');
-
+const EndpointError = require('poolee/lib/error')(require('util').inherits);
+const sinon = require('sinon');
+const verror = require('verror');
 const Hapi = require('hapi');
+const Sentry = require('@sentry/node');
 
 const config = require('../../config').getProperties();
-const _configureSentry = require('../../lib/server')._configureSentry;
-const server = new Hapi.Server({});
+const configureSentry = require('../../lib/sentry').configureSentry;
+
+const sandbox = sinon.createSandbox();
 
 describe('Sentry', () => {
-  let sentryDsn;
+  let server;
 
   beforeEach(() => {
-    sentryDsn = config.sentryDsn;
+    server = new Hapi.Server({});
   });
 
   afterEach(() => {
-    config.sentryDsn = sentryDsn;
+    sandbox.restore();
   });
 
-  it('can be set up when sentry is enabled', () => {
+  it('can be set up when sentry is enabled', async () => {
     config.sentryDsn = 'https://deadbeef:deadbeef@127.0.0.1/123';
-    assert.doesNotThrow(() => _configureSentry(server, config));
+    let throws = false;
+    try {
+      await configureSentry(server, config);
+    } catch (err) {
+      throws = true;
+    }
+    assert.equal(throws, false);
   });
 
-  it('can be set up when sentry is not enabled', () => {
-    assert.doesNotThrow(() => _configureSentry(server, config));
+  it('can be set up when sentry is not enabled', async () => {
+    let throws = false;
+    try {
+      await configureSentry(server, config);
+    } catch (err) {
+      throws = true;
+    }
+    assert.equal(throws, false);
+  });
+
+  it('adds EndpointError details to a reported error', async () => {
+    config.sentryDsn = 'https://deadbeef:deadbeef@127.0.0.1/123';
+    await configureSentry(server, config);
+    const endError = new EndpointError(
+      'An internal server error has occurred',
+      {
+        reason: 'connect refused',
+        attempt: {
+          method: 'PUT',
+          path: '/account/',
+        },
+      }
+    );
+    endError.output = {
+      error: '127.0.0.1 error',
+      message: 'Underlying error',
+      statusCode: 500,
+    };
+    const sentryCaptureSpy = sandbox.stub(Sentry, 'captureException');
+    const scopeContextSpy = sinon.fake();
+    const scopeSpy = {
+      addEventProcessor: sinon.fake(),
+      setContext: scopeContextSpy,
+      setExtra: sinon.fake(),
+    };
+    sandbox.replace(Sentry, 'withScope', fn => fn(scopeSpy));
+    const fullError = new verror.WError(endError, 'Something bad happened');
+    await server.events.emit({ name: 'request', channel: 'error' }, [
+      {},
+      { error: fullError },
+    ]);
+
+    sentryCaptureSpy.calledOnceWith(fullError);
+    assert.equal(scopeContextSpy.calledOnce, true);
+    const ctx = scopeContextSpy.args[0][1];
+    assert.equal(ctx.method, endError.attempt.method);
+    assert.equal(ctx.path, endError.attempt.path);
+    assert.equal(ctx.errorName, 'EndpointError');
+    assert.equal(ctx.reason, endError.reason);
   });
 });