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

Exception on captureException() call #5622

Closed
3 tasks done
mark-b-ab opened this issue Aug 22, 2022 · 22 comments
Closed
3 tasks done

Exception on captureException() call #5622

mark-b-ab opened this issue Aug 22, 2022 · 22 comments
Labels
Package: node Issues related to the Sentry Node SDK Type: Bug

Comments

@mark-b-ab
Copy link

Is there an existing issue for this?

How do you use Sentry?

Sentry Saas (sentry.io)

Which package are you using?

@sentry/node

SDK Version

7.11.1

Framework Version

No response

Link to Sentry event

No response

Steps to Reproduce

Catch exception, call captureException.

Expected Result

Exception being reported to Sentry

Actual Result

/home/node/node_modules/@sentry/utils/cjs/misc.js:25
    crypto && crypto.getRandomValues ? () => crypto.getRandomValues(new Uint8Array(1))[0] : () => Math.random() * 16;
                                                                                      ^

TypeError: Cannot read properties of undefined (reading '0')
    at /home/node/node_modules/@sentry/utils/cjs/misc.js:25:87
    at /home/node/node_modules/@sentry/utils/cjs/misc.js:30:19
    at String.replace (<anonymous>)
    at Object.uuid4 (/home/node/node_modules/@sentry/utils/cjs/misc.js:29:46)
    at Hub.captureException (/home/node/node_modules/@sentry/hub/cjs/hub.js:130:86)
    at /home/node/node_modules/@sentry/node/cjs/integrations/onuncaughtexception.js:71:17
    at Hub.withScope (/home/node/node_modules/@sentry/hub/cjs/hub.js:98:7)
    at OnUncaughtException.handler (/home/node/node_modules/@sentry/node/cjs/integrations/onuncaughtexception.js:69:15)
    at process.emit (node:events:525:35)
    at process.emit (node:domain:489:12)
    at process._fatalException (node:internal/process/execution:167:25)
@Lms24
Copy link
Member

Lms24 commented Aug 29, 2022

Hi @mark-b-ab thanks for reporting!

