Skip to content

Express is not instrumented #12105

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
3 tasks done
adriaanmeuris opened this issue May 17, 2024 · 28 comments
Closed
3 tasks done

Express is not instrumented #12105

adriaanmeuris opened this issue May 17, 2024 · 28 comments
Labels
Package: node Issues related to the Sentry Node SDK

Comments

@adriaanmeuris
Copy link

adriaanmeuris commented May 17, 2024

Is there an existing issue for this?

How do you use Sentry?

Sentry Saas (sentry.io)

Which SDK are you using?

@sentry/node

SDK Version

8.2.1

Framework Version

No response

Link to Sentry event

No response

SDK Setup

Sentry.init({
  dsn: __DSN_HERE__
});

Steps to Reproduce

import * as Sentry from '@sentry/node';
import express from 'express';

Sentry.init({
  dsn: __DSN_HERE__
});

// Setup Express app
const app = express();

// Add Sentry error handler
Sentry.setupExpressErrorHandler(app);

app.listen(3000);

Expected Result

No error about Express not being instrumented.

Actual Result

Error:

[Sentry] Express is not instrumented. This is likely because you required/imported express before calling `Sentry.init()`.

Initializing Sentry before importing Express doesn't work.

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 May 17, 2024
@github-actions github-actions bot added the Package: node Issues related to the Sentry Node SDK label May 17, 2024
@Lms24
Copy link
Member

Lms24 commented May 17, 2024

Hey @adriaanmeuris

the error message tells you to call Sentry.init before requireing express. Why does this not work for you?

@adriaanmeuris
Copy link
Author

I've made this change as follows:

import * as Sentry from '@sentry/node';

Sentry.init({
  dsn: __DSN_HERE__
});

import express from 'express';

But the error message still occurs, I'm not sure why. Am I missing something?

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 May 17, 2024
@mydea
Copy link
Member

mydea commented May 17, 2024

Hello,

are you running your application in CJS or ESM? Meaning, what is it you do like node src/my-app.js? Or do you have a build step?

If you are running it as ESM app, you need to follow the docs here: https://docs.sentry.io/platforms/javascript/guides/express/install/esm/

@amiranvarov
Copy link

hey, i'm having absolutely same issue, and it's annoying AF. And i'm using CJS, not ESM. Double checked it.

