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

Warning: Not implemented: process.dlopen doesn't support 'flags' argument #20075

Closed
janpio opened this issue Aug 5, 2023 · 5 comments · Fixed by #20124
Closed

Warning: Not implemented: process.dlopen doesn't support 'flags' argument #20075

janpio opened this issue Aug 5, 2023 · 5 comments · Fixed by #20124
Assignees
Labels
bug Something isn't working correctly node compat

Comments

@janpio
Copy link

janpio commented Aug 5, 2023

A user reported that when using Prisma with Deno 1.36, you get the following warning right now:

deno run -A main.ts
Uncaught Error: Assertion failed
Warning: Not implemented: process.dlopen doesn't support 'flags' argument
[
  { id: 1, email: "test@test.com", name: null },
  { id: 2, email: "test2@test.com", name: null },
  { id: 3, email: "test3@test.com", name: null }
]

prisma/prisma#2452 (comment)

Seems #19860 implemented support for process.dlopen, but not the flags argument.

Prisma indeed uses flags: https://github.com/prisma/prisma/blob/3276015cee8358b3706b5e3014cbafae7b8eb529/packages/client/src/runtime/core/engines/library/DefaultLibraryLoader.ts#L39-L57C56

@bartlomieju
Copy link
Member

@janpio would you be fine if we just ignored this field altogether? It appears the library we use for opening N-API files doesn't support "deepbind" flag anyway: https://docs.rs/libloading/latest/libloading/os/unix/index.html

@janpio
Copy link
Author

janpio commented Aug 5, 2023

I don't have the slightest idea 🤷 😆

I see that it was introduced via prisma/prisma#18426, and before we basically used a lazy require - but had to switch to dlopen with those flag to fix some very nasty bugs on our side related to OpenSSL. Maybe @aqrln knows more.

@aqrln
Copy link

aqrln commented Aug 10, 2023

@bartlomieju

would you be fine if we just ignored this field altogether?

Yeah should be fine. We use the flag to make the dynamic linker resolve OpenSSL symbols from libssl.so.* rather from the Node.js binary (which may not be ABI compatible as we check the system OpenSSL version when determining which engines flavor to use).

As far as I can tell, Deno only exports napi symbols, so it should be totally safe to ignore.

@bartlomieju
Copy link
Member

Sounds good, I'll open a PR that removes the warning tonight and it will be released next week in v1.36.2.

@aqrln
Copy link

aqrln commented Aug 10, 2023

As far as I can tell, Deno only exports napi symbols

Actually, looking at your build.rs and not just at nm -D $(which deno) output on my system, it seems that it's only true when building on a system with modern glibc, otherwise it exports everything.

But looking at the dynamic symbols table in the deno binary, the only thing with global binding and default visibility that strikes my attention as relevant to Prisma is a bunch of sqlite symbols, and those shouldn't pose a problem even when exported since we statically link sqlite.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working correctly node compat
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants