From 559f07809ac4bf0c53d6de26db8775d85a58bfda Mon Sep 17 00:00:00 2001 From: emadum Date: Tue, 24 Aug 2021 13:56:56 -0400 Subject: [PATCH 1/2] fix(NODE-3463): pass explain error through to callback --- lib/core/wireprotocol/query.js | 10 ++++++--- test/functional/explain.test.js | 38 +++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 3 deletions(-) diff --git a/lib/core/wireprotocol/query.js b/lib/core/wireprotocol/query.js index 33f858ec5b..2988ae63da 100644 --- a/lib/core/wireprotocol/query.js +++ b/lib/core/wireprotocol/query.js @@ -37,9 +37,13 @@ function query(server, ns, cmd, cursorState, options, callback) { // If we have explain, we need to rewrite the find command // to wrap it in the explain command - const explain = Explain.fromOptions(options); - if (explain) { - findCmd = decorateWithExplain(findCmd, explain); + try { + const explain = Explain.fromOptions(options); + if (explain) { + findCmd = decorateWithExplain(findCmd, explain); + } + } catch (err) { + return callback(err); } // NOTE: This actually modifies the passed in cmd, and our code _depends_ on this diff --git a/test/functional/explain.test.js b/test/functional/explain.test.js index c74c09d15a..9771773c35 100644 --- a/test/functional/explain.test.js +++ b/test/functional/explain.test.js @@ -3,6 +3,7 @@ const chai = require('chai'); const expect = chai.expect; const withClient = require('./shared').withClient; const setupDatabase = require('./shared').setupDatabase; +const MongoError = require('../../index').MongoError; describe('Explain', function() { before(function() { @@ -762,4 +763,41 @@ describe('Explain', function() { }); }) ); + + it('should throw a catchable error with invalid explain string (promise)', { + metadata: { + requires: { + mongodb: '>=3.4' + } + }, + test: withClient(function(client, done) { + const db = client.db('shouldThrowCatchableError'); + const collection = db.collection('test'); + collection + .find({ a: 1 }) + .explain('invalidExplain') + .catch(err => { + expect(err).to.exist; + expect(err).to.be.instanceOf(MongoError); + done(); + }); + }) + }); + it('should throw a catchable error with invalid explain string (callback)', { + metadata: { + requires: { + mongodb: '>=3.4' + } + }, + test: withClient(function(client, done) { + const db = client.db('shouldThrowCatchableError'); + const collection = db.collection('test'); + collection.find({ a: 1 }).explain('invalidExplain', (err, result) => { + expect(err).to.exist; + expect(result).to.not.exist; + expect(err).to.be.instanceOf(MongoError); + done(); + }); + }) + }); }); From bf931cc5125a171fc2b620cdfb1317749380f216 Mon Sep 17 00:00:00 2001 From: emadum Date: Wed, 25 Aug 2021 10:27:27 -0400 Subject: [PATCH 2/2] review feedback --- test/functional/explain.test.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/functional/explain.test.js b/test/functional/explain.test.js index 9771773c35..af3de96fbc 100644 --- a/test/functional/explain.test.js +++ b/test/functional/explain.test.js @@ -776,6 +776,7 @@ describe('Explain', function() { collection .find({ a: 1 }) .explain('invalidExplain') + .then(() => done(new Error('expected explain to fail but it succeeded'))) .catch(err => { expect(err).to.exist; expect(err).to.be.instanceOf(MongoError); @@ -783,6 +784,7 @@ describe('Explain', function() { }); }) }); + it('should throw a catchable error with invalid explain string (callback)', { metadata: { requires: {