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

Regression: Patched fetch fails with node-fetch #13220

Closed
3 tasks done
bhollis opened this issue Aug 5, 2024 · 3 comments · Fixed by #13246
Closed
3 tasks done

Regression: Patched fetch fails with node-fetch #13220

bhollis opened this issue Aug 5, 2024 · 3 comments · Fixed by #13246
Assignees
Labels
Package: browser Issues related to the Sentry Browser SDK Type: Bug

Comments

@bhollis
Copy link

bhollis commented Aug 5, 2024

Is there an existing issue for this?

How do you use Sentry?

Sentry Saas (sentry.io)

Which SDK are you using?

@sentry/browser

SDK Version

8.21.0

Framework Version

Node 22.2.0, Jest 29.7.0 (jsdom)

Link to Sentry event

No response

Reproduction Example/SDK Setup

A standard Jest setup with jsdom. Sentry init:

const options: BrowserOptions = {
  enabled: $featureFlags.sentry,
  dsn: 'redacted',
  release: $DIM_VERSION,
  environment: $DIM_FLAVOR,
  ignoreErrors: [],
  sampleRate: $DIM_VERSION === 'beta' ? 0.5 : 0.01, // Sample Beta at 50%, Prod at 1%
  attachStacktrace: true,
  // Only send trace headers to our own server
  tracePropagationTargets: ['https://api.destinyitemmanager.com'],
  integrations: [
    browserTracingIntegration({
      beforeStartSpan: (context) => ({
        ...context,
        // We could use the React-Router integration but it's annoying
        name: window.location.pathname
          .replace(/\/\d+\/d(1|2)/g, '/profileMembershipId/d$1')
          .replace(/\/vendors\/\d+/g, '/vendors/vendorId')
          .replace(/index\.html/, ''),
      }),
    }),
  ],
  tracesSampleRate: 0.001, // Performance traces at 0.1%
};

Steps to Reproduce

In 03257e0#diff-ebaa2ab57cde4323877d589a960abf7decc00a617a9eda536fdb2882bc7073eeR122 Sentry starts calling res.body.getReader(). This is generally available in browser fetch, but not in node-fetch. We use node-fetch to test our code from Jest.

Expected Result

Fetch would work, as it did with @sentry/browser 8.20.0.

Actual Result

TypeError: res.body.getReader is not a function

      113 |
      114 |     try {
    > 115 |       return await fetchFunction(input, init);
          |              ^
      116 |     } finally {
      117 |       if (timer !== undefined) {
      118 |         clearTimeout(timer);

      at resolveResponse (node_modules/.pnpm/@sentry+utils@8.[21](https://github.com/DestinyItemManager/DIM/actions/runs/10193490277/job/28280266147?pr=10364#step:7:22).0/node_modules/@sentry/utils/src/instrument/fetch.ts:125:37)
      at streamHandler (node_modules/.pnpm/@sentry+utils@8.21.0/node_modules/@sentry/utils/src/instrument/fetch.ts:170:9)
      at originalFetch.apply.then.erroredHandlerData (node_modules/.pnpm/@sentry+utils@8.21.0/node_modules/@sentry/utils/src/instrument/fetch.ts:81:13)
@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Aug 5, 2024
@github-actions github-actions bot added the Package: browser Issues related to the Sentry Browser SDK label Aug 5, 2024
@chargome
Copy link
Member

chargome commented Aug 5, 2024

Hey @bhollis, thanks for reaching out!

We use node-fetch to test our code from Jest.

Just trying to understand your setup, can you explain what you mean with that? Since node-fetch is designed for Node.js server environments, why is it used with @sentry/browser?

@bhollis
Copy link
Author

bhollis commented Aug 5, 2024

Sorry, we're implicitly using node-fetch (I think) through the Jest jsdom environment. Jest is a testing tool that runs in NodeJS, but simulates a browser environment (through jsdom) so that you can test code meant for the browser.

@chargome
Copy link
Member

chargome commented Aug 6, 2024

Got it, will be fixed with the next release. Thanks for pointing that out!

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

Successfully merging a pull request may close this issue.

2 participants