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

Mongoose and deno support #15824

Closed
Uzlopak opened this issue Sep 8, 2022 · 17 comments
Closed

Mongoose and deno support #15824

Uzlopak opened this issue Sep 8, 2022 · 17 comments

Comments

@Uzlopak
Copy link

Uzlopak commented Sep 8, 2022

Hi,

We are currently trying to support deno in the mongoose project.
Automattic/mongoose#12397 (comment)

We are actually successful with the deno support but it seems that we have some flaky/failing unit tests/performance issues. In node it works reliable.

Maybe you deno experts can give use some valuable feedback regarding the issue we are facing.

Thanks in advance

@littledivy
Copy link
Member

littledivy commented Sep 10, 2022

@Uzlopak Are there any particular performance issues you'd like us to look into?

@vkarpov15
Copy link

@littledivy I ran into a couple of potential issues I'd like to highlight.

  1. TCP sockets using require('net') emit 'error' after 'close' in case of DNS error. For example, in the following script, 'Emitted close' will print before 'Emitted error'. In Node v16.17, these events occur in opposite order. This is significant because the MongoDB Node driver assumes that 'error' will happen before 'close', so DNS errors end up as an uncaught exception in Mongoose currently.
import { createRequire } from "https://deno.land/std/node/module.ts";

const require = createRequire(import.meta.url);

const net = require('net');

const socket = net.createConnection('27009', 'doesnotexist');
socket.once('error', err => console.log('Emitted error', err));
socket.once('close', err => console.log('Emitted close'));

console.log('Done');
  1. I get the following error when running the following script without --unstable. It looks like maybe require('os') is an unstable API?
import { createRequire } from "https://deno.land/std/node/module.ts";

const require = createRequire(import.meta.url);

const mongoose = require('mongoose');

