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

deno: move from std node polyfills to node: specifier #771

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Kyiro
Copy link

@Kyiro Kyiro commented Jan 7, 2024

Hello

I've been using postgres.js in a Deno project for a while but I just noticed how outdated the compatibility really is.

Before this gets accepted though, I'd also like to solve these errors when the connection string is invalid/can't connect. I have no idea if these happen on node but I'd assume not.

error: Uncaught TypeError: Cannot read properties of undefined (reading 'replace')
      stack: { value: err.stack + query.origin.replace(/.*\n/, '\n'), enumerable: options.debug },
                                               ^
    at queryError (file:///C:/Projects/postgres/deno/src/connection.js:391:48)
    at errored (file:///C:/Projects/postgres/deno/src/connection.js:386:17)
    at Socket.error (file:///C:/Projects/postgres/deno/src/connection.js:378:5)
    at Socket.emit (ext:deno_node/_stream.mjs:1852:9)
    at emitErrorNT (ext:deno_node/_stream.mjs:1572:13)
    at emitErrorCloseNT (ext:deno_node/_stream.mjs:1544:7)
    at processTicksAndRejections (ext:deno_node/_next_tick.ts:33:15)
    at runNextTicks (ext:deno_node/_next_tick.ts:71:3)
    at eventLoopTick (ext:core/01_core.js:189:21)

@mufaroxyz
Copy link

same 👍

@Kyiro
Copy link
Author

Kyiro commented Jan 30, 2024

It'd honestly be good to get this merged and resolve the error issue at a later date because of those warnings, could we get this PR reviewed?

@porsager
Copy link
Owner

I'm thinking a hard cut in v4 where we just remove the deno polyfill completely if possible?

@Kyiro
Copy link
Author

Kyiro commented Feb 21, 2024

As Deno got npm compatibility a while back, I think it might be the right play but this could still get accepted for v3 to avoid headaches

@mdluo
Copy link

mdluo commented Feb 21, 2024

As Deno got npm compatibility a while back, I think it might be the right play but this could still get accepted for v3 to avoid headaches

Yes, the current version on supabase/edge-runtime it's not just a warning, but it triggers an error and become unusable:

runtime has escaped from the event loop unexpectedly: event loop error: TypeError: internals.warnOnDeprecatedApi is not a function
    at get rid (ext:deno_io/12_io.js:277:15)
    at createWritableStdioStream (https://deno.land/[email protected]/node/_process/streams.mjs:29:21)
    at https://deno.land/[email protected]/node/_process/streams.mjs:68:38

While switching to use this PR the error is gone.

@ardabeyazoglu
Copy link

ardabeyazoglu commented Nov 10, 2024

Note that removing tls polyfill cause ssl connections fail to a database with self signed certificate. This breaks cloud services like Supabase as well. The reason for that is the incompatibility of tls socket options in different runtimes. In nodejs, you set ca: ["base64 certificate content"] in postgres connections options.ssl to mitigate this. However, this didn't work in my Deno. Using the polyfill with Deno and setting caCerts: ["base64 certificate content"] the same way fixed issue. It seems Deno does not map ca parameter to caCerts in node:tls.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants