Skip to content

Integration for FastifyJS #4784

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
AbhiPrasad opened this issue Mar 25, 2022 · 26 comments
Closed

Integration for FastifyJS #4784

AbhiPrasad opened this issue Mar 25, 2022 · 26 comments
Labels
Meta: Help Wanted Package: node Issues related to the Sentry Node SDK Type: Improvement
Milestone

Comments

@AbhiPrasad
Copy link
Member

Problem Statement

https://www.fastify.io/

https://www.npmjs.com/package/fastify

Solution Brainstorm

Build an integration for Fastify, similar to what we do for Express

@iamchathu
Copy link

Official support like express would be great!

@xr0master
Copy link
Contributor

I wrote and use the following code for FastifyJS 3, it works but has problems due to global scoping. Might be useful to someone, better than nothing. I would be happy to have official support.

  function parseRequest(event: Event, req: FastifyRequest) {
    event.contexts = {
      ...event.contexts,
      runtime: {
        name: 'node',
        version: process.version,
      },
    };
  
    event.request = {
      ...event.request,
      url: `${req.protocol}://${req.hostname}${req.url}`,
      method: req.method,
      headers: req.headers as Record<string, string>,
      query_string: req.query as Record<string, string>,
    };
  
    return event;
  }

  fastify.addHook('onRequest', (req, reply, done) => {
    const currentHub = getCurrentHub();
    currentHub.pushScope();

    currentHub.configureScope((scope) => {
      scope.addEventProcessor((event) => parseRequest(event, req));
    });

    done();
  });

  fastify.addHook('onResponse', (req, reply, done) => {
    const currentHub = getCurrentHub();
    currentHub.popScope();

    done();
  });
  fastify.setErrorHandler(async (error: FastifyError, req, reply) => {
    if (!error.statusCode || error.statusCode >= 500) {
      withScope((scope) => {
        captureException(error);
      });

      return res.send('Something is wrong, please contact support');
    }

    return res.send(error);
  });

@Xhale1
Copy link

Xhale1 commented Jun 24, 2022

I would love this. Fastify is a growing framework with over 500k downloads / month.

@xr0master
Copy link
Contributor

@AbhiPrasad Is there any progress in this direction? Can we at least discuss theoretically how to do this?
I still don't understand how to achieve the scoping for single process nodejs code using the Sentry architecture. Since the hub will always be the same for all requests in a particular application. Some easy way to bind the scope to the request context and explain to the Sentry which scope to use in the current code block.

@AbhiPrasad
Copy link
Member Author

You can bind the hub to the current request using domains or async hooks - storing it in async storage. This is what we do in nextjs for example - #3788

It’s not ideal, but using async storage mechanisms is what every observability SDK for node uses. We will revaluate this at a later point.

@xr0master
Copy link
Contributor

@AbhiPrasad Thanks for the link! As far as I can see, there is still a domain in use that is deprecated and "especially" not recommended. However, yes, without a serious rethinking of the Sentry architecture at the moment it is difficult to find another way.
Not quite understood with async hooks. Do you mean memoize the scope and bind it to the request? But how then to remove the used scope from Sentry's hub?

@xr0master
Copy link
Contributor

xr0master commented Aug 20, 2022

My first version of a plugin for catching errors in Fastify. I'm interested to see what anyone thinks.
@AbhiPrasad This plugin does not add transactions, but I think there is no problem to add this by analogy with nextjs.

Known Issues

  • Outputs to the console through Fastify hooks will be duplicated in breadcrumbs from error to error.
  • Works only if the route is registered. (use await for plugin registration to resolve it)

Code

Register the plugin:

await app.register(sentry, {
  dsn: process.env.SENTRY_DNS,
});

@sentry/fastify file:

import { create } from 'domain';
import fastifyPlugin from 'fastify-plugin';
import { init, getCurrentHub, captureException, type NodeOptions, type Event } from '@sentry/node';
import type { FastifyRequest, FastifyPluginCallback, FastifyError } from 'fastify';

