Sentry.Hapi.Integration mixes event state between requests

[dschom]

Is there an existing issue for this?

• [X] I have checked for existing issues https://github.com/getsentry/sentry-javascript/issues
• [X] I have reviewed the documentation https://docs.sentry.io/
• [X] I am using the latest SDK release https://github.com/getsentry/sentry-javascript/releases

How do you use Sentry?

Sentry Saas (sentry.io)

Which SDK are you using?

@sentry/node

SDK Version

7.109.0

Framework Version

Hapi v21.3.7

Link to Sentry event

https://mozilla.sentry.io/issues/5137492864/?project=6231058&query=is%3Aunresolved&referrer=issue-stream&statsPeriod=1h&stream_index=2

SDK Setup

Sentry.init({ env: 'local', dsn: process.env.SENTRY_DSN, tracesSampleRate: 1.0, integrations: [ new Sentry.Integrations.Hapi({server}), ], beforeSend(event) { // Logging to disk so as not generate any extra breadcrumbs fs.appendFileSync(debug.${TEST_NAME}.json, JSON.stringify({event}, null, ' ') + '\n'); return event; }, });

Steps to Reproduce

I've created a repo to illustrate the issue. Simply follow the readme instructions to reproduce.

In a nutshell, two parallel requests made to different endpoints will result in the state of breadcrumbs and tags.transaction being mixed together. There are three test scenarios each of which exhibits a slightly different set of behaviors.

• Synchronous Request handlers
• Asynchronous request handlers
• Asynchronous request handlers where the handler is wrapped with Sentry.runWithAsyncContext

Expected Result

That sentry events have state that is isolated per request.

Actual Result

The state of the Sentry events being captured contains state from other requests. The aforementioned repo shows three different ways this can happen, each of which has a slightly different behavior. Here are basic descriptions of these scenarios and links to actual Sentry events produced.

Synchronous Request Handlers: One event is correct, but one event contains the breadcrumbs that weren't produced by the request.

• Breadcrumbs from another request included
• Event is okay!

Asynchronous Request Handlers: The breadcrumbs for the events are mixed, and tags.transaction shows the incorrect value for one event.

• Breadcrumbs from another request included and wrong transaction
• breadcrumbs from another request included

Asynchronous Requests Handlers Wrapped with Sentry.runWithAsyncContext: The breadcrumbs are lost, and tag.transaction is incorrect.

• Missing breadcrumbs and wrong transaction
• Just missing breadcrumbs

[AbhiPrasad]

Hey @dschom thanks for writing in!

I think this is because we aren't wrapping stuff in async context properly with Hapi. https://github.com/getsentry/sentry-javascript/blob/9bfadb5852312d5fac7103867436de4a5da377df/packages/node/src/integrations/hapi/index.ts#L66

For now to unblock yourself, you can wrap request handling middleware with Sentry.runWithAsyncContext as per https://docs.sentry.io/platforms/node/configuration/async-context/. This will ensure requests get isolated.

We might not fix this in v7 because we have a new v8 major releasing very soon, and that dramatically improves the Hapi instrumentation experience. I would give this a try:

v8 alpha: https://github.com/getsentry/sentry-javascript/releases/tag/8.0.0-alpha.7 migration guide: https://github.com/getsentry/sentry-javascript/blob/8.0.0-alpha.7/MIGRATION.md

In v8 setting up hapi works like the following:

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

Sentry.init({
  dsn: '__DSN__',
  tracesSampleRate: 1.0,
});

// Sentry must be initialized before Hapi is imported.
const Hapi = require('@hapi/hapi');

const init = async () => {
  // Hapi server is automatically instrumented
  const server = Hapi.server({
    host: 'localhost',
    port: 3000,
  });

  server.route({
    method: 'GET',
    path: '/',
    handler: (_request, _h) => {
      return 'Hello World!';
    },
  });

  // Add Sentry error handler to catch errors
  await Sentry.setupHapiErrorHandler(server);
  await server.start();
};

init();

[Marsup]

I have tried about every possible way to hook runWithAsyncContext to the request, has anyone succeeded using it?

hapi 21 not being instrumented in the beta (https://github.com/open-telemetry/opentelemetry-js-contrib/pull/1985), it's also not an option we can currently use unfortunately.

[mydea]

I have tried about every possible way to hook runWithAsyncContext to the request, has anyone succeeded using it?

hapi 21 not being instrumented in the beta (open-telemetry/opentelemetry-js-contrib#1985), it's also not an option we can currently use unfortunately.

I think at this point we can hope that a new @opentelemetry/instrumentation-hapi is released soon with the fix for the new hapi version 😬 I made sure to watch that PR as well so we can bump it as soon as they release a fix 🤞

[mydea]

The next release of the SDK will support v21 of hapi properly!

[Marsup]

Cool, I have tried enforcing the new release with a pnpm override but it didn't have any breadcrumbs, I hope the full release will 🤞🏻