mongoose.connect('mongodb://localhost:27017/test').then(() => console.log('Conn'));
$ deno run --allow-env --allow-read --allow-net gh-12397.js
error: Uncaught TypeError: Requires --unstable
    throw new TypeError("Requires --unstable");
          ^
    at Module.osRelease (https://deno.land/std@0.152.0/_deno_unstable.ts:95:11)
    at Object.release (https://deno.land/std@0.152.0/node/os.ts:230:23)
    at makeClientMetadata (file:///home/val/Workspace/MongoDB/mongoose/node_modules/mongodb/lib/utils.js:544:25)
    at Object.<anonymous> (file:///home/val/Workspace/MongoDB/mongoose/node_modules/mongodb/lib/connection_string.js:581:49)
    at Object.<anonymous> (file:///home/val/Workspace/MongoDB/mongoose/node_modules/mongodb/lib/connection_string.js:1095:4)
    at Module._compile (https://deno.land/std@0.152.0/node/module.ts:260:36)
    at Object.Module._extensions..js (https://deno.land/std@0.152.0/node/module.ts:1332:10)
    at Module.load (https://deno.land/std@0.152.0/node/module.ts:239:34)
    at Function._load (https://deno.land/std@0.152.0/node/module.ts:558:14)
    at Module.require (https://deno.land/std@0.152.0/node/module.ts:225:21)
  1. I get a different error when running Mongoose tests locally. This error looks like it is coming from the Mocha test framework. Do you have any recommendations on how to work around this error?
$ npm run test-deno -- -g "6697"

> mongoose@6.5.4 test-deno
> deno run --allow-env --allow-read --allow-net --allow-run ./test/deno.js

error: Uncaught TypeError: Cannot convert undefined or null to object
          ? Object.values(Deno.consoleSize?.(writer?.rid))
                   ^
    at Function.values (<anonymous>)
    at Writable.value [as getWindowSize] (https://deno.land/std@0.152.0/node/_process/streams.mjs:66:20)
    at Object.<anonymous> (file:///home/val/Workspace/MongoDB/mongoose/node_modules/mocha/lib/reporters/base.js:134:43)
    at Object.<anonymous> (file:///home/val/Workspace/MongoDB/mongoose/node_modules/mocha/lib/reporters/base.js:552:4)
    at Module._compile (https://deno.land/std@0.152.0/node/module.ts:260:36)
    at Object.Module._extensions..js (https://deno.land/std@0.152.0/node/module.ts:1332:10)
    at Module.load (https://deno.land/std@0.152.0/node/module.ts:239:34)
    at Function._load (https://deno.land/std@0.152.0/node/module.ts:558:14)
    at Module.require (https://deno.land/std@0.152.0/node/module.ts:225:21)
    at require (https://deno.land/std@0.152.0/node/module.ts:1387:16)
  1. One of our contributors mentioned that they're seeing connection issues when running tests locally. I have not been able to repro this issue locally. But I'd like to ask if there are any known incompatibilities between Deno's net.Socket implementation and Node's?

@hasezoey
Copy link

@Uzlopak Are there any particular performance issues you'd like us to look into?

i think with the performance issues @Uzlopak means was my comment at Automattic/mongoose#12397 (comment) seeing that deno requires 5 minutes to execute tests while node only requires 3 minutes on the same system with the same code and same backend database
(see Automattic/mongoose#12397 (comment) for the information of deno that was used to run that for that comment)

@piscisaureus
Copy link
Member

  1. TCP sockets using require('net') emit 'error' after 'close' in case of DNS error. For example, in the following script, 'Emitted close' will print before 'Emitted error'. In Node v16.17, these events occur in opposite order. This is significant because the MongoDB Node driver assumes that 'error' will happen before 'close', so DNS errors end up as an uncaught exception in Mongoose currently.

@kt3k we need to fix this, can you help here?

  1. I get the following error when running the following script without --unstable. It looks like maybe require('os') is an unstable API?

require('os') is not unstable in itself, but it depends on certain unstable APIs: Deno.hostname() and Deno.osRelease().
I can't think of a reason why we wouldn't stabilize them. (cc @bartlomieju)

  1. I get a different error when running Mongoose tests locally. This error looks like it is coming from the Mocha test framework. Do you have any recommendations on how to work around this error?

Another case of "unstable API required", in this case Deno.consoleSize(). Do the tests pass when running with --unstable?

One of our contributors mentioned that they're seeing Automattic/mongoose#12397 (comment). I have not been able to repro this issue locally. But I'd like to ask if there are any known incompatibilities between Deno's net.Socket implementation and Node's?

There are no intentional incompatibilities as far as I know - but you've mentioned one bug already in (1), I'm sure there might be others.

@kt3k
Copy link
Member

kt3k commented Sep 22, 2022

1 should be now fixed in deno_std denoland/deno_std#2676

@vkarpov15
Copy link

@piscisaureus I did some more investigation about exactly what bits of unstable APIs Mongoose uses, either directly or indirectly via the MongoDB driver. Below is a list. I can confirm that if I comment out the following uses of unstable APIs, I can run the Mongoose script from this comment without --unstable.

  1. os.release()
  2. socket.setTimeout() from require('net').createConnection() currently fails without --unstable with the following error. With --unstable, no error. The issue is that socket.setTimeout() calls Deno.unrefTimer() under the hood, which is an unstable API. Looks like feat(ext/timers): add refTimer, unrefTimer API (alt) #12953 is the fix?
$ deno run --allow-env --allow-read --allow-net ./deno-test.js 
Error TypeError: Deno.unrefTimer is not a function
    at Timeout.unref (https://deno.land/std@0.152.0/node/internal/timers.mjs:42:10)
    at setUnrefTimeout (https://deno.land/std@0.152.0/node/timers.ts:39:36)
    at Socket.setStreamTimeout [as setTimeout] (https://deno.land/std@0.152.0/node/internal/stream_base_commons.ts:339:22)
    at makeConnection (file:///home/val/Workspace/MongoDB/mongoose/node_modules/mongodb/lib/cmap/connect.js:299:12)
    at connect (file:///home/val/Workspace/MongoDB/mongoose/node_modules/mongodb/lib/cmap/connect.js:31:5)
    at checkServer (file:///home/val/Workspace/MongoDB/mongoose/node_modules/mongodb/lib/sdam/monitor.js:204:27)
    at file:///home/val/Workspace/MongoDB/mongoose/node_modules/mongodb/lib/sdam/monitor.js:237:9
    at executeAndReschedule (file:///home/val/Workspace/MongoDB/mongoose/node_modules/mongodb/lib/utils.js:652:9)
    at makeInterruptibleAsyncInterval (file:///home/val/Workspace/MongoDB/mongoose/node_modules/mongodb/lib/utils.js:659:9)
    at Monitor.connect (file:///home/val/Workspace/MongoDB/mongoose/node_modules/mongodb/lib/sdam/monitor.js:91:71)

The MongoDB node driver uses socket.setTimeout() in the following places:

It was a bit tricky to figure this out because the MongoDB node driver was swallowing one of the Deno.unrefTimer() is not a function errors and attempting to reconnect instead of surfacing the error.

@vkarpov15
Copy link

@piscisaureus it looks like we still need require('os').release() to be stabilized. It looks like os.hostname() is stable, but os.release() still requires --unstable.

@bartlomieju
Copy link
Member

@piscisaureus it looks like we still need require('os').release() to be stabilized. It looks like os.hostname() is stable, but os.release() still requires --unstable.

@vkarpov15 we'll stabilize the required API in the next minor release.

@bartlomieju
Copy link
Member

@Uzlopak it seems Mongoose has now support for Deno, can we close this issue?

@Uzlopak
Copy link
Author

Uzlopak commented Dec 16, 2022

Sure. It was nice to watch the progress ;).

@Uzlopak Uzlopak closed this as completed Dec 16, 2022
@bartlomieju
Copy link
Member

Thanks! Great to see mongoose can be used with Deno!

@eikooc
Copy link

eikooc commented Feb 22, 2023

Sorry to post here, but it looks like it is not possible to connect to a Atlas cluster or a sharded cluster.

The issue is logged at MongoDB's Jira and it is mentioned on the Deno main repo as well
https://jira.mongodb.org/browse/NODE-5042
#16633

@vkarpov15
Copy link

@eikooc it looks like this issue only affects Mongoose 6, works fine in Mongoose 7. We will ship Mongoose 6.11.0 with the fix.

@kazeens
Copy link

kazeens commented May 2, 2023

@vkarpov15 what is the correct way to connect to mongodb with SSL using mongoose ?

The basic options does not seem to work
image

Produces the following error
image

The paths are correct and mongo set up correctly.(checked the ssl connection working fine using GUI(Studio3T))

Could you tell what might be the isssue?

@hasezoey
Copy link

hasezoey commented May 2, 2023

@kazeens please open a discussion / issue at mongoose if you have questions, this issue is not really about this

and also read mongoose: SSL connections if you didnt already

it looks like you are using a older mongoose version, maybe try to upgrade to mongoose 6 or 7 (mongoose 6 is the first version tested with deno too)

PS: if you use atlas with ssl, you might wanna upgrade your mongoose version, because there were recent problems

@kazeens
Copy link

kazeens commented May 2, 2023

@hasezoey Thank you for the lead
But still I struggle to connect after reading documentation multiple times (created even regular nodejs application to check that it works an it works) but with deno it doesn't

The mongoose version is used in my deno application

npm:mongoose@^6.7

Maybe you have any kind of example or reference would, that would help me very much

@hasezoey
Copy link

hasezoey commented May 3, 2023

The mongoose version is used in my deno application

maybe try to upgrade to the currently latest version, which is 6.11.0, maybe the fix feat: upgrade to mongodb 4.16.0 for Deno+Atlas connection fix #13337 #13075 also affects your case

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

No branches or pull requests

9 participants