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

Sentry.nativeNodeFetchIntegration fails to import opentelemetry-instrumentation-fetch-node in jest #12225

Closed
3 tasks done
TheHolyWaffle opened this issue May 27, 2024 · 6 comments
Labels
Package: node Issues related to the Sentry Node SDK Type: Bug

Comments

@TheHolyWaffle
Copy link

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.4.0

Framework Version

No response

Link to Sentry event

No response

SDK Setup

            Sentry.init({
                dsn: 'https://00000000000000000000000000000000@0000000.ingest.us.sentry.io/0000000',
                integrations: [Sentry.nativeNodeFetchIntegration()],
            });

Steps to Reproduce

  1. Run the SDK setup in debug mode in a commonjs environment such as jest
  2. add breakpoint in node_modules/@sentry/node/cjs/integrations/node-fetch.js on the line const pkg = await import('opentelemetry-instrumentation-fetch-node');

Expected Result

It correctly imports opentelemetry-instrumentation-fetch-node without throwing an error in cjs mode

Actual Result

It throws an error

image

TypeError: A dynamic import callback was invoked without --experimental-vm-modules
    at importModuleDynamicallyCallback (node:internal/modules/esm/utils:226:11)
    at getInstrumentation (/Users/bert.degeyter/dev/showpad-typescript/packages/sentry-utils/node_modules/@sentry/src/integrations/node-fetch.ts:51:19)
    at Object.setupOnce (/Users/bert.degeyter/dev/showpad-typescript/packages/sentry-utils/node_modules/@sentry/src/integrations/node-fetch.ts:85:7)
    at setupIntegration (/Users/bert.degeyter/dev/showpad-typescript/packages/sentry-utils/node_modules/@sentry/src/integration.ts:122:105)
    at /Users/bert.degeyter/dev/showpad-typescript/packages/sentry-utils/node_modules/@sentry/src/integration.ts:93:7
    at Array.forEach (<anonymous>)
    at Object.setupIntegrations (/Users/bert.degeyter/dev/showpad-typescript/packages/sentry-utils/node_modules/@sentry/src/integration.ts:90:16)
    at NodeClient.setupIntegrations [as _setupIntegrations] (/Users/bert.degeyter/dev/showpad-typescript/packages/sentry-utils/node_modules/@sentry/src/baseclient.ts:554:20)
    at NodeClient.init (/Users/bert.degeyter/dev/showpad-typescript/packages/sentry-utils/node_modules/@sentry/src/baseclient.ts:313:12)
    at _init (/Users/bert.degeyter/dev/showpad-typescript/packages/sentry-utils/node_modules/@sentry/src/sdk/init.ts:170:12)
    at Object.init (/Users/bert.degeyter/dev/showpad-typescript/packages/sentry-utils/node_modules/@sentry/src/sdk/init.ts:101:10)
    at Object.init (/Users/bert.degeyter/dev/showpad-typescript/packages/sentry-utils/src/lib/before-breadcrumb.spec.ts:17:20)
    at Promise.then.completed (/Users/bert.degeyter/dev/showpad-typescript/node_modules/jest-circus/build/utils.js:298:28)
    at new Promise (<anonymous>)
    at callAsyncCircusFn (/Users/bert.degeyter/dev/showpad-typescript/node_modules/jest-circus/build/utils.js:231:10)
    at _callCircusHook (/Users/bert.degeyter/dev/showpad-typescript/node_modules/jest-circus/build/run.js:281:40)
    at _runTestsForDescribeBlock (/Users/bert.degeyter/dev/showpad-typescript/node_modules/jest-circus/build/run.js:95:7)
    at _runTestsForDescribeBlock (/Users/bert.degeyter/dev/showpad-typescript/node_modules/jest-circus/build/run.js:121:9)
    at _runTestsForDescribeBlock (/Users/bert.degeyter/dev/showpad-typescript/node_modules/jest-circus/build/run.js:121:9)
    at run (/Users/bert.degeyter/dev/showpad-typescript/node_modules/jest-circus/build/run.js:71:3)
    at runAndTransformResultsToJestFormat (/Users/bert.degeyter/dev/showpad-typescript/node_modules/jest-circus/build/legacy-code-todo-rewrite/jestAdapterInit.js:122:21)
    at jestAdapter (/Users/bert.degeyter/dev/showpad-typescript/node_modules/jest-circus/build/legacy-code-todo-rewrite/jestAdapter.js:79:19)
    at runTestInternal (/Users/bert.degeyter/dev/showpad-typescript/node_modules/jest-runner/build/runTest.js:367:16)
    at runTest (/Users/bert.degeyter/dev/showpad-typescript/node_modules/jest-runner/build/runTest.js:444:34)
