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(shell-api): Fix invalid regular expression error in db.currentOp() MONGOSH-1703 #2187

Merged
merged 23 commits into from
Oct 9, 2024

Conversation

gagik
Copy link
Contributor

@gagik gagik commented Oct 1, 2024

Fixes a bug where a regular expression error in db.currentOp() would cause it to error. Also adds e2e tests for currentoOp

Reproduction

Running

db.coll.insertOne({})
db.coll.find({$where: function() { sleep(20000) }}).projection({re: BSONRegExp('^(?i)\\Qchho0842\\E')})

and then concurrently in a different shell

db.currentOp()

would cause

Uncaught:
SyntaxError: Invalid regular expression: /^(?i)\Qchho0842\E/: Invalid group 

@gagik gagik changed the title MONGOSH-1703: FIx mongosh error when converting invalid regular expression in db.currentOp() fix(shell-api): Fix mongosh error when converting invalid regular expression in db.currentOp() MONGOSH-1703 Oct 1, 2024
@gagik gagik changed the title fix(shell-api): Fix mongosh error when converting invalid regular expression in db.currentOp() MONGOSH-1703 fix(shell-api): Fix error when converting invalid regular expression in db.currentOp() MONGOSH-1703 Oct 1, 2024
@gagik gagik changed the title fix(shell-api): Fix error when converting invalid regular expression in db.currentOp() MONGOSH-1703 fix(shell-api): Fix invalid regular expression error in db.currentOp() MONGOSH-1703 Oct 1, 2024
@gagik gagik marked this pull request as ready for review October 2, 2024 10:43
@gagik
Copy link
Contributor Author

gagik commented Oct 4, 2024

I realize CI isn't passing but I have trouble tracking down anything related to my change (and I know some tests have always been flakey), let me know if there's some way I can access better logs.

@gagik
Copy link
Contributor Author

gagik commented Oct 7, 2024

Remaining failing CI tests seem unrelated to the changes with exception to 1 MacOS test which should be fixed with a higher timeout 🤞

Copy link
Contributor

@kraenhansen kraenhansen left a comment

Choose a reason for hiding this comment

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

👍 Nothing blocking.

packages/e2e-tests/test/util-helpers.ts Outdated Show resolved Hide resolved
@gagik
Copy link
Contributor Author

gagik commented Oct 9, 2024

Rest of the failing tests are not related to the changes, going ahead.

@gagik gagik merged commit 28f9f52 into main Oct 9, 2024
63 of 73 checks passed
@gagik gagik deleted the gagik/fix-regex-crash branch October 9, 2024 07:04
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.

3 participants