Skip to content

Using nodejs build-in fetch doesn't seem to work with the undici plugin #7958

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
wirtsi opened this issue Apr 25, 2023 · 7 comments
Closed
3 tasks done
Labels
Package: node Issues related to the Sentry Node SDK

Comments

@wirtsi
Copy link

wirtsi commented Apr 25, 2023

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

7.49.0

Framework Version

No response

Link to Sentry event

No response

SDK Setup

  Sentry.init({
    dsn: `<dsn>`,
    environment,
    integrations: [
      // enable HTTP calls tracing
      new Sentry.Integrations.Http({ tracing: true }),
      new Sentry.Integrations.Undici({
        shouldCreateSpanForRequest: (url) => {
          console.log(url);
          return true;
        },
      }),
    ],
    debug: true,
    // Set tracesSampleRate to 1.0 to capture 100%
    // of transactions for performance monitoring.
    // We recommend adjusting this value in production
    tracesSampleRate: 1,
  });

Steps to Reproduce

I have some custom written code that uses Node v19 fetch (so without undici). Code looks like this

const response = await fetch(endpoint, { method: 'POST', body: JSON.stringify({ query: print(document), variables }), headers: { 'Content-Type': 'application/json', ...headers }, });
I am also using the algoliasearch package which uses node's http.

Expected Result

I would expect that sentries creates a span for my fetchoperation.

Actual Result

First call is the one using algolia, second one fetch

Sentry Logger [log]: [Tracing] Starting 'http.client' span on transaction '/en-gb/c
ollections/women-dresses' (bd112c3d23c5805b).
Sentry Logger [log]: [Tracing] Adding sentry-trace header 924d5e8b06ea43fb8bef49de1
d763358-9e98a1ca69022d7c-1 to outgoing request to "https://5fgvyedx6g-1.algolianet.
com/":
Sentry Logger [log]: [Tracing] Finishing 'http.client' span on transaction '/en-gb/
collections/women-dresses' (bd112c3d23c5805b).
Sentry Logger [log]: [Tracing] Finishing 'http.client' span on transaction '/en-gb/
collections/women-dresses' (bd112c3d23c5805b).
Sentry Logger [log]: [Tracing] Finishing handle-request transaction: /en-gb/collect
ions/women-dresses.
Sentry Logger [log]: [Tracing] starting handle-request transaction - /en-gb/p/s14-o
nl-li-5656-black
Sentry Logger [log]: [Tracing] Finishing handle-request transaction: /en-gb/p/s14-o
nl-li-5656-black.

@wirtsi
Copy link
Author

wirtsi commented Apr 25, 2023

Ah and I am not seeing the url from the console.log in the shouldCreateSpanForRequest ... so it's not even triggered I guess

@wirtsi
Copy link
Author

wirtsi commented Apr 25, 2023

So dug around this a bit more. I was able to subscribe to the events emitted by fetch (which is built into my node version) like this:

const ds = require('diagnostics_channel')
ds.subscribe('undici:request:create', message => {console.log(message)})
await fetch()

But it still doesn't seem to work in the sentry undici plugin. Turns out I can make it work if I do this in the node_modules/@sentry/node/cjs/integrations/undici/index.js file

    try {
      // eslint-disable-next-line @typescript-eslint/no-var-requires
      // ds = utils.dynamicRequire(module, 'diagnostics_channel');
      ds = require('diagnostics_channel');
    } catch (e) {
      // no-op
    }

So for some reasons, the dynamic import fails for me, just requiring it works fine

🤷

@lforst
Copy link
Contributor

lforst commented Apr 26, 2023

Hi, are you using ESM mode (or "type": "module" in package.json) when running node?

@lforst lforst added the Package: node Issues related to the Sentry Node SDK label Apr 26, 2023
@wirtsi
Copy link
Author

wirtsi commented Apr 26, 2023

No, it's using commonjs. I verified by also editing the file in the esm folder (in node_modules), that had no impact

@lforst
Copy link
Contributor

lforst commented Apr 26, 2023

No, it's using commonjs. I verified by also editing the file in the esm folder (in node_modules), that had no impact

Aight. This one is kinda weird. Can you check if this is still not working with Node 20? Maybe Node 19 is buggy (wouldn't be the first time it happened)?

@wirtsi
Copy link
Author

wirtsi commented Apr 26, 2023

Dammit, you are right ... node@20 fixes the issue. This is wild! Thanks for your help

@lforst
Copy link
Contributor

lforst commented Apr 26, 2023

Glad to hear! I will close this then. @AbhiPrasad said he will update our docs in case anybody else runs into this.

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
None yet
Development

No branches or pull requests

2 participants