diff --git a/src/client-side-encryption/client_encryption.ts b/src/client-side-encryption/client_encryption.ts index 6d2ff784ee6..44b2411bb94 100644 --- a/src/client-side-encryption/client_encryption.ts +++ b/src/client-side-encryption/client_encryption.ts @@ -719,8 +719,8 @@ export class ClientEncryption { }); const context = this._mongoCrypt.makeExplicitEncryptionContext(valueBuffer, contextOptions); - const result = deserialize(await stateMachine.execute(this, context)); - return result.v; + const { v } = deserialize(await stateMachine.execute(this, context)); + return v; } } diff --git a/src/client-side-encryption/state_machine.ts b/src/client-side-encryption/state_machine.ts index 94df0c728d3..8cd0e1fb5f7 100644 --- a/src/client-side-encryption/state_machine.ts +++ b/src/client-side-encryption/state_machine.ts @@ -112,8 +112,25 @@ export type CSFLEKMSTlsOptions = { azure?: ClientEncryptionTlsOptions; }; -/** `{ v: [] }` */ -const EMPTY_V = Uint8Array.from([13, 0, 0, 0, 4, 118, 0, 5, 0, 0, 0, 0, 0]); +/** + * This is kind of a hack. For `rewrapManyDataKey`, we have tests that + * guarantee that when there are no matching keys, `rewrapManyDataKey` returns + * nothing. We also have tests for auto encryption that guarantee for `encrypt` + * we return an error when there are no matching keys. This error is generated in + * subsequent iterations of the state machine. + * Some apis (`encrypt`) throw if there are no filter matches and others (`rewrapManyDataKey`) + * do not. We set the result manually here, and let the state machine continue. `libmongocrypt` + * will inform us if we need to error by setting the state to `MONGOCRYPT_CTX_ERROR` but + * otherwise we'll return `{ v: [] }`. + */ +const EMPTY_V = Uint8Array.from([ + ...[13, 0, 0, 0], // document size = 13 bytes + ...[ + ...[4, 118, 0], // array type (4), "v\x00" basic latin "v" + ...[5, 0, 0, 0, 0] // empty document (5 byte size, null terminator) + ], + 0 // null terminator +]); /** * @internal @@ -211,15 +228,7 @@ export class StateMachine { const keys = await this.fetchKeys(keyVaultClient, keyVaultNamespace, filter); if (keys.length === 0) { - // This is kind of a hack. For `rewrapManyDataKey`, we have tests that - // guarantee that when there are no matching keys, `rewrapManyDataKey` returns - // nothing. We also have tests for auto encryption that guarantee for `encrypt` - // we return an error when there are no matching keys. This error is generated in - // subsequent iterations of the state machine. - // Some apis (`encrypt`) throw if there are no filter matches and others (`rewrapManyDataKey`) - // do not. We set the result manually here, and let the state machine continue. `libmongocrypt` - // will inform us if we need to error by setting the state to `MONGOCRYPT_CTX_ERROR` but - // otherwise we'll return `{ v: [] }`. + // See docs on EMPTY_V result = EMPTY_V; } for await (const key of keys) { diff --git a/src/cmap/wire_protocol/responses.ts b/src/cmap/wire_protocol/responses.ts index fd32aa56790..cf65885e034 100644 --- a/src/cmap/wire_protocol/responses.ts +++ b/src/cmap/wire_protocol/responses.ts @@ -66,6 +66,14 @@ export type MongoDBResponseConstructor = { /** @internal */ export class MongoDBResponse extends OnDemandDocument { + /** + * Devtools need to know which keys were encrypted before the driver automatically decrypted them. + * If decorating is enabled (`Symbol.for('@@mdb.decorateDecryptionResult')`), this field will be set, + * storing the original encrypted response from the server, so that we can build an object that has + * the list of BSON keys that were encrypted stored at a well known symbol: `Symbol.for('@@mdb.decryptedKeys')`. + */ + encryptedResponse?: MongoDBResponse; + static is(value: unknown): value is MongoDBResponse { return value instanceof MongoDBResponse; } @@ -161,13 +169,11 @@ export class MongoDBResponse extends OnDemandDocument { } return { utf8: { writeErrors: false } }; } - - // TODO: Supports decorating result - encryptedResponse?: MongoDBResponse; } // Here's a little blast from the past. // OLD style method definition so that I can override get without redefining ALL the fancy TS :/ +// TODO there must be a better way... Object.defineProperty(MongoDBResponse.prototype, 'get', { value: function get(name: any, as: any, required: any) { try { diff --git a/src/cursor/abstract_cursor.ts b/src/cursor/abstract_cursor.ts index 826594a55dc..8faf23649a1 100644 --- a/src/cursor/abstract_cursor.ts +++ b/src/cursor/abstract_cursor.ts @@ -294,8 +294,7 @@ export abstract class AbstractCursor< return bufferedDocs; } - - private async *asyncIterator() { + async *[Symbol.asyncIterator](): AsyncGenerator { if (this.closed) { return; } @@ -343,10 +342,6 @@ export abstract class AbstractCursor< } } - async *[Symbol.asyncIterator](): AsyncGenerator { - yield* this.asyncIterator(); - } - stream(options?: CursorStreamOptions): Readable & AsyncIterable { if (options?.transform) { const transform = options.transform; diff --git a/src/error.ts b/src/error.ts index 8ed8958d1e2..066cc1f82a7 100644 --- a/src/error.ts +++ b/src/error.ts @@ -1177,17 +1177,16 @@ export class MongoWriteConcernError extends MongoServerError { * * @public **/ - constructor( - result: { - writeConcernError: { - code: number; - errmsg: string; - codeName?: string; - errInfo?: Document; - }; - } & Document - ) { - super(result.writeConcernError); + constructor(result: { + writeConcernError: { + code: number; + errmsg: string; + codeName?: string; + errInfo?: Document; + }; + errorLabels?: string[]; + }) { + super({ ...result, ...result.writeConcernError }); this.errInfo = result.writeConcernError.errInfo; this.result = result; } diff --git a/src/operations/bulk_write.ts b/src/operations/bulk_write.ts index 76dba810ee2..64c0664b5a1 100644 --- a/src/operations/bulk_write.ts +++ b/src/operations/bulk_write.ts @@ -7,7 +7,6 @@ import type { import type { Collection } from '../collection'; import type { Server } from '../sdam/server'; import type { ClientSession } from '../sessions'; -import { throwIfWriteConcernError } from '../utils'; import { AbstractOperation, Aspect, defineAspects } from './operation'; /** @internal */ @@ -51,9 +50,7 @@ export class BulkWriteOperation extends AbstractOperation { } // Execute the bulk - const result = await bulk.execute({ ...options, session }); - throwIfWriteConcernError(result); - return result; + return await bulk.execute({ ...options, session }); } } diff --git a/src/operations/delete.ts b/src/operations/delete.ts index a601b7a5a43..f0ef61cb7b1 100644 --- a/src/operations/delete.ts +++ b/src/operations/delete.ts @@ -4,8 +4,8 @@ import { MongoCompatibilityError, MongoServerError } from '../error'; import { type TODO_NODE_3286 } from '../mongo_types'; import type { Server } from '../sdam/server'; import type { ClientSession } from '../sessions'; -import { type MongoDBNamespace, throwIfWriteConcernError } from '../utils'; -import type { WriteConcernOptions } from '../write_concern'; +import { type MongoDBNamespace } from '../utils'; +import { type WriteConcernOptions } from '../write_concern'; import { type CollationOptions, CommandOperation, type CommandOperationOptions } from './command'; import { Aspect, defineAspects, type Hint } from './operation'; @@ -96,7 +96,6 @@ export class DeleteOperation extends CommandOperation { } const res: TODO_NODE_3286 = await super.executeCommand(server, session, command); - throwIfWriteConcernError(res); return res; } } diff --git a/src/operations/find_and_modify.ts b/src/operations/find_and_modify.ts index de107f4835b..c295fcb0a88 100644 --- a/src/operations/find_and_modify.ts +++ b/src/operations/find_and_modify.ts @@ -5,13 +5,8 @@ import { ReadPreference } from '../read_preference'; import type { Server } from '../sdam/server'; import type { ClientSession } from '../sessions'; import { formatSort, type Sort, type SortForCmd } from '../sort'; -import { - decorateWithCollation, - hasAtomicOperators, - maxWireVersion, - throwIfWriteConcernError -} from '../utils'; -import type { WriteConcern, WriteConcernSettings } from '../write_concern'; +import { decorateWithCollation, hasAtomicOperators, maxWireVersion } from '../utils'; +import { type WriteConcern, type WriteConcernSettings } from '../write_concern'; import { CommandOperation, type CommandOperationOptions } from './command'; import { Aspect, defineAspects } from './operation'; @@ -219,7 +214,6 @@ export class FindAndModifyOperation extends CommandOperation { // Execute the command const result = await super.executeCommand(server, session, cmd); - throwIfWriteConcernError(result); return options.includeResultMetadata ? result : result.value ?? null; } } diff --git a/src/operations/update.ts b/src/operations/update.ts index 85fbe0808ad..ba0ad6d95ff 100644 --- a/src/operations/update.ts +++ b/src/operations/update.ts @@ -4,7 +4,7 @@ import { MongoCompatibilityError, MongoInvalidArgumentError, MongoServerError } import type { InferIdType, TODO_NODE_3286 } from '../mongo_types'; import type { Server } from '../sdam/server'; import type { ClientSession } from '../sessions'; -import { hasAtomicOperators, type MongoDBNamespace, throwIfWriteConcernError } from '../utils'; +import { hasAtomicOperators, type MongoDBNamespace } from '../utils'; import { type CollationOptions, CommandOperation, type CommandOperationOptions } from './command'; import { Aspect, defineAspects, type Hint } from './operation'; @@ -123,7 +123,6 @@ export class UpdateOperation extends CommandOperation { } const res = await super.executeCommand(server, session, command); - throwIfWriteConcernError(res); return res; } } diff --git a/src/sdam/server.ts b/src/sdam/server.ts index d0de87cedff..59a7231b4f4 100644 --- a/src/sdam/server.ts +++ b/src/sdam/server.ts @@ -46,9 +46,9 @@ import { makeStateMachine, maxWireVersion, type MongoDBNamespace, - supportsRetryableWrites, - throwIfWriteConcernError + supportsRetryableWrites } from '../utils'; +import { throwIfWriteConcernError } from '../write_concern'; import { type ClusterTime, STATE_CLOSED, @@ -337,7 +337,9 @@ export class Server extends TypedEventEmitter { ) { await this.pool.reauthenticate(conn); try { - return await conn.command(ns, cmd, finalOptions, responseType); + const res = await conn.command(ns, cmd, finalOptions, responseType); + throwIfWriteConcernError(res); + return res; } catch (commandError) { throw this.decorateCommandError(conn, cmd, finalOptions, commandError); } diff --git a/src/utils.ts b/src/utils.ts index 120af227380..cebb81e0a0d 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -11,7 +11,6 @@ import { promisify } from 'util'; import { deserialize, type Document, ObjectId, resolveBSONOptions } from './bson'; import type { Connection } from './cmap/connection'; import { MAX_SUPPORTED_WIRE_VERSION } from './cmap/wire_protocol/constants'; -import { MongoDBResponse } from './cmap/wire_protocol/responses'; import type { Collection } from './collection'; import { kDecoratedKeys, LEGACY_HELLO_COMMAND } from './constants'; import type { AbstractCursor } from './cursor/abstract_cursor'; @@ -24,8 +23,7 @@ import { MongoNetworkTimeoutError, MongoNotConnectedError, MongoParseError, - MongoRuntimeError, - MongoWriteConcernError + MongoRuntimeError } from './error'; import type { Explain } from './explain'; import type { MongoClient } from './mongo_client'; @@ -1418,19 +1416,3 @@ export function decorateDecryptionResult( decorateDecryptionResult(decrypted[k], originalValue, false); } } - -/** Called with either a plain object or MongoDBResponse */ -export function throwIfWriteConcernError(response: unknown): void { - if (typeof response === 'object' && response != null) { - const writeConcernError: object | null = - MongoDBResponse.is(response) && response.has('writeConcernError') - ? response.toObject() - : !MongoDBResponse.is(response) && 'writeConcernError' in response - ? response - : null; - - if (writeConcernError != null) { - throw new MongoWriteConcernError(writeConcernError as any); - } - } -} diff --git a/src/write_concern.ts b/src/write_concern.ts index e3a3f5510e3..b315798fd43 100644 --- a/src/write_concern.ts +++ b/src/write_concern.ts @@ -1,4 +1,6 @@ import { type Document } from './bson'; +import { MongoDBResponse } from './cmap/wire_protocol/responses'; +import { MongoWriteConcernError } from './error'; /** @public */ export type W = number | 'majority'; @@ -159,3 +161,19 @@ export class WriteConcern { return undefined; } } + +/** Called with either a plain object or MongoDBResponse */ +export function throwIfWriteConcernError(response: unknown): void { + if (typeof response === 'object' && response != null) { + const writeConcernError: object | null = + MongoDBResponse.is(response) && response.has('writeConcernError') + ? response.toObject() + : !MongoDBResponse.is(response) && 'writeConcernError' in response + ? response + : null; + + if (writeConcernError != null) { + throw new MongoWriteConcernError(writeConcernError as any); + } + } +} diff --git a/test/integration/retryable-writes/non-server-retryable_writes.test.ts b/test/integration/retryable-writes/non-server-retryable_writes.test.ts index 8a4190615c2..40513134d5d 100644 --- a/test/integration/retryable-writes/non-server-retryable_writes.test.ts +++ b/test/integration/retryable-writes/non-server-retryable_writes.test.ts @@ -34,13 +34,14 @@ describe('Non Server Retryable Writes', function () { async () => { const serverCommandStub = sinon.stub(Server.prototype, 'command'); serverCommandStub.onCall(0).rejects(new PoolClearedError('error')); - serverCommandStub - .onCall(1) - .returns( - Promise.reject( - new MongoWriteConcernError({ errorLabels: ['NoWritesPerformed'], errorCode: 10107 }, {}) - ) - ); + serverCommandStub.onCall(1).returns( + Promise.reject( + new MongoWriteConcernError({ + errorLabels: ['NoWritesPerformed'], + writeConcernError: { errmsg: 'NotWritablePrimary error', errorCode: 10107 } + }) + ) + ); const insertResult = await collection.insertOne({ _id: 1 }).catch(error => error); sinon.restore(); diff --git a/test/integration/retryable-writes/retryable_writes.spec.prose.test.ts b/test/integration/retryable-writes/retryable_writes.spec.prose.test.ts index a5a53e7cf7f..d02540526a8 100644 --- a/test/integration/retryable-writes/retryable_writes.spec.prose.test.ts +++ b/test/integration/retryable-writes/retryable_writes.spec.prose.test.ts @@ -277,23 +277,22 @@ describe('Retryable Writes Spec Prose', () => { { requires: { topology: 'replicaset', mongodb: '>=4.2.9' } }, async () => { const serverCommandStub = sinon.stub(Server.prototype, 'command'); - serverCommandStub - .onCall(0) - .returns( - Promise.reject( - new MongoWriteConcernError({ errorLabels: ['RetryableWriteError'], code: 91 }, {}) - ) - ); - serverCommandStub - .onCall(1) - .returns( - Promise.reject( - new MongoWriteConcernError( - { errorLabels: ['RetryableWriteError', 'NoWritesPerformed'], errorCode: 10107 }, - {} - ) - ) - ); + serverCommandStub.onCall(0).returns( + Promise.reject( + new MongoWriteConcernError({ + errorLabels: ['RetryableWriteError'], + writeConcernError: { errmsg: 'ShutdownInProgress error', code: 91 } + }) + ) + ); + serverCommandStub.onCall(1).returns( + Promise.reject( + new MongoWriteConcernError({ + errorLabels: ['RetryableWriteError', 'NoWritesPerformed'], + writeConcernError: { errmsg: 'NotWritablePrimary error', errorCode: 10107 } + }) + ) + ); const insertResult = await collection.insertOne({ _id: 1 }).catch(error => error); sinon.restore();