function parseRequest(event: Event, req: FastifyRequest) {
  event.contexts = {
    ...event.contexts,
    runtime: {
      name: 'node',
      version: process.version,
    },
  };

  event.request = {
    ...event.request,
    url: `${req.protocol}://${req.hostname}${req.url}`,
    method: req.method,
    headers: req.headers as Record<string, string>,
    query_string: req.query as Record<string, string>,
  };

  return event;
}

const sentry: FastifyPluginCallback<NodeOptions> = (fastify, options, next) => {
  init(options);

  fastify.addHook('onRoute', (options) => {
    const originalHandler = options.handler;

    options.handler = async (req, res) => {
      const domain = create();

      domain.add(req as never);
      domain.add(res as never);

      const boundHandler = domain.bind(async () => {
        getCurrentHub().configureScope((scope) => {
          scope.addEventProcessor((event) => parseRequest(event, req));
        });

        try {
          return await originalHandler.call(fastify, req, res);
        } catch (error) {
          // If there is no statusCode, it means an internal error that was not fired by Fastify.
          // Maybe it's good to catch 5xx errors as well. Can be removed or added as plugin options.
          if (!(error as FastifyError).statusCode || (error as FastifyError).statusCode! >= 500) {
            captureException(error);
          }
          throw error;
        }
      });

      await boundHandler();
    };
  });

  next();
};

export default fastifyPlugin(sentry, {
  fastify: '4.x',
  name: '@sentry/fastify',
});

@biaomingzhong
Copy link

I am using @fastify/middle .

  • Http request handler is working.
// eslint-disable-next-line @typescript-eslint/no-var-requires
  const Sentry = require('@sentry/node');
  // eslint-disable-next-line @typescript-eslint/no-var-requires
  const { ProfilingIntegration } = require('@sentry/profiling-node');
  // eslint-disable-next-line @typescript-eslint/no-var-requires
  // const Tracing = require('@sentry/tracing');
  Sentry.init({
    dsn: dataSourceName,
    integrations: [
      // enable HTTP calls tracing
      new Sentry.Integrations.Http({ tracing: true }),
      // // enable Express.js middleware tracing
      // new Tracing.Integrations.Express({ app: app.getHttpAdapter().getInstance() }),
      new ProfilingIntegration(),
    ],

    // Set tracesSampleRate to 1.0 to capture 100%
    // of transactions for performance monitoring.
    // We recommend adjusting this value in production
    tracesSampleRate: 1.0,
    profilesSampleRate: 1.0,
    // or pull from params
    // tracesSampleRate: parseFloat(params.SENTRY_TRACES_SAMPLE_RATE),
  });

  // RequestHandler creates a separate execution context using domains, so that every
  // transaction/span/breadcrumb is attached to its own Hub instance
  app.use(Sentry.Handlers.requestHandler());
  // TracingHandler creates a trace for every incoming request
  // app.use(Sentry.Handlers.tracingHandler());
  • But the tracing module is not working with integrations Tracing.Integrations.Express
// eslint-disable-next-line @typescript-eslint/no-var-requires
  const Sentry = require('@sentry/node');
  // eslint-disable-next-line @typescript-eslint/no-var-requires
  const { ProfilingIntegration } = require('@sentry/profiling-node');
  // eslint-disable-next-line @typescript-eslint/no-var-requires
  const Tracing = require('@sentry/tracing');
  Sentry.init({
    dsn: dataSourceName,
    integrations: [
      // enable HTTP calls tracing
      new Sentry.Integrations.Http({ tracing: true }),
      // enable Express.js middleware tracing
      new Tracing.Integrations.Express({ app: app.getHttpAdapter().getInstance() }),
      new ProfilingIntegration(),
    ],

    // Set tracesSampleRate to 1.0 to capture 100%
    // of transactions for performance monitoring.
    // We recommend adjusting this value in production
    tracesSampleRate: 1.0,
    profilesSampleRate: 1.0,
    // or pull from params
    // tracesSampleRate: parseFloat(params.SENTRY_TRACES_SAMPLE_RATE),
  });

  // RequestHandler creates a separate execution context using domains, so that every
  // transaction/span/breadcrumb is attached to its own Hub instance
  app.use(Sentry.Handlers.requestHandler());
  // TracingHandler creates a trace for every incoming request
  app.use(Sentry.Handlers.tracingHandler());

