Express middleware not sending request details

[ajsharp]

Expected behavior

Bugsnag node in an express app should send request details. There should be a "req" tab in the web UI.

Observed behavior

No request details are being sent. The only tabs I see are stacktrace, app and device.

Steps to reproduce

Trigger an exception in a request.

Version

busgnag node 2.3.1

Additional information

This is a typescript app, using async / await in the request handler. I've set up the bugsnag middlewares as recommended in the docs:

import express from 'express';
import bugsnag from 'bugsnag';

const app = express();

bugsnag.register('API_KEY', {autoNotify: true});
app.use(bugsnag.requestHandler);
app.use(bugsnag.errorHandler);

[bengourley]

Hey @ajsharp. Can you provide an example of a route that is erroring in the small code snippet you provided please?

e.g. I tried this example and it worked as expected (creates a request tab in the dashboard)…

import express from 'express';
import bugsnag from 'bugsnag';

const app = express();

bugsnag.register('API_KEY', {autoNotify: true});
app.use(bugsnag.requestHandler);

// routes must go _after_ request handler

app.get('/', (req, res) => {
  throw new Error('bad route')
});

// and _before_ error handler

app.use(bugsnag.errorHandler);

app.listen(3000);

[ajsharp]

Ok, so if I define a normal endpoint, it works as expected, and I get a request tab in the web UI. However, if it's an async endpoint, I get no request tab:

Source:

app.get('/test', async (req, res) => {
  throw new Error('test error');
});

Generated Javascript:

"use strict";
var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, generator) {
    return new (P || (P = Promise))(function (resolve, reject) {
        function fulfilled(value) { try { step(generator.next(value)); } catch (e) { reject(e); } }
        function rejected(value) { try { step(generator["throw"](value)); } catch (e) { reject(e); } }
        function step(result) { result.done ? resolve(result.value) : new P(function (resolve) { resolve(result.value); }).then(fulfilled, rejected); }
        step((generator = generator.apply(thisArg, _arguments || [])).next());
    });
};

var __importDefault = (this && this.__importDefault) || function (mod) {
    return (mod && mod.__esModule) ? mod : { "default": mod };
};

const express_1 = __importDefault(require("express"));
const bugsnag_1 = __importDefault(require("bugsnag"));

const app = express_1.default();
bugsnag_1.default.register('API_KEY', { autoNotify: true });

app.use(bugsnag_1.default.requestHandler);

app.get('/test', (req, res) => __awaiter(this, void 0, void 0, function* () {
    throw new Error('test error');
}));

app.use(bugsnag_1.errorHandler);

app.listen(3000, () => {
    console.log('running server on', PORT);
});

[bengourley]

Hey @ajsharp. This happens because express routes are not "async/promise-aware". I'll try to explain…

When you create an async function () {}, what it actually does it create a Promise (as you can see from your generated code).

When express runs this route function which returns a Promise, it doesn't do anything with either then eventual return value (from .then()) or the error value (from .catch()).

This means the error gets thrown outside of the context of the route, which is why you don't get any request metadata.

This post by Dom @ Readme goes into more detail about this if you want to understand more. But if you just want the solution, here is the example from the post applied to our small test case:

import express from 'express';
import bugsnag from 'bugsnag';

const app = express();

bugsnag.register('API_KEY', {autoNotify: true});
app.use(bugsnag.requestHandler);

// routes must go _after_ request handler

app.get('/', asyncWrap(async (req, res) => {
  throw new Error('test error');
}));

function asyncWrap(fn) {
  return (req, res, next) => {
    fn(req, res, next).catch(next);
  };
};

// and _before_ error handler

app.use(bugsnag.errorHandler);

app.listen(3000);

Basically what this does is catch any error in the route function and passes it on to the express error handler with .catch(next).

I hope this helps! Cheers

[ajsharp]

@bengourley Awesome, thanks for wading through this, I'm very appreciative of this fix.

I'd love to see this become part of the busgnag express middleware. In my mind, this should be built in to bugsnag, because async is a language feature, and as a user of the bugsnag library, I expect that would just handle async gracefully.

[bengourley]

You're very welcome.

I would love to get this into bugsnag too but there isn't a lot we can do… if either of the following changes, things can work "out-of-the-box":

• Express routes/middleware support async functions/Promises. There is actually an open PR on the routing module that express uses internally, so this may land in the near future.
• Domains are updated to work with Promises. We use domains (although deprecated, they didn't replace it with anything yet!) to track the execution context across asynchronous calls, but this only works with callbacks, not Promises. This is highly unlikely to be fixed since the module is deprecated :(

In the future we will probably end up using the new async hooks module to track async execution contexts, but that is a little way off just now!

[ajsharp]

Got it, that makes sense.

Also, if the errorHandler middleware needs to be added after the routes, the documentation on the bugsnag website needs to be updated, as it doesn't reflect that: https://docs.bugsnag.com/platforms/nodejs/express/.