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

fix(connection): unref the "_waitForConnect" #15245

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

hasezoey
Copy link
Collaborator

@hasezoey hasezoey commented Feb 8, 2025

Summary

The recent change in #15229 has added a timeout, which can lead to having the node process around for longer than expected (ie until the timeout has fired), which can cause jest to detect open handles.
(I had also encountered this while upgrading typegoose)

Also the check of unref existing is made for alternative engines like deno and bun, where i had previously encountered that unref may not always be implemented.

fixes #15241

to not wait for this timeout to finish before the node process can exit
Copy link
Collaborator

@vkarpov15 vkarpov15 left a comment

Choose a reason for hiding this comment

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

I'm not entirely convinced this is the right behavior because unref() here undermines the primary purpose of _waitForConnect(), which is to error out if the user tries to execute operations on a connection that is never opened. For example, with this change, the following script would exit immediately with no output:

'use strict';

const mongoose = require('mongoose');

(async function() {
  await mongoose.connection.listCollections();
})();

I'm not saying we shouldn't merge this, but I would like to discuss further in #15241 to understand the use case here.

@hasezoey
Copy link
Collaborator Author

This PR can be closed once #15251 lands, or should this be kept open for something?

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.

Jest detect open handles in tests after upgrading from 8.9.6 to 8.10.0
2 participants