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 support #12397

Merged
merged 33 commits into from Nov 28, 2022
Merged

Deno support #12397

merged 33 commits into from Nov 28, 2022

Conversation

vkarpov15
Copy link
Collaborator

Summary

Tests are passing against Deno 👍 Still with --unstable, so there's some cleanup there, but this is still a big step.

For the most part, no noteworthy runtime issues beyond #12313. Just a bunch of test issues due to our tests being a bit sloppy with schematype global state cleanup, and a quirk with dns resolution throwing an uncatchable error. We'll sort those out a little more later.

Examples

@hasezoey
Copy link
Collaborator

hasezoey commented Sep 6, 2022

tested it locally and seems to be fine, though there are some errors which dont seem to be consistent, but dont really happen in normal node:

  2862 passing (5m)
  31 pending
  5 failing

  1) Model
       dropDatabase() after init allows re-init (gh-6967):
     MongoNetworkError: connection 3 to 127.0.0.1:27017 closed
      at Connection.onClose (file:///mnt/projects/nodejs/mongoose/node_modules/mongodb/lib/cmap/connection.js:135:19)
      at Socket.<anonymous> (file:///mnt/projects/nodejs/mongoose/node_modules/mongodb/lib/cmap/connection.js:62:46)
      at Socket.emit (https://deno.land/std@0.154.0/node/_events.mjs:391:35)
      at https://deno.land/std@0.154.0/node/net.ts:1417:14
      at TCP.close (https://deno.land/std@0.154.0/node/internal_binding/handle_wrap.ts:37:5)
      at Socket._destroy (https://deno.land/std@0.154.0/node/net.ts:1409:20)
      at _destroy (https://deno.land/std@0.154.0/node/internal/streams/destroy.mjs:96:25)
      at Socket.destroy (https://deno.land/std@0.154.0/node/internal/streams/destroy.mjs:58:5)
      at Timeout.<anonymous> (file:///mnt/projects/nodejs/mongoose/node_modules/mongodb/lib/cmap/connection.js:145:27)
      at https://deno.land/std@0.154.0/node/timers.ts:21:21
      at Object.action (deno:ext/web/02_timers.js:149:13)
      at handleTimerMacrotask (deno:ext/web/02_timers.js:66:12)

  2) Model
       "after each" hook for "dropDatabase() after init allows re-init (gh-6967)":
     Error: Timeout of 8000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (/mnt/projects/nodejs/mongoose/test/model.test.js)
      at https://deno.land/std@0.154.0/node/timers.ts:21:21
      at Object.action (deno:ext/web/02_timers.js:149:13)
      at handleTimerMacrotask (deno:ext/web/02_timers.js:66:12)

  3) Query
       deleteOne/deleteMany
         handles deleteMany:
     MongoNetworkError: connection 2 to 127.0.0.1:27017 closed
      at Connection.onClose (file:///mnt/projects/nodejs/mongoose/node_modules/mongodb/lib/cmap/connection.js:135:19)
      at Socket.<anonymous> (file:///mnt/projects/nodejs/mongoose/node_modules/mongodb/lib/cmap/connection.js:62:46)
      at Socket.emit (https://deno.land/std@0.154.0/node/_events.mjs:391:35)
      at https://deno.land/std@0.154.0/node/net.ts:1417:14
      at TCP.close (https://deno.land/std@0.154.0/node/internal_binding/handle_wrap.ts:37:5)
      at Socket._destroy (https://deno.land/std@0.154.0/node/net.ts:1409:20)
      at _destroy (https://deno.land/std@0.154.0/node/internal/streams/destroy.mjs:96:25)
      at Socket.destroy (https://deno.land/std@0.154.0/node/internal/streams/destroy.mjs:58:5)
      at Timeout.<anonymous> (file:///mnt/projects/nodejs/mongoose/node_modules/mongodb/lib/cmap/connection.js:145:27)
      at https://deno.land/std@0.154.0/node/timers.ts:21:21
      at Object.action (deno:ext/web/02_timers.js:149:13)
      at handleTimerMacrotask (deno:ext/web/02_timers.js:66:12)

  4) Query
       "after each" hook for "handles deleteMany":
     Error: Timeout of 8000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (/mnt/projects/nodejs/mongoose/test/query.test.js)
      at https://deno.land/std@0.154.0/node/timers.ts:21:21
      at Object.action (deno:ext/web/02_timers.js:149:13)
      at handleTimerMacrotask (deno:ext/web/02_timers.js:66:12)

  5) Query
       "after all" hook for "skips applying default projections over slice projections (gh-11940)":
     Error: Timeout of 8000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (/mnt/projects/nodejs/mongoose/test/query.test.js)
      at https://deno.land/std@0.154.0/node/timers.ts:21:21
      at Object.action (deno:ext/web/02_timers.js:149:13)
      at handleTimerMacrotask (deno:ext/web/02_timers.js:66:12)
normal node run on my system
  3246 passing (3m)
  47 pending
  6 failing

  1) model: querying:
       lean
         casts $elemMatch (gh-2199):
     Error: Timeout of 2000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (/mnt/projects/nodejs/mongoose/test/model.querying.test.js)
      at listOnTimeout (node:internal/timers:564:17)
      at process.processTimers (node:internal/timers:507:7)

  2) Model
       can be created using _id as embedded document:
     Uncaught AssertionError [ERR_ASSERTION]: undefined == 'Daniel'
      at /mnt/projects/nodejs/mongoose/test/model.test.js:117:16
      at /mnt/projects/nodejs/mongoose/lib/model.js:5135:18
      at process.processTicksAndRejections (node:internal/process/task_queues:77:11)

  3) Model
       getters
         nested
           pre-existing null object re-save:
     Uncaught AssertionError [ERR_ASSERTION]: ifError got unwanted exception: Operation `Test.findOne()` buffering timed out after 10000ms
      at /mnt/projects/nodejs/mongoose/test/model.querying.test.js:2477:18
      at /mnt/projects/nodejs/mongoose/lib/model.js:5135:18
      at process.processTicksAndRejections (node:internal/process/task_queues:77:11)
      at Timeout.<anonymous> (lib/drivers/node-mongodb-native/collection.js:153:23)
      at listOnTimeout (node:internal/timers:564:17)
      at process.processTimers (node:internal/timers:507:7)

  4) Model
       bug fixes
         createCollection() enforces expireAfterSeconds (gh-11229):
     Uncaught AssertionError [ERR_ASSERTION]: ifError got unwanted exception: Operation `Test.updateOne()` buffering timed out after 10000ms
      at /mnt/projects/nodejs/mongoose/test/model.test.js:1889:20
      at /mnt/projects/nodejs/mongoose/lib/model.js:5135:18
      at process.processTicksAndRejections (node:internal/process/task_queues:77:11)
      at Timeout.<anonymous> (lib/drivers/node-mongodb-native/collection.js:153:23)
      at listOnTimeout (node:internal/timers:564:17)
      at process.processTimers (node:internal/timers:507:7)

  5) Model
       bug fixes
         createCollection() enforces expires when set by Schema (gh-11229):

      AssertionError [ERR_ASSERTION]: 12 == 0
      + expected - actual

      -12
      +0
      
      at Context.<anonymous> (test/model.test.js:7203:14)
      at process.processTicksAndRejections (node:internal/process/task_queues:95:5)

  6) Model
       bug fixes
         createCollection() enforces expireAfterSeconds (gh-11229):
     done() called multiple times in test <Model bug fixes createCollection() enforces expireAfterSeconds (gh-11229)> of file /mnt/projects/nodejs/mongoose/test/model.test.js; in addition, done() received error: MongooseError: Operation `TestGH11229Var1.count()` buffering timed out after 10000ms
    at Timeout.<anonymous> (/mnt/projects/nodejs/mongoose/lib/drivers/node-mongodb-native/collection.js:153:23)
    at listOnTimeout (node:internal/timers:564:17)
    at process.processTimers (node:internal/timers:507:7)
  Error: done() called multiple times in test <Model bug fixes createCollection() enforces expireAfterSeconds (gh-11229)> of file /mnt/projects/nodejs/mongoose/test/model.test.js; in addition, done() received error: MongooseError: Operation `TestGH11229Var1.count()` buffering timed out after 10000ms
      at Timeout.<anonymous> (lib/drivers/node-mongodb-native/collection.js:153:23)
      at listOnTimeout (node:internal/timers:564:17)
      at process.processTimers (node:internal/timers:507:7)

@hasezoey hasezoey mentioned this pull request Sep 6, 2022
Copy link
Collaborator

@AbdelrahmanHafez AbdelrahmanHafez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks 👍

Copy link
Collaborator

@Uzlopak Uzlopak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no objections.

Copy link
Collaborator

@hasezoey hasezoey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also looks good to me, though i still dont know why in deno so many timeout / connection close errors are happening locally (see earlier comment)

@vkarpov15
Copy link
Collaborator Author

@hasezoey what version of Deno are you using?

@vkarpov15 vkarpov15 added this to the 6.7 milestone Sep 8, 2022
@hasezoey
Copy link
Collaborator

hasezoey commented Sep 8, 2022

@hasezoey what version of Deno are you using?

deno 1.25.1
installed through cargo install deno (no changed features)
compiled with rust 1.63.0 (stable toolchain)
on a x86_64 manjaro-linux system (same system and mongodb with which i run node)
tests run through yarn run test-deno (and node with yarn run test)

PS: sometimes i also get the timeouts with node, but it seems to be more common in deno because deno somehow runs slower than node (see the time in the comment, where deno was 5m and node only 3m)

Edit: i am on the machine again and can provide deno --version:

deno 1.25.1 (release, x86_64-unknown-linux-gnu)
v8 10.6.194.5
typescript 4.7.4

README.md Show resolved Hide resolved
@vkarpov15 vkarpov15 modified the milestones: 6.7, 6.8 Oct 21, 2022
@vkarpov15
Copy link
Collaborator Author

Moving this to 6.8, still waiting on feedback from Deno team.

@Uzlopak
Copy link
Collaborator

Uzlopak commented Oct 27, 2022

@vkarpov15
Deno 1.27 is out. I guess it has now the necessary compatibility?

.github/workflows/test.yml Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
test/deno.js Show resolved Hide resolved
test/deno.js Outdated Show resolved Hide resolved
@vkarpov15 vkarpov15 changed the base branch from 6.7 to 6.8 November 28, 2022 17:10
@vkarpov15 vkarpov15 merged commit 0c98cbb into 6.8 Nov 28, 2022
@hasezoey hasezoey deleted the vkarpov15/deno branch November 29, 2022 10:16
@roj1512
Copy link

roj1512 commented Nov 29, 2022

Does this really need a node_modules?

@hasezoey
Copy link
Collaborator

Does this really need a node_modules?

@roj1512 could you explain what you mean?

@roj1512
Copy link

roj1512 commented Nov 29, 2022

Do we need NPM and node_modules to use this Deno support?

@hasezoey
Copy link
Collaborator

hasezoey commented Nov 29, 2022

Do we need NPM and node_modules to use this Deno support?

read the deno documentation at NPM Specifiers

TL;DR:

When doing this, no npm install is necessary and no node_modules folder is created. These packages are also subject to the same permissions as Deno applications. At the moment though, there are some unnecessary permissions that get asked for, but in the future the above program will only require network permissions.

unless option --node-modules-dir is used (though from what i can tell, it is not required for mongoose)

though note that i did not use this yet so i cannot verify that this is actually true (probably deno still has a cache somewhere)

@roj1512
Copy link

roj1512 commented Nov 29, 2022

Sad.

@vkarpov15
Copy link
Collaborator Author

Currently yes, you do need npm and node_modules. We'll consider improving this in the future.

if (typeof Deno !== 'undefined') {
// In Deno dns throws an uncatchable error here.
return this.skip();
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

btw, are there any news regarding this "uncatchable dns error"?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hasezoey nope, I don't think this issue is significant enough to block shipping deno support.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

didnt mean to block with this question, just wanted to ask if there are any known updates regarding this problem to re-enable the test for deno

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.

None yet

5 participants