I spent 5 hours in total trying to modify my application initialization. Even tried using some workaround with setTimeout. None of that helped. Still having this annoyind AG red text saying [Sentry] Express is not instrumented. This is likely because you required/imported express before calling `Sentry.init().

I don't even have any other express related import other than Sentry.expressIntegration(), can you guys please bring back possiblity to initialise the Sentry init not at the top, it's really frustrating not being able to init how you want. I have a microservice app, and there i have one common function that initializes microservice. I used to Sentry inside that microservice initializing function, but now, after migrating to 8 i had to take sentry init out of that function and init manually inside every of microservice to make sure it's the fitst import, but even after that I'm having this error.

It's such a drawback in developer experience, it's hard to believe that you had to do that. I would assuke that you had technical reasons to do so, but it was working perfectly fine in v7. Why did you guys had to remove that flexibility?

Sorry that i'm being emotional, but spent way too much hours trying to fix error, by following exactly what you wrote to do, and still this error is there. You could write some better error message, other than "most likely"

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 May 18, 2024
@amiranvarov
Copy link

update: I noticed that this error is happening when i add Sentry.setupExpressErrorHandler(app); at the end of all midlewares and controllers, right before my app.listen() call

// initSentry.ts

import * as Sentry from "@sentry/node";
import { nodeProfilingIntegration } from "@sentry/profiling-node";

export function initSentry({
	name,
	dsn,
	env,
	debug = false,
	integrations,
	enabled = true,
	includeLocalVariables = false,
	callback,
}: {
	name: string;
	enabled?: boolean;
	dsn: string;
	env: { node_env: string };
	debug: boolean;
	integrations: any[];
	includeLocalVariables?: boolean;
	callback?: () => void;
}) {
	const defaultIntegrations = [
		Sentry.httpIntegration(),
		Sentry.expressIntegration(),
		nodeProfilingIntegration(),
		Sentry.localVariablesIntegration(),
	];

	Sentry.init({
		serverName: name,
		dsn: dsn,
		environment: env.node_env,
		enabled: enabled,
		debug: debug,
		integrations: [...(includeLocalVariables ? [Sentry.localVariablesIntegration()] : []), ...defaultIntegrations, ...integrations],
		// Set tracesSampleRate to 1.0 to capture 100%
		// of transactions for performance monitoring.
		// We recommend adjusting this value in production

		beforeSend(event, hint) {
			const error = hint.originalException as Error;
			if (error?.message?.match(/database unavailable/i)) {
				event.fingerprint = ["database-unavailable"];
			}

			// Modify or drop the event here
			if (event.user) {
				// Don't send user's email address
				event.user.email = undefined;
			}

			return event;
		},

		// Called for transaction events
		beforeSendTransaction(event) {
			// Modify or drop the event here
			if (event.transaction === "/unimportant/route") {
				// Don't send the event to Sentry
				return null;
			}
			return event;
		},
		beforeBreadcrumb(breadcrumb, hint) {
			// Check if the breadcrumb has sensitive data like user email
			if (breadcrumb.data?.["user"]?.email) {
				// Remove the user email from the breadcrumb
				breadcrumb.data["user"].email = undefined;
			}
			return breadcrumb;
		},
		includeLocalVariables: includeLocalVariables,
		attachStacktrace: true,
		tracesSampleRate: 1.0,
		profilesSampleRate: 1.0,
	});

	callback?.();
}

// main.ts

mport * as Sentry from "@sentry/node";

import { initSentry } from "@myorg/sentry-init";

initSentry({
	name: "auth-service",
	dsn: process.env.SENTRY_DSN,
	env: { node_env: process.env.NODE_ENV },
	debug: false,
	integrations: [],
});
import { MicroserviceName, TRPC_ENDPOINTS_MAP } from "@myorg/constants";

const MICROSERVICE_NAME = MicroserviceName.AuthService;
import { authRouter, prisma, authjsPlugin, envServer, logger } from "@myorg/auth-service";
import { natsWrapper } from "@myorg/auth-service/lib/nats-wrapper";
import { initRestRouter } from "@myorg/auth-service";

import { createMicroserviceApp } from "@myorg/utils";

createMicroserviceApp({
	name: MICROSERVICE_NAME,
	env: {
		port: envServer.TRPC_PORT,
		host: envServer.HOST,
		node_env: envServer.NODE_ENV,
	},
	trpcRouter: {
		path: TRPC_ENDPOINTS_MAP[MicroserviceName.AuthService],
		router: authRouter,
	},
	logger,
	nats: {
		natsWrapper: natsWrapper,
		listeners: [],
	},

	initRestRouter,
	plugins: [authjsPlugin],
});
createMicroservice.ts
// skipped some imports 

type MicroserviceConfig = {
	name: NeiraOwnMicroservicesNames;
	trpcRouter?: {
		path: string;
		router: any;
	};

	logger: LoggerInstance;
	env: {
		port: number;
		host: string;
		node_env: string;
	};
	nats?: {
		natsWrapper: AbstractNatsWrapper;
		listeners: (typeof NatsListener)[];
	};
	initRestRouter?: (app: express.Express) => void;
	plugins?: ((app: express.Express) => void)[] | ((app: express.Express) => Promise<void>)[];
	testConnectionWithMicroservices?: boolean;
};
export async function createMicroserviceApp<T>(
	{
		name,
		env,
		// sentry: { debug = false, dsn, integrations = [] },
		trpcRouter,
		logger,
		nats,
		initRestRouter,
		plugins = [],
		testConnectionWithMicroservices = false,
	}: MicroserviceConfig,
	callback?: () => void,
) {
	if (env.node_env === "production") {
		logger.info(`Starting [${name}] Microservice`);
	}

	const app = express();

	app.use(bodyParser.urlencoded({ extended: true }));
	app.use(cors());
	app.use(bodyParser.json());

	app.use(makeWinstonLogger(name));

	app.use(createTransactionIdExpressMiddleware);
	listenForConnectionCheck(app, name);

	if (initRestRouter) {
		initRestRouter(app);
	}

	if (plugins) {
		for (const plugin of plugins) {
			await plugin(app);
		}
	}
	// important! trpcRouter must be after plugins!
	if (trpcRouter) {
		app.use(
			trpcRouter.path,
			trpcExpress.createExpressMiddleware({
				router: trpcRouter.router,
				createContext: createExpressTrpcContext,
			}),
		);
	}

	app.use(expressCustomErrorHandler);
	Sentry.setupExpressErrorHandler(app);

	app.listen(env.port, env.host, () => {
		logger.info(`[${name}] tRPC server is listening on http://${env.host}:${env.port}`);

		if (callback) {
			callback();
		}
	});
	// rest of the app