@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 May 27, 2024
@github-actions github-actions bot added the Package: node Issues related to the Sentry Node SDK label May 27, 2024
@mydea
Copy link
Member

mydea commented May 27, 2024

So what is the actual problem here? It seems that this is a problem with the env not supporting lazy loading properly. If this is not loaded in jest it's probably not a problem, or are you actually having a problem because of this in your app? Is this only happening in test env, for example?

@TheHolyWaffle
Copy link
Author

TheHolyWaffle commented May 27, 2024

Hi @mydea Apologies if this isn't well defined.

For more context, I maintain a beforeBreadcrumb function in our company and I'd like to test it's behavior using jest. I leverage sentry-testkit for that but I noticed the breadcrumbs are not coming through anymore when upgrading from v7 to v8.

The reason it fails is because it fails to load the native node fetch integration during the jest beforeEach. (The stacktrace from above). I'd like to get this functioning again 🙏

Here's the bigger context:

import { beforeBreadcrumb } from './before-breadcrumb';
import * as Sentry from '@sentry/node';
import sentryTestkit from 'sentry-testkit';

describe('integration test', () => {
    const { testkit, sentryTransport } = sentryTestkit();
    beforeAll(() => {
        Sentry.init({
            dsn: 'https://00000000000000000000000000000000@0000000.ingest.us.sentry.io/0000000',
            transport: sentryTransport,
            beforeBreadcrumb: beforeBreadcrumb,
            integrations: [Sentry.nativeNodeFetchIntegration()],
        });
    });
    beforeEach(() => testkit.reset());
    afterEach(() => Sentry.getCurrentScope().clearBreadcrumbs());

    it('should work for native node fetch', async () => {
        await fetch('http://example.com/fetch', {
            headers: {
                'My-custom-header': 'foo',
                'My-Custom-Header-2': ['foo', 'bar'],
            },
        });
        Sentry.captureException(new Error('foo'));
        await Sentry.flush();
        expect(testkit.reports()).toHaveLength(1);
        expect(testkit.reports()[0]!.breadcrumbs[0]).toStrictEqual({
            timestamp: expect.any(Number),
            category: 'http',
            data: {
                'http.method': 'GET',
                status_code: 200,
                url: 'http://example.com/fetch',
                'http.header.My-custom-header': 'foo',
                'http.header.My-Custom-Header-2': 'foo,bar',
                'http.header.accept': '*/*',
                'http.header.accept-language': '*',
                'http.header.sec-fetch-mode': 'cors',
                'http.header.user-agent': 'node',
                'http.header.accept-encoding': expect.any(String),
                'http.header.sentry-trace': expect.any(String),
                'http.header.baggage': expect.any(String),
            },
            type: 'http',
        });
    });
});

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

mydea commented May 27, 2024

Ah, OK, I can see the problem now.

Honestly I don't think there is an easy way to solve this in the SDK side of things 🤔 did you try to follow docs in jest here: https://jestjs.io/docs/ecmascript-modules for this?

@mydea mydea changed the title Sentry.nativeNodeFetchIntegration fails to import opentelemetry-instrumentation-fetch-node in cjs Sentry.nativeNodeFetchIntegration fails to import opentelemetry-instrumentation-fetch-node in jest May 27, 2024
@TheHolyWaffle
Copy link
Author

I can run the tests using NODE_OPTIONS=--experimental-vm-modules npx jest which makes it succeed.

But I'm wondering if this isn't also an issue in other cjs environments (besides Jest) where one might forget to use the --experimental-vm-modules flag?

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

mydea commented May 27, 2024

I can run the tests using NODE_OPTIONS=--experimental-vm-modules npx jest which makes it succeed.

But I'm wondering if this isn't also an issue in other cjs environments (besides Jest) where one might forget to use the --experimental-vm-modules flag?

So this should work just fine in true cjs apps (we have a bunch of tests covering this, and this is actually what makes this work normally for almost all apps, as the vast majority of apps out there is running in CJS mode as of now). I think it's really just a jest issue which does not properly support all of these things (yet), unfortunately 🤔 See #12137 for tests for this, which all run in CJS.

It's good to know that this flag fixes it, at least! 🙏

I would close this issue for now as there really isn't much we can do about this I believe!

@TheHolyWaffle
Copy link
Author

I agree, this is jest specific problem and using the vm flag fixes it. I should really spend the time to switch to something like vitest....

Thanks for spending some time on this anyway 🙏 @mydea

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 Type: Bug
Projects
Archived in project
Development

No branches or pull requests

2 participants