fix some bugs in our error transformation

mongodb-js · Aug 23, 2024 · ff9203a · ff9203a
1 parent bc7190f
commit ff9203a
Show file tree

Hide file tree

Showing 5 changed files with 58 additions and 21 deletions.
diff --git a/packages/compass-import-export/src/import/import-csv.spec.ts b/packages/compass-import-export/src/import/import-csv.spec.ts
@@ -630,7 +630,12 @@ describe('importCSV', function () {
       errorCallback,
     });
 
-    expect(result.docsWritten).to.equal(1);
+    expect(omit(result, 'biggestDocSize')).to.deep.equal({
+      docsErrored: 2,
+      docsProcessed: 3,
+      docsWritten: 1,
+      hasUnboundArray: false,
+    });
 
     expect(progressCallback.callCount).to.equal(3);
     expect(errorCallback.callCount).to.equal(2);
@@ -738,33 +743,43 @@ describe('importCSV', function () {
       errorCallback,
     });
 
-    expect(result.docsWritten).to.equal(0);
+    expect(omit(result, 'biggestDocSize')).to.deep.equal({
+      docsErrored: 3,
+      docsProcessed: 3,
+      docsWritten: 0,
+      hasUnboundArray: false,
+    });
 
     expect(progressCallback.callCount).to.equal(3);
     expect(errorCallback.callCount).to.equal(3);
 
     const expectedErrors: ErrorJSON[] = [
       {
-        name: 'WriteConcernError',
+        name: 'WriteError',
         message: 'Document failed validation',
         index: 0,
         code: 121,
       },
       {
-        name: 'WriteConcernError',
+        name: 'WriteError',
         message: 'Document failed validation',
         index: 1,
         code: 121,
       },
       {
-        name: 'WriteConcernError',
+        name: 'WriteError',
         message: 'Document failed validation',
         index: 2,
         code: 121,
       },
     ];
 
     const errors = errorCallback.args.map((args) => args[0]);
+    for (const [index, error] of errors.entries()) {
+      expect(error.op).to.exist;
+      // cheat and copy them over because it is big and with buffers
+      expectedErrors[index].op = error.op;
+    }
     expect(errors).to.deep.equal(expectedErrors);
 
     const errorsText = await fs.promises.readFile(output.path, 'utf8');

diff --git a/packages/compass-import-export/src/import/import-json.spec.ts b/packages/compass-import-export/src/import/import-json.spec.ts
@@ -440,7 +440,12 @@ describe('importJSON', function () {
       errorCallback,
     });
 
-    expect(result.docsWritten).to.equal(1);
+    expect(omit(result, 'biggestDocSize')).to.deep.equal({
+      docsErrored: 1,
+      docsProcessed: 2,
+      docsWritten: 1,
+      hasUnboundArray: false,
+    });
 
     expect(progressCallback.callCount).to.equal(2);
     expect(errorCallback.callCount).to.equal(1);
@@ -522,27 +527,37 @@ describe('importJSON', function () {
       errorCallback,
     });
 
-    expect(result.docsWritten).to.equal(0);
+    expect(omit(result, 'biggestDocSize')).to.deep.equal({
+      docsErrored: 2,
+      docsProcessed: 2,
+      docsWritten: 0,
+      hasUnboundArray: false,
+    });
 
     expect(progressCallback.callCount).to.equal(2);
     expect(errorCallback.callCount).to.equal(2);
 
     const expectedErrors: ErrorJSON[] = [
       {
-        name: 'WriteConcernError',
+        name: 'WriteError',
         message: 'Document failed validation',
         index: 0,
         code: 121,
       },
       {
-        name: 'WriteConcernError',
+        name: 'WriteError',
         message: 'Document failed validation',
         index: 1,
         code: 121,
       },
     ];
 
     const errors = errorCallback.args.map((args) => args[0]);
