Skip to content
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

@simplewebauthn/server is not supported when using ESM #12996

Open
3 tasks done
P4sca1 opened this issue Jul 21, 2024 · 5 comments
Open
3 tasks done

@simplewebauthn/server is not supported when using ESM #12996

P4sca1 opened this issue Jul 21, 2024 · 5 comments
Assignees
Labels
Package: node Issues related to the Sentry Node SDK

Comments

@P4sca1
Copy link

P4sca1 commented Jul 21, 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.19.0

Framework Version

No response

Link to Sentry event

No response

SDK Setup/Reproduction Example

// Preloaded using node --import flag. Do not import code from other source files here.
// https://docs.sentry.io/platforms/javascript/guides/node/install/esm/
// @ts-check

import { dirname } from 'node:path'
import { fileURLToPath } from 'node:url'
import * as Sentry from '@sentry/node'
import { nodeProfilingIntegration } from '@sentry/profiling-node'

/**
 * The root directory of the application.
 * @see {@link https://docs.sentry.io/platforms/javascript/guides/node/configuration/integrations/rewriteframes/}
 */
const root = dirname(fileURLToPath(import.meta.url))

const env =
	process.env['SENTRY_ENV'] || process.env['NODE_ENV'] || 'development'
const version = process.env['PACKAGE_VERSION']

Sentry.init({
	dsn: 'redacted',
	release: version ? `api@${version}` : undefined,
	environment: env,
	integrations: [
		nodeProfilingIntegration(),
		Sentry.dedupeIntegration(),
		Sentry.rewriteFramesIntegration({ root }),
		Sentry.prismaIntegration(),
	],
	beforeBreadcrumb: (breadcrumb) => {
		// Filter out writes to apollographql reporting API, because they are not helpful for error tracing
		// and spam breadcrumbs.
		if (
			breadcrumb.type === 'http' &&
			typeof breadcrumb.data?.['url'] === 'string' &&
			breadcrumb.data['url'].includes('api.apollographql.com')
		) {
			return null
		}

		return breadcrumb
	},
	tracesSampleRate: 0.1,
	profilesSampleRate: 0.1,
})

Steps to Reproduce

Install @simplewebauthn/server version 10.0.0.

import {
	generateAuthenticationOptions,
	generateRegistrationOptions,
	verifyAuthenticationResponse,
	verifyRegistrationResponse,
} from '@simplewebauthn/server'

Expected Result

Using @simplewebauthn/server using Sentry intstrumentation in an ESM environment should work.
Everything works fine when removing the --import ./instrument.js flag.

Actual Result

[nodemon] starting `node --env-file=.env --import ./instrument.js --import @swc-node/register/esm-register ./src/main.ts`
ReferenceError: generateChallenge is not defined
    at Function.assign (<anonymous>)
    at file:///Users/pascal/code/ips-hosting/api/node_modules/.pnpm/@simplewebauthn+server@10.0.0/node_modules/@simplewebauthn/server/esm/helpers/index.js?iitm=true:6:18
    at ModuleJob.run (node:internal/modules/esm/module_job:262:25)
    at async onImport.tracePromise.__proto__ (node:internal/modules/esm/loader:485:26)
    at async asyncRunEntryPointWithESMLoader (node:internal/modules/run_main:109:5)
@github-actions github-actions bot added the Package: node Issues related to the Sentry Node SDK label Jul 21, 2024
@mydea
Copy link
Member

mydea commented Jul 23, 2024

Hey,

can you try to exclude this package from instrumentation - I just noticed this was not properly documented yet, so I added docs here getsentry/sentry-docs#10803

So something like this:

Sentry.init({
  registerEsmLoaderHooks: {
    // Provide a list of package names to exclude from instrumentation
    exclude: ["@simplewebauthn/server"],
  },
});

@timfish
Copy link
Collaborator

timfish commented Jul 23, 2024

You might need to use a regular expression:

Sentry.init({
  registerEsmLoaderHooks: {
    // Provide a list of package names to exclude from instrumentation
    exclude: [/@simplewebauthn\/server/],
  },
});

For regular expressions to work you'll need to ensure that v1.10.0 of import-in-the-middle is in your dependencies. I only released this a few moments ago so you will need to remove your lock file and re-install or run npm/yarn update import-in-the-middle to update to it.

The Sentry type definitions don't yet allow regular expressions so I've opened a PR for that.

@P4sca1
Copy link
Author

P4sca1 commented Jul 23, 2024

Thank you for your help. After upgrading import-in-the-middle to v1.10.0 and adding the following to my Sentry.init options, I can confirm that using @simplewebauthn/server is now working.

registerEsmLoaderHooks: {
	// @ts-expect-error -- https://github.com/getsentry/sentry-javascript/pull/13016
	exclude: [/@simplewebauthn\/server/],
},

It was indeed required to use regular expressions. Using "@simplewebauthn/server" did not work.

@timfish
Copy link
Collaborator

timfish commented Jul 23, 2024

Thank you for confirming!

We should hopefully have an update coming soon which will negate the need for this manual configuration.

@timfish
Copy link
Collaborator

timfish commented Sep 9, 2024

v8.29.0 of the Node SDK adds the new onlyIncludeInstrumentedModules option to registerEsmLoaderHooks.

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

Sentry.init({
  dsn: '__PUBLIC_DSN__',
  registerEsmLoaderHooks: { onlyIncludeInstrumentedModules: true },
});

When set to true, import-in-the-middle will only wrap ESM modules that are specifically instrumented by OpenTelemetry plugins. This is useful to avoid issues where import-in-the-middle is not compatible with some of your dependencies.

This feature will only work if you Sentry.init() the SDK before the instrumented modules are loaded. This can be achieved via the Node --register and --import CLI flags or by loading your app code via async import() after calling Sentry.init().

Please let me know if this helps solve the import-in-the-middle incompatibility issues you are experiencing!

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
Status: No status
Development

No branches or pull requests

4 participants