@simplewebauthn/server is not supported when using ESM

P4sca1 commented 1 month ago

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

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)

mydea commented 1 month ago

Hey,

can you try to exclude this package from instrumentation - I just noticed this was not properly documented yet, so I added docs here https://github.com/getsentry/sentry-docs/pull/10803

So something like this:

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

timfish commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

Thank you for confirming!

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

timfish commented 5 days ago

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!

getsentry / sentry-javascript