+    for (const [index, error] of errors.entries()) {
+      expect(error.op).to.exist;
+      // cheat and copy them over because it is big and with buffers
+      expectedErrors[index].op = error.op;
+    }
     expect(errors).to.deep.equal(expectedErrors);
 
     const errorsText = await fs.promises.readFile(output.path, 'utf8');

diff --git a/packages/compass-import-export/src/import/import-utils.ts b/packages/compass-import-export/src/import/import-utils.ts
@@ -14,11 +14,12 @@ const debug = createDebug('import');
 export function makeImportResult(
   importWriter: ImportWriter,
   numProcessed: number,
+  numParseErrors: number,
   docStatsStream: DocStatsCollector,
   aborted?: boolean
 ): ImportResult {
   const result: ImportResult = {
-    docsErrored: importWriter.docsErrored,
+    docsErrored: numParseErrors + importWriter.docsErrored,
     docsWritten: importWriter.docsWritten,
     ...docStatsStream.getStats(),
     // docsProcessed is not on importWriter so that it includes docs that
@@ -134,6 +135,7 @@ export async function doImport(
   const importWriter = new ImportWriter(dataService, ns, stopOnErrors);
 
   let numProcessed = 0;
+  let numParseErrors = 0;
 
   // Stream errors just get thrown synchronously unless we listen for the event
   // on each stream we use in the pipeline. By destroying the stream we're
@@ -181,6 +183,7 @@ export async function doImport(
       try {
         doc = transformer.transform(chunk);
       } catch (err: unknown) {
+        ++numParseErrors;
         // deal with transform error
 
         // rethrow with the line number / array index appended to aid debugging
@@ -271,6 +274,7 @@ export async function doImport(
       const result = makeImportResult(
         importWriter,
         numProcessed,
+        numParseErrors,
         docStatsCollector,
         true
       );
@@ -282,6 +286,7 @@ export async function doImport(
     err.result = makeImportResult(
       importWriter,
       numProcessed,
+      numParseErrors,
       docStatsCollector
     );
 
@@ -293,6 +298,7 @@ export async function doImport(
   const result = makeImportResult(
     importWriter,
     numProcessed,
+    numParseErrors,
     docStatsCollector
   );
   debug('import:completed', result);

diff --git a/packages/compass-import-export/src/import/import-writer.spec.ts b/packages/compass-import-export/src/import/import-writer.spec.ts
diff --git a/packages/compass-import-export/src/import/import-writer.ts b/packages/compass-import-export/src/import/import-writer.ts
@@ -40,23 +40,24 @@ type ImportWriterProgressError = Error & {
   errInfo: MongoServerError['errInfo'];
 };
 
-function mongodbServerErrorToJSError({
-  index,
-  code,
-  errmsg,
-  op,
+function writeErrorToJSError({
   errInfo,
-}: Pick<MongoServerError, 'code' | 'errInfo'> &
-  Partial<
-    Pick<MongoServerError, 'index' | 'errmsg' | 'op'>
-  >): ImportWriterProgressError {
+  errmsg,
+  err,
+  code,
+  index,
+}: WriteError): ImportWriterProgressError {
+  const op = err.op;
+
   const e: ImportWriterProgressError = new Error(errmsg) as any;
   e.index = index;
   e.code = code;
   e.op = op;
   e.errInfo = errInfo;
+
   // https://www.mongodb.com/docs/manual/reference/method/BulkWriteResult/#mongodb-data-BulkWriteResult.writeErrors
-  e.name = index && op ? 'WriteError' : 'WriteConcernError';
+  e.name = index !== undefined && op ? 'WriteError' : 'WriteConcernError';
+
   return e;
 }
 
@@ -156,7 +157,7 @@ export class ImportWriter {
     const bulkOpResult = this._getBulkOpResult(bulkWriteResult);
 
     const writeErrors = (bulkWriteResult?.getWriteErrors?.() || []).map(
-      mongodbServerErrorToJSError
+      writeErrorToJSError
     );
 
     this.docsWritten += bulkOpResult.insertedCount;