Segmentation fault in recent Okta versions

[PinkaminaDianePie]

Describe the bug?

Application with Okta 6.4, tested through Jest fails with segmentation fault error (SIGSEGV).

What is expected to happen?

It should work.

What is the actual behavior?

Process crashed with SIGSEGV code

Reproduction Steps?

Run Jest tests with latest Okta. 1 out of 10 runs approximately process hangs indefinitely. If Jest run in a single thread (--detectOpenHandles --forceExit" flags), process crashes with segmentation fault error.

I used this package https://github.com/Shiranuit/node-segfault-handler to get a stack trace and debug segmentation fault and it showed me that process crashes on the line return await require('crypto'); introduced in this commit: 1ef5298#diff-5b4547ee12ebc3778f7ddf11232feba3cdd86509e2eed67952400d3a87a359b8R42

Manual change in node_modules from import to require fixes the issue. For now I have to rely on yarn patch to use latest Okta.

Importing crypto using require is an already reported bug (nodejs/node#38695)
That's a well-known issue and probably won't be fixed anytime soon: nodejs/node#25424 (comment)

My suggestion is to switch back from imports to requires here and all other places just in case

SDK Versions

"@okta/okta-auth-js": "6.4.0"

Execution Environment

Node: 14.x/16.x
OS: mac/ubuntu

Additional Information?

No response

[jaredperreault-okta]

@PinkaminaDianePie Can you provide more information about your application and testing environment? Is this a node app or web app? What version of jest are you using? Perhaps paste your jest config?

Do you experience this same issue with older 6.x versions of auth-js? Or this new with 6.4?

[PinkaminaDianePie]

web app, jest 27.5.1. jest config is created by a script, so I can't paste it.
I tested 6.2/6.3/6.4, issue is everywhere, and only later I realized that it's related to the usage of import function, so it's present in all versions where crypto is imported via import instead of require

[shuowu]

@PinkaminaDianePie he APIs Jest uses to implement ESM support is still considered experimental by Node, you probably will need the --experimental-vm-modules flag when use ESM syntax.

Or you can try to point to the cjs version of auth-js with the moduleMapper jest config.

Reference: https://jestjs.io/docs/ecmascript-modules

[PinkaminaDianePie]

Okta V5:

Retiring on 2022-10-31

And the only thing I can do is switch to the new version, which relies on the experimental node feature? Sounds weird. And still not sure how is it related to my problem. Modules just work, but they are bugged, why an additional flag would help me?

I can't point to the cjs version of auth-js just because it's already used by default, and it still has import instead of require. Check the node_modules/@okta/okta-auth-js/cjs/crypto/node.js, there will be esm import inside.

[PinkaminaDianePie]

Btw, you probably confusing normal ESM imports with dynamic imports, the dynamic one is not anyhow related to the Jest, and is supported by Node natively: https://nodejs.org/api/esm.html#import-expressions

That kind of import is used by okta even in cjs files, so that's the reason of segmentation fault

[shuowu]

@PinkaminaDianePie Thanks for the explanation! I think I understand the issue now, it's because the ESM syntax (dynamic import) is leaked to the CJS bundle during transpiling stage. We will work on a patch release soon.

Internal Ref: OKTA-488924

[PinkaminaDianePie]

I think it should not go to the ESM bundle as well, because of the node issues mentioned above. Dynamic imports are available in CJS context as well, so CJS environment could still use it in the same way as ESM. The issue is in how Node handle modules, so ESM environment would still have this segmentation fault. You would have it even in REPL! You can read more here nodejs/node#38695 or here nodejs/node#25424 (comment)

It's just not safe to import('crypto') from any context, it's better to use require('crypto')

[PinkaminaDianePie]

I'd appreciate it if you merge this one #1179, it's critical for us. We can't upgrade to the okta 6 because of it and so we postpone the planned major release of our software

[shuowu]

@PinkaminaDianePie I have released a patch that fixes the cjs transpiling issue, which I think should unblock you from the reported Jest error. Can you try that version out see if it resolves your issue?

Meanwhile we'll keep investigating if we should replacing dynamic import with require. This change may affect node version ESM bundle, we will need sometime to test the suggested approach.

[PinkaminaDianePie]

I can confirm, that solved my issue, thank you!