We recently made a change (#5426) to how we create uuids so this might have to something with your problem.

Could you tell us a little bit about your setup? For instance, it would be good to know the node version you're using? Also, are you using any frameworks (e.g. Express)? Can you provide a small reproduction what to do to get to this error?

@Lms24 Lms24 added Package: node Issues related to the Sentry Node SDK Status: Needs Reproduction and removed Status: Untriaged labels Aug 29, 2022
@mark-b-ab
Copy link
Author

I use node:18 container. NestJS as a framework. I can't send you code to reproduce as this error I receive if I have any unhandled exception in my app.

@mark-b-ab
Copy link
Author

I added this code it try catch where I report exceptions

      try {
        const { crypto, msCrypto } = getGlobalObject();
        console.log('global.crypto || global.msCrypto', crypto, msCrypto);
      } catch (e) {
        console.log('global.crypto || global.msCrypto crashed', e);
      }

      try {
        const global = getGlobalObject();
        const crypto = global.crypto || global.msCrypto;

        console.log('randomUUID func', crypto.randomUUID);
        console.log('randomUUID called', crypto.randomUUID());
      } catch (e) {
        console.log('randomUUID crashed', e);
      }

      try {
        const global = getGlobalObject();
        const crypto = global.crypto || global.msCrypto;

        console.log('randomBytes func', crypto.randomBytes);
        console.log('randomBytes called', crypto.randomBytes());
      } catch (e) {
        console.log('randomBytes crashed', e);
      }

      try {
        const global = getGlobalObject();
        const crypto = global.crypto || global.msCrypto;

        console.log('getRandomValues func', crypto.getRandomValues);
        console.log('getRandomValues called', crypto.getRandomValues(new Uint8Array(1)));
      } catch (e) {
        console.log('getRandomValues crashed', e);
      }

      try {
        console.log('uuid4 func', uuid4);
        console.log('uuid4 called', uuid4());
      } catch (e) {
        console.log('uuid4 crashed', e);
      }

      try {
        console.log('crypto UUID func', randomUUID);
        console.log('crypto UUID called', randomUUID());
      } catch (e) {
        console.log('crypto UUID crashed', e);
      }

@mark-b-ab
Copy link
Author

mark-b-ab commented Sep 10, 2022

Here is result

global.crypto || global.msCrypto { getRandomValues: [Function: getRandomValues] } undefined
randomUUID func undefined
randomUUID crashed TypeError: crypto.randomUUID is not a function
    at KafkaConsumer.onBatch (/home/node/dist/apps/ethereum-indexer-sushiswap/main.js:1:181174)
    at async Runner.processEachBatch (/home/node/node_modules/kafkajs/src/consumer/runner.js:274:7)
    at async onBatch (/home/node/node_modules/kafkajs/src/consumer/runner.js:449:9)
    at async Runner.handleBatch (/home/node/node_modules/kafkajs/src/consumer/runner.js:461:5)
    at async /home/node/node_modules/kafkajs/src/consumer/worker.js:29:9
randomBytes func undefined
randomBytes crashed TypeError: crypto.randomBytes is not a function
    at KafkaConsumer.onBatch (/home/node/dist/apps/ethereum-indexer-sushiswap/main.js:1:181397)
    at async Runner.processEachBatch (/home/node/node_modules/kafkajs/src/consumer/runner.js:274:7)
    at async onBatch (/home/node/node_modules/kafkajs/src/consumer/runner.js:449:9)
    at async Runner.handleBatch (/home/node/node_modules/kafkajs/src/consumer/runner.js:461:5)
    at async /home/node/node_modules/kafkajs/src/consumer/worker.js:29:9
getRandomValues func [Function: getRandomValues]
getRandomValues called undefined
uuid4 func [Function: uuid4]
uuid4 crashed TypeError: Cannot read properties of undefined (reading '0')
    at /home/node/node_modules/@sentry/utils/cjs/misc.js:25:87
    at /home/node/node_modules/@sentry/utils/cjs/misc.js:30:19
    at String.replace (<anonymous>)
    at uuid4 (/home/node/node_modules/@sentry/utils/cjs/misc.js:29:46)
    at KafkaConsumer.onBatch (/home/node/dist/apps/ethereum-indexer-sushiswap/main.js:1:181783)
    at async Runner.processEachBatch (/home/node/node_modules/kafkajs/src/consumer/runner.js:274:7)
    at async onBatch (/home/node/node_modules/kafkajs/src/consumer/runner.js:449:9)
    at async Runner.handleBatch (/home/node/node_modules/kafkajs/src/consumer/runner.js:461:5)
    at async /home/node/node_modules/kafkajs/src/consumer/worker.js:29:9
crypto UUID func [Function: randomUUID]
crypto UUID called 7871dbb7-ccd7-4ca9-8a8e-4b6c33af6468

@mark-b-ab
Copy link
Author

The last UUID is from node crypto module

@mark-b-ab
Copy link
Author

Seems that Sentry SDK is not using NodeJS Crypto module but a polyfill

@lobsterkatie
Copy link
Member

lobsterkatie commented Sep 12, 2022

Hi, @mark-b-ab.

Thanks for doing that investigation! The results are puzzling, though:

  • The global crypto object should only exist when running your node process with the --experimental-global-webcrypto flag. Are you? If not, I would not expect global.crypto to be defined at all, and yet for you it shows as { getRandomValues: [Function: getRandomValues] }.

  • Given that global.crypto is there at all, though, I'd expect it to have randomUUID (added in node 16), since you say you're running your app in node 18.

  • global.crypto does seem to have getRandomValues (as I would expect, since it was added in node 15), but for some reason it's returning undefined, which isn't listed in the docs as one of the possible return values.

  • At the beginning of your test code, you call crypto.randomUUID and it's undefined. At the end, you call randomUUID (which you say is from crypto) and it's defined. I assume that's because in the latter case you're talking about actually having required/imported the built-in crypto module, rather than relying on the global version?

UPDATE: I did some experimenting on my own machine, with more confusing results. Specifically, while the node docs say that global.crypto should be different from the built-in crypto module, for me, both bare crypto and global.crypto give me the full crypto module, same as when I require('crypto'). (And no, I'm not running with the special flag I mentioned.) This happens in both node 14 and node 18.

In any case, my best guess is that maybe this has to do with the node binary you're using inside your container. Do you know anything about it other than its version?

Meanwhile, @timfish, since you worked on this - do you have any idea what's going on here? It all seems very strange...

@timfish
Copy link
Collaborator

timfish commented Sep 12, 2022

Yeah very strange. The stack trace and results from that script don't add up in a way I can fathom. Would love to try out that container!

(And no, I'm not running with the special flag I mentioned.)

My testing showed that in node v18+ global.crypo.randomUUID() worked in official builds. I never did work out what was wrong with their docs!

@lforst
Copy link
Member

lforst commented Sep 13, 2022

In any case, my best guess is that maybe this has to do with the node binary you're using inside your container. Do you know anything about it other than its version?

@mark-b-ab Do you happen to have the specific version and hash/id of the docker image you were using to get this error? Or were you just using the unpinned version? Having the exact version would help a lot in debugging this.

One other random thought: Maybe there's stuff missing we need in reduced versions of images like alpine.

@mark-b-ab
Copy link
Author

I am using just node:18. Weird thing that if I just launch container I can see full crypto module using global.crypto but when I can an exception in Kafka Consumer and try to report it does not see all of those methods

@lforst
Copy link
Member

lforst commented Sep 13, 2022

There is some funky stuff going on in the kafkajs package: https://github.com/tulios/kafkajs/blob/master/src/producer/partitioners/legacy/randomBytes.js

I can't pinpoint the exact problem yet but I bet it has to do with this.

@max-korsun-nuant
Copy link

Hello guys!
I faced the same issue using node:18
Library version: 7.12.1 installed by yarn version 3.2.0
The error occurred in function uuid4 from utils/cjs/misc.js:

function uuid4() {
 var global$1 = global.getGlobalObject() ;
 var crypto = (global$1.crypto || global$1.msCrypto) ;

 if (crypto && crypto.randomUUID) {
   return crypto.randomUUID().replace(/-/g, '');
 }

 var getRandomByte =
   crypto && crypto.getRandomValues ? () => crypto.getRandomValues(new Uint8Array(1))[0] : () => Math.random() * 16;

 // http://stackoverflow.com/questions/105034/how-to-create-a-guid-uuid-in-javascript/2117523#2117523
 // Concatenating the following numbers as strings results in '10000000100040008000100000000000'
 return (([1e7] ) + 1e3 + 4e3 + 8e3 + 1e11).replace(/[018]/g, c =>
       ((c ) ^ ((getRandomByte() & 15) >> ((c ) / 4))).toString(16),
 );
}

I have tried to watch the returned global.crypto and global.msCrypto (properties from the uuid4 function code) object from function getGlobalObject.
And its prints me:
crypto: { getRandomValues: [Function: getRandomValues] }

I print the function code of crypto.getRandomValues:

  getRandomValues(b) {
      crypto.randomFillSync(b);
  }

And there is no return keyword so crypto.getRandomValues always returns undefined.

The function getGlobalObject from utils/cjs/global.js:

function getGlobalObject() {
  return (
    node.isNodeEnv()
      ? global
      : typeof window !== 'undefined'       ? window       : typeof self !== 'undefined'
      ? self
      : fallbackGlobalObject
  ) ;
}

node.isNodeEnv returns true

@lforst
Copy link
Member

lforst commented Sep 14, 2022

@mark-b-ab @max-korsun-nuant Random question but is anyone of you using Astro? Or wasm for that matter?

@shellscape
Copy link

I'm getting this same error, but for exception while calling captureException.

TypeError: Cannot read properties of undefined (reading 'exception')
  File "/node_modules/.pnpm/@[email protected]/node_modules/@sentry/src/integrations/linkederrors.ts", line 59, in e._handler
  File "/node_modules/.pnpm/@[email protected]/node_modules/@sentry/src/integrations/linkederrors.ts", line 10, in <anonymous>
  File "/node_modules/.pnpm/@[email protected]/node_modules/@sentry/src/scope.ts", line 515, in <anonymous>
  File "/node_modules/.pnpm/@[email protected]/node_modules/@sentry/src/syncpromise.ts", line 58, in new is
  File "/node_modules/.pnpm/@[email protected]/node_modules/@sentry/src/scope.ts", line 510, in BA._notifyEventProcessors
...
(4 additional frame(s) were not displayed)

How is the error handling within the sentry node package so extremely poor, that errors like this bubble up with vague messaging instead of being caught within the lib, with proper custom errors (with codes) thrown? This is basic package authoring stuff folks.

@lforst
Copy link
Member

lforst commented Sep 20, 2022

@shellscape I believe the error you posted is unrelated to this particular issue (crypto module methods being undefined for some reason), but I think what you posted is still valid and needs to be fixed.

Would you mind creating a separate issue for this so we can track it there? Thank you!

@shellscape
Copy link

@lforst already did. you can see the reflink directly above your reply

@lforst
Copy link
Member

lforst commented Sep 20, 2022

@shellscape Cool thank you!

@github-actions
Copy link
Contributor

This issue has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you label it Status: Backlog or Status: In Progress, I will leave it alone ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

@cdignam-segment
Copy link

I just encountered this issue too. Same exception as #5622 (comment), but I haven't been able to replicate on my local machine

@cdignam-segment
Copy link

I'm on node v16.16.0, which doesn't have crypto.getRandomValues. Yet somehow sentry was triggering this code. I'm guessing there is some conflicting node module.

crypto && crypto.getRandomValues ? () => crypto.getRandomValues(new Uint8Array(1))[0] : () => Math.random() * 16;
                                                                                      ^
TypeError: Cannot read properties of undefined (reading '0')
    at /control-plane/node_modules/@sentry/src/misc.ts:33:1
    at /control-plane/node_modules/@sentry/src/misc.ts:39:1
    at String.replace (<anonymous>)
    at Object.uuid4 (/control-plane/node_modules/@sentry/src/misc.ts:37:1)
    at Hub.captureException (/control-plane/node_modules/@sentry/src/hub.ts:187:1)
    at Object.captureException (/control-plane/node_modules/@sentry/src/exports.ts:36:1)

@cdignam-segment
Copy link

I found the issue. We had a bad polyfill included by a dependency.

The polyfill was:

globalThis.crypto = {                                                                                                                                                                                                                                                                                                                                                                                        
  getRandomValues: function getRandomValues(b) {                                                                                                                                                                  
    require('crypto').randomFillSync(b) // missing return statement!                                                                                                                                                   
  },                                                                                                                                                                                                              
}

should have been:

globalThis.crypto = {                                                                                                                                                                                                                                                                                                                                                                                        
  getRandomValues: function getRandomValues(b) {                                                                                                                                                                  
    return require('crypto').randomFillSync(b)                                                                                                                                                                    
  },                                                                                                                                                                                                              
} 

@Marko298
Copy link

Marko298 commented Feb 8, 2023

it helped, thanks!

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

No branches or pull requests

10 participants