wip

Author: nbbeeken

src/cmap/connection.ts

@@ -20,8 +20,7 @@ import {
   MongoNetworkTimeoutError,
   MongoParseError,
   MongoServerError,
-  MongoUnexpectedServerResponseError,
-  MongoWriteConcernError
+  MongoUnexpectedServerResponseError
 } from '../error';
 import type { ServerApi, SupportedNodeConnectionOptions } from '../mongo_client';
 import { type MongoClientAuthProviders } from '../mongo_client_auth_providers';
@@ -510,7 +509,7 @@ export class Connection extends TypedEventEmitter<ConnectionEvents> {
           this.emit(Connection.CLUSTER_TIME_RECEIVED, document.$clusterTime);
         }
 
-        if (document.isError) {
+        if (document.ok === 0) {
           throw new MongoServerError((object ??= document.toObject(bsonOptions)));
         }
 
@@ -573,16 +572,8 @@ export class Connection extends TypedEventEmitter<ConnectionEvents> {
   ): Promise<Document> {
     this.throwIfAborted();
     for await (const document of this.sendCommand(ns, command, options, responseType)) {
-      if (
-        (MongoDBResponse.is(document) && document.has('writeConcernError')) ||
-        (!MongoDBResponse.is(document) && document.writeConcernError)
-      ) {
-        const object = MongoDBResponse.is(document) ? document.toObject(options) : document;
-        throw new MongoWriteConcernError(object.writeConcernError, object);
-      }
       return document;
     }
-
     throw new MongoUnexpectedServerResponseError('Unable to get response from server');
   }
 

src/cmap/wire_protocol/responses.ts

@@ -81,15 +81,6 @@ export class MongoDBResponse extends OnDemandDocument {
   // {ok:1}
   static empty = new MongoDBResponse(new Uint8Array([13, 0, 0, 0, 16, 111, 107, 0, 1, 0, 0, 0, 0]));
 
-  /** Indicates this document is a server error */
-  public get isError() {
-    let isError = this.ok === 0;
-    isError ||= this.has('errmsg');
-    isError ||= this.has('code');
-    isError ||= this.has('$err'); // The '$err' field is used in OP_REPLY responses
-    return isError;
-  }
-
   /**
    * Drivers can safely assume that the `recoveryToken` field is always a BSON document but drivers MUST NOT modify the
    * contents of the document.
@@ -120,6 +111,7 @@ export class MongoDBResponse extends OnDemandDocument {
     return this.get('operationTime', BSONType.timestamp);
   }
 
+  /** Normalizes whatever BSON value is "ok" to a JS number 1 or 0. */
   public get ok(): 0 | 1 {
     return this.getNumber('ok') ? 1 : 0;
   }
@@ -174,7 +166,7 @@ export class MongoDBResponse extends OnDemandDocument {
   encryptedResponse?: MongoDBResponse;
 }
 
-// Here's a litle blast from the past.
+// Here's a little blast from the past.
 // OLD style method definition so that I can override get without redefining ALL the fancy TS :/
 Object.defineProperty(MongoDBResponse.prototype, 'get', {
   value: function get(name: any, as: any, required: any) {
@@ -210,7 +202,11 @@ export class CursorResponse extends MongoDBResponse {
   }
 
   public get id(): Long {
-    return Long.fromBigInt(this.cursor.get('id', BSONType.long, true));
+    try {
+      return Long.fromBigInt(this.cursor.get('id', BSONType.long, true));
+    } catch (cause) {
+      throw new MongoUnexpectedServerResponseError(cause.message, { cause });
+    }
   }
 
   public get ns() {

test/integration/collection-management/collection.test.ts

@@ -458,6 +458,13 @@ describe('Collection', function () {
       expect(countC).to.be.a('number').that.equals(0);
     });
 
+    it('does not mutate options', async () => {
+      const options = Object.freeze(Object.create(null));
+      const count = await collection.countDocuments({}, options);
+      expect(count).to.be.a('number').that.equals(100);
+      expect(options).to.deep.equal({});
+    });
+
     context('when a filter is applied', () => {
       it('adds a $match pipeline', async () => {
         await collection.countDocuments({ test: 'a' });

test/unit/assorted/sessions_collection.test.js

@@ -48,27 +48,5 @@ describe('Sessions - unit/sessions', function () {
           return client.close();
         });
     });
-
-    it('does not mutate command options', function () {
-      const options = Object.freeze({});
-      test.server.setMessageHandler(request => {
-        const doc = request.document;
-        if (isHello(doc)) {
-          request.reply(mock.HELLO);
-        } else if (doc.count || doc.aggregate || doc.endSessions) {
-          request.reply({ ok: 1 });
-        }
-      });
-
-      const client = new MongoClient(`mongodb://${test.server.uri()}/test`);
-      return client.connect().then(client => {
-        const coll = client.db('foo').collection('bar');
-
-        return coll.countDocuments({}, options).then(() => {
-          expect(options).to.deep.equal({});
-          return client.close();
-        });
-      });
-    });
   });
 });

test/unit/client-side-encryption/auto_encrypter.test.ts

@@ -162,7 +162,7 @@ describe('AutoEncrypter', function () {
           local: { key: Buffer.alloc(96) }
         }
       });
-      const decrypted = await mc.decrypt(input);
+      const decrypted = BSON.deserialize(await mc.decrypt(input));
       expect(decrypted).to.eql({ filter: { find: 'test', ssn: '457-55-5462' } });
       expect(decrypted).to.not.have.property(Symbol.for('@@mdb.decryptedKeys'));
       expect(decrypted.filter).to.not.have.property(Symbol.for('@@mdb.decryptedKeys'));
@@ -186,7 +186,7 @@ describe('AutoEncrypter', function () {
         }
       });
       mc[Symbol.for('@@mdb.decorateDecryptionResult')] = true;
-      let decrypted = await mc.decrypt(input);
+      let decrypted = BSON.deserialize(await mc.decrypt(input));
       expect(decrypted).to.eql({ filter: { find: 'test', ssn: '457-55-5462' } });
       expect(decrypted).to.not.have.property(Symbol.for('@@mdb.decryptedKeys'));
       expect(decrypted.filter[Symbol.for('@@mdb.decryptedKeys')]).to.eql(['ssn']);
@@ -243,7 +243,7 @@ describe('AutoEncrypter', function () {
             aws: {}
           }
         });
-        const decrypted = await mc.decrypt(input);
+        const decrypted = BSON.deserialize(await mc.decrypt(input));
         expect(decrypted).to.eql({ filter: { find: 'test', ssn: '457-55-5462' } });
       });
     });