@amiranvarov
Copy link

For context, i'm also using Sentry 8.2.1. Node v20.13.1.

@amiranvarov
Copy link

any updates? :) I can provide more info if needed

@rnenjoy
Copy link

rnenjoy commented May 20, 2024

Same here. Error happens when i add Sentry.setupExpressErrorHandler(app);.
Even though i have init at the very top of my index.js.

My setup is ESM, so i guess i have to use the --import step. However i cant, cause then i get the import-in-the-middle bug and the app crashes.

@mydea
Copy link
Member

mydea commented May 21, 2024

Just to note these here too (this was figured out in #12056), the warning will be incorrectly logged today if tracing is not enabled. We will fix this in an upcoming release. For now, you can ignore the warning if tracing is not enabled!

So some notes @amiranvarov:

  1. You need to call Sentry.setupExpressErrorHandler() before any other error handling middleware, but after all routes are registered. We are currently updating docs to be more explicit about this, sorry about the confusion there.
  2. The only integration you have to manually add in v8 is nodeProfilingIntegration - no need to add either httpIntegration, expressIntegrationorlocalVariablesIntegration`. All of these are automatically added for you.
  3. Your init seems correct to me, other than the nits I posted above (both of these should not really affect the basic instrumentation). Could you share the full startup logs you get when you run this with debug: true?

@rnenjoy:
For now, without --import you can use Sentry, and everything error-related should work. But for performance, only basic instrumentation (=incoming and outgoing http/fetch instrumentation) will work without --import. See https://docs.sentry.io/platforms/javascript/guides/node/install/esm-without-import/ for details.

@jorgeavaldez
Copy link

We're having the same error with our app. We're using ESM compiled from typescript. When trying to use the --import flag as shown, we get a separate issue:
image

the init function call is pretty barebones, and the express error handler is called before any other error handling middleware as well

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 May 21, 2024
@xmunoz
Copy link

xmunoz commented May 21, 2024

We're having the same error with our app. We're using ESM compiled from typescript. When trying to use the --import flag as shown, we get a separate issue

This is the issue I'm experiencing as well. It appears that if your app and Sentry's SDK share a dependency, that dependency will try to be imported twice, causing this error.

@mydea
Copy link
Member

mydea commented May 22, 2024

Could you share your instrument.js file - what do you have in there?

@xmunoz
Copy link

xmunoz commented May 22, 2024

import * as Sentry from '@sentry/node';

import config from 'src/config/index.js';

Sentry.init({
  dsn: config.sentryDSN,
  environment: config.sentryEnv,
  tracesSampleRate: 1.0,
});

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 May 22, 2024
@mydea
Copy link
Member

mydea commented May 22, 2024

Could you share the gist of src/config/index.js? Is this importing other modules, or just exporting some static config stuff?

@xmunoz
Copy link

xmunoz commented May 22, 2024

I have modified the code as follows and it still crashes the app in the same way.

import * as Sentry from '@sentry/node';                                                                                                             

const { NODE_ENV } = process.env;

Sentry.init({
  dsn: 'actualDSNstring',
  environment: NODE_ENV,
  tracesSampleRate: 1.0,
});

Run command: NODE_ENV=development nodemon --inspect=0.0.0.0:9229 --import ./src/instrument.js ./src/index.js | pino-colada

Crash:

SyntaxError: Identifier '$longFormatters' has already been declared
    at ModuleLoader.moduleStrategy (node:internal/modules/esm/translators:169:18)
    at callTranslator (node:internal/modules/esm/loader:272:14)
    at ModuleLoader.moduleProvider (node:internal/modules/esm/loader:278:30)
[nodemon] app crashed - waiting for file changes before starting...

Our prod run command is slightly different, but it also crashes like this on prod.

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 May 22, 2024
@xmunoz
Copy link

xmunoz commented May 22, 2024

I removed one of our dependencies in package.json, and it works now. The dependency in question is "date-fns": "^3.6.0". However, eliminating this dependency in order to use Sentry SDK v8 is not a compromise we can make at this time.

@jorgeavaldez
Copy link

our application also relies on date-fns. unfortunately we're in a monorepo so removing it may not be that simple, but that's good to know!

@mydea
Copy link
Member

mydea commented May 23, 2024

Ah, I see, we've seen this before, there seems to be a problem with date-fns: date-fns/date-fns#3770

This is related/a "duplicate" of #12154 then, which runs into the same problem. We'll try to come up with a fix!

@AbhiPrasad
Copy link
Member

We released https://github.com/getsentry/sentry-javascript/releases/tag/8.8.0 which should fix this problem. Please give it a try!

@ozozozd
Copy link

ozozozd commented Oct 10, 2024

This is still a problem on "@sentry/node": "^8.33.1"

@pjs355
Copy link

pjs355 commented Dec 5, 2024

I too have this issue in a CJS app... followed everything for the CJS setup... it's literally the first thing I import and I have logs saying "sentry init"
"express init"
[Sentry] express is not instrumented...

sentry 8.42
express 4.21

@esmael-Abdlkadr
Copy link

esmael-Abdlkadr commented Jan 3, 2025

I encountered the same issue, and here is how I resolved it:

1. Create a dedicated file called instrument.js:

import * as Sentry from "@sentry/node";
import { ProfilingIntegration } from "@sentry/profiling-node";

Sentry.init({
  dsn: process.env.SENTRY_DSN,
  integrations: [new ProfilingIntegration()],
  tracesSampleRate: 1.0,
  profilesSampleRate: 1.0,
});

export { Sentry };

2. Import this file at the very top of your app.js:

import { Sentry } from "./instrument.js";                                   
const app = express();
Sentry.setupExpressErrorHandler(app);

After implementing these changes, the error was resolved.

@Eduardo-Miguel
Copy link

Did you guys fix the error? I have the same issue and nothing commented here works 😿

@GwFreak01
Copy link

GwFreak01 commented Jan 26, 2025

Still having this issue too...

Sentry: 8.51.0

Running typescript and Express and tsx

Found this comment and it removes my error too when I comment out tracesSampleRate
hiroshinishio#17 (comment)

@mschmid
Copy link

mschmid commented Jan 29, 2025

Same here, moving Sentry.init before every other import does not help,
removingtracesSampleRate fixes the issue.

"@sentry/node": "8.52.0"
"express": "4.21.2"

@mydea
Copy link
Member

mydea commented Jan 30, 2025

If you use express 4x, and have top-level import, and you are still getting this warning: The most probable cause is that your app is running in ESM mode. When you run with debug: true you should see a log about this (saying running in CommonJS or ESM mode). If it is ESM, you have to follow this guide to get rid of this warning: https://docs.sentry.io/platforms/javascript/guides/express/install/esm/

Commenting out the tracesSampleRate removes the warning because it then does not add the express tracing instrumentation. If you do not care about tracing this is fine, but if you want proper tracing and are using ESM, sadly --import is the only way to get this working as of today.

@jarimustonen
Copy link

If you use express 4x, and have top-level import, and you are still getting this warning: The most probable cause is that your app is running in ESM mode. When you run with debug: true you should see a log about this (saying running in CommonJS or ESM mode). If it is ESM, you have to follow this guide to get rid of this warning: https://docs.sentry.io/platforms/javascript/guides/express/install/esm/

First of all, this is not just "warning". Sentry just does not work with Express even as the instrumentation is put in the first think in the file.

Second, the instructions that you pointed us is pretty flimsy and does not really address the issues. I've been spending now 6 hours of trying to get node/tsx to work with this with no success. Basically all I can do is try different shots in the dark and see if the cryptic error messages shows up or not. Well, it does, all the time.

@mydea
Copy link
Member

mydea commented Mar 31, 2025

What exactly is not working/clear from the docs?

Are you shipping .mjs module-based code? Follow these docs: https://docs.sentry.io/platforms/javascript/guides/express/install/esm/

Are you shipping .js/.cjs require-based code? Follow these docs: https://docs.sentry.io/platforms/javascript/guides/express/install/commonjs/

If anything there is not working/unclear, please open an issue in https://github.com/getsentry/sentry-docs as we def. want to make this as clear as possible!

Second, the instructions that you pointed us is pretty flimsy and does not really address the issues. I

What exactly is not addressed by the docs?

Basically all I can do is try different shots in the dark and see if the cryptic error messages shows up or not.

Can you share the full logs you are actually getting?

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

No branches or pull requests