@thinkocapo
Copy link

Hi I'm assisting an organization that could benefit from this.

@xr0master
Copy link
Contributor

xr0master commented Aug 28, 2023

@thinkocapo
The new version of the code for the Sentry Async API

import fastifyPlugin from 'fastify-plugin';
import {
  init,
  getCurrentHub,
  captureException,
  runWithAsyncContext,
  type NodeOptions,
  type Event,
} from '@sentry/node';

import type { FastifyRequest, FastifyPluginCallback, FastifyError } from 'fastify';

function parseRequest(event: Event, req: FastifyRequest) {
  event.contexts = {
    ...event.contexts,
    runtime: {
      name: 'node',
      version: process.version,
    },
  };

  event.request = {
    ...event.request,
    url: `${req.protocol}://${req.hostname}${req.url}`,
    method: req.method,
    headers: req.headers as Record<string, string>,
    query_string: req.query as Record<string, string>,
  };

  return event;
}

const sentry: FastifyPluginCallback<NodeOptions> = (fastify, options, next) => {
  init(options);

  fastify.addHook('onRoute', (options) => {
    const originalHandler = options.handler;

    options.handler = async (req, res) => {
      return runWithAsyncContext(async () => {
        getCurrentHub().configureScope((scope) => {
          scope.addEventProcessor((event) => parseRequest(event, req));
        });

        try {
          return await originalHandler.call(fastify, req, res);
        } catch (error) {
          // If there is no statusCode, it means an internal error that was not fired by Fastify.
          // Maybe it's good to catch 5xx errors as well. Can be removed or added as plugin options.
          if (!(error as FastifyError).statusCode || (error as FastifyError).statusCode! >= 500) {
            captureException(error);
            console.log('onError', error);
          }
          throw error;
        }
      });
    };
  });

  next();
};

export default fastifyPlugin(sentry, {
  fastify: '4.x',
  name: '@sentry/fastify',
});

P.S. I have been using it for production for a long time without any issues.

@csvan
Copy link

csvan commented Dec 22, 2023

Very open to help work on this, is there anything that needs assistance with? I work for a major company which uses Fastify extensively and recently started evaluating Sentry. Not having native official support is a major roadblock for general adoption currently.

@AbhiPrasad AbhiPrasad added this to the 8.0.0 milestone Apr 10, 2024
@AbhiPrasad
Copy link
Member Author