test/unit/cmap/wire_protocol/responses.test.ts

@@ -86,30 +86,58 @@ describe('class MongoDBResponse', () => {
 });
 
 describe('class CursorResponse', () => {
-  describe('constructor()', () => {
+  describe('get cursor()', () => {
     it('throws if input does not contain cursor embedded document', () => {
-      expect(() => new CursorResponse(BSON.serialize({ ok: 1 }))).to.throw(BSONError);
+      // @ts-expect-error: testing private getter
+      expect(() => new CursorResponse(BSON.serialize({ ok: 1 })).cursor).to.throw(
+        MongoUnexpectedServerResponseError
+      );
     });
+  });
 
+  describe('get id()', () => {
     it('throws if input does not contain cursor.id int64', () => {
-      expect(() => new CursorResponse(BSON.serialize({ ok: 1, cursor: {} }))).to.throw(BSONError);
+      expect(() => new CursorResponse(BSON.serialize({ ok: 1, cursor: {} })).id).to.throw(
+        MongoUnexpectedServerResponseError
+      );
+    });
+  });
+
+  describe('get batch()', () => {
+    it('throws if input does not contain firstBatch nor nextBatch', () => {
+      expect(
+        // @ts-expect-error: testing private getter
+        () => new CursorResponse(BSON.serialize({ ok: 1, cursor: { id: 0n, batch: [] } })).batch
+      ).to.throw(MongoUnexpectedServerResponseError);
     });
+  });
 
+  describe('get ns()', () => {
     it('sets namespace to null if input does not contain cursor.ns', () => {
       expect(new CursorResponse(BSON.serialize({ ok: 1, cursor: { id: 0n, firstBatch: [] } })).ns)
         .to.be.null;
     });
+  });
 
-    it('throws if input does not contain firstBatch nor nextBatch', () => {
-      expect(
-        () => new CursorResponse(BSON.serialize({ ok: 1, cursor: { id: 0n, batch: [] } }))
-      ).to.throw(MongoUnexpectedServerResponseError);
+  describe('get batchSize()', () => {
+    it('reports the returned batch size', () => {
+      const response = new CursorResponse(
+        BSON.serialize({ ok: 1, cursor: { id: 0n, nextBatch: [{}, {}, {}] } })
+      );
+      expect(response.batchSize).to.equal(3);
+      expect(response.shift()).to.deep.equal({});
+      expect(response.batchSize).to.equal(3);
     });
+  });
 
-    it('reports a length equal to the batch', () => {
-      expect(
-        new CursorResponse(BSON.serialize({ ok: 1, cursor: { id: 0n, nextBatch: [1, 2, 3] } }))
-      ).to.have.lengthOf(3);
+  describe('get length()', () => {
+    it('reports number of documents remaining in the batch', () => {
+      const response = new CursorResponse(
+        BSON.serialize({ ok: 1, cursor: { id: 0n, nextBatch: [{}, {}, {}] } })
+      );
+      expect(response).to.have.lengthOf(3);
+      expect(response.shift()).to.deep.equal({});
+      expect(response).to.have.lengthOf(2); // length makes CursorResponse act like an array
     });
   });
 
@@ -162,12 +190,4 @@ describe('class CursorResponse', () => {
       expect(response.shift()).to.be.null;
     });
   });
-
-  describe('pushMany()', () =>
-    it('throws unsupported error', () =>
-      expect(CursorResponse.prototype.pushMany).to.throw(/Unsupported/i)));
-
-  describe('push()', () =>
-    it('throws unsupported error', () =>
-      expect(CursorResponse.prototype.push).to.throw(/Unsupported/i)));
 });

test/unit/error.test.ts

@@ -301,7 +301,7 @@ describe('MongoErrors', () => {
     };
 
     const RAW_USER_WRITE_CONCERN_ERROR = {
-      ok: 0,
+      ok: 1,
       errmsg: 'waiting for replication timed out',
       code: 64,
       codeName: 'WriteConcernFailed',
@@ -316,7 +316,7 @@ describe('MongoErrors', () => {
     };
 
     const RAW_USER_WRITE_CONCERN_ERROR_INFO = {
-      ok: 0,
+      ok: 1,
       errmsg: 'waiting for replication timed out',
       code: 64,
       codeName: 'WriteConcernFailed',
@@ -367,7 +367,7 @@ describe('MongoErrors', () => {
       replSet.connect();
     }
 
-    it('should expose a user command writeConcern error like a normal WriteConcernError', function () {
+    it.only('should expose a user command writeConcern error like a normal WriteConcernError', function () {
       test.primaryServer.setMessageHandler(request => {
         const doc = request.document;
         if (isHello(doc)) {