Coming soon in v8 of the sdk (alpha out now: https://www.npmjs.com/package/@sentry/node/v/8.0.0-alpha.9)

Example app: https://github.com/getsentry/sentry-javascript/tree/develop/dev-packages/e2e-tests/test-applications/node-fastify-app

const Sentry = require('@sentry/node');

Sentry.init({
  dsn: process.env.E2E_TEST_DSN,
  // distributed tracing + performance monitoring automatically enabled for fastify
  tracesSampleRate: 1,
});

// Make sure Sentry is initialized before you require fastify
const { fastify } = require('fastify');

const app = fastify();

// add error handler to whatever fastify router instances you want (we recommend all of them!)
Sentry.setupFastifyErrorHandler(app);

@gajus
Copy link
Contributor

gajus commented Apr 10, 2024

Screenshot 2024-04-10 at 1 56 58 PM

@AbhiPrasad Just heads up that we are getting a type error with Sentry alpha-9 and Fastify 4.24.3.

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 2 Apr 10, 2024
@AbhiPrasad
Copy link
Member Author

shoot we're on it! Thanks @gajus

AbhiPrasad added a commit that referenced this issue Apr 12, 2024
As per
#4784 (comment)

make sure fastify types use any is that we don't collide with what
fastify types expect.

Also convert e2e tests to use typescript to validate build.
@xr0master
Copy link
Contributor

xr0master commented Apr 13, 2024

Can I write my criticism?

  1. Fastify has an excellent plugin system. However, for some reason, the non-standard wrapper is used here. It would be much clearer for Fastify users to read the code below, IMHO.
await app.register(Sentry.setupFastify, {
  dsn: process.env.SENTRY_DNS,
  environment: process.env.NODE_ENV,
  // ...etc
});
  1. The code into "suffered" node packages instead of creating an independent package for Fastify with node dependency. I can only find one reason, it's lazy to create a new package and manage it :)

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 2 Apr 13, 2024
@mydea
Copy link
Member

mydea commented Apr 15, 2024

Thank you for the feedback!

For 1., we would not want to do that because Sentry should be initialised before fastify is initialised, as otherwise we may miss errors in Fastify initialisation itself.

For 2., especially in v8 this will not really be that much of an issue anymore, because fastify will be auto-instrumented without any config. So no need to do anything fastify specific, except to add an error handler. Having a dedicated package just for that feels a bit overkill IMHO :)

@xr0master
Copy link
Contributor

xr0master commented Apr 15, 2024

@mydea Thank you very much for the explanation.

  1. In this case, isn't the error critical? What I mean is that the system will immediately crash during the startup phase. I can't imagine anyone not noticing this without Sentry since the issue can even be on the const Sentry = require('@sentry/node'); line... Although you're right, Sentry should catch errors as early as possible.
  2. You may be right, but the code is still a little bit specific to each platform. If you need to fix, let's say: "expressjs", then you will release a new version for all platforms. What happens if you need to release break changes (for example, expressjs version 5)? You will need to release a major version for all platforms, right?

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 2 Apr 15, 2024
@AbhiPrasad
Copy link
Member Author

In this case, isn't the error critical

Some applications have complicated conditional logic (or run differently in production) during initialization. This is also important for getting accurate traces/spans for performance monitoring.

You will need to release a major version for all platforms, right?

We avoid this via a very small public API surface. We have a general SDK philosophy to stay compatible with as many things as possible, and we'll make sure our integrations can work with multiple majors as much as possible (see what we do with nextjs as an example).

cadesalaberry pushed a commit to cadesalaberry/sentry-javascript that referenced this issue Apr 19, 2024
As per
getsentry#4784 (comment)

make sure fastify types use any is that we don't collide with what
fastify types expect.

Also convert e2e tests to use typescript to validate build.
@mydea
Copy link
Member

mydea commented May 15, 2024

v8 is out and has proper fastify support! 🎉

@mydea mydea closed this as completed May 15, 2024
@lilouartz
Copy link

lilouartz commented Aug 17, 2024

v8 is out and has proper fastify support! 🎉

I am using v8 with Fastify and the problem is that breadcrumbs include a lot of information that is not related to user's request.

e.g.

Image

https://pillser.sentry.io/issues/5576662697/?project=4507122653003776&query=is%3Aunresolved+issue.priority%3A%5Bhigh%2C+medium%5D&referrer=issue-stream&statsPeriod=30d&stream_index=2

Those ping checks cannot be part of the error/user session because they are happening in background.

Fastify is instantiated with:

const app = fastify();

setupFastifyErrorHandler(app);

Sentry is loaded before fastify. Not sure what else to look at.

@punkpeye
Copy link

@mydea same issue ^

Lots of errors are just associated with wrong users/wrong requests/wrong breadcrumbs.

@AbhiPrasad
Copy link
Member Author

@punkpeye mind opening a new GH issue? And adding the output of debug logs when you add debug: true to your Sentry.init call?

We can investigate further then, we should be isolating the errors to each individual request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Meta: Help Wanted Package: node Issues related to the Sentry Node SDK Type: Improvement
Projects
Archived in project
Archived in project
Development

No branches or pull requests