fix(medusa): Batch job not saving the errors properly (#2812)

.changeset/red-squids-compare.md

@@ -0,0 +1,5 @@
+---
+"@medusajs/medusa": patch
+---
+
+fix: Batch job not saving the errors properly

integration-tests/api/__tests__/batch-jobs/product/import.js

@@ -9,7 +9,9 @@ const adminSeeder = require("../../../helpers/admin-seeder")
 const batchJobSeeder = require("../../../helpers/batch-job-seeder")
 const userSeeder = require("../../../helpers/user-seeder")
 const { simpleProductFactory } = require("../../../factories")
-const { simpleProductCollectionFactory } = require("../../../factories/simple-product-collection-factory");
+const {
+  simpleProductCollectionFactory,
+} = require("../../../factories/simple-product-collection-factory")
 
 const adminReqConfig = {
   headers: {
@@ -50,8 +52,8 @@ describe("Product import batch job", () => {
   let medusaProcess
   let dbConnection
 
-  let collectionHandle1 = "test-collection1"
-  let collectionHandle2 = "test-collection2"
+  const collectionHandle1 = "test-collection1"
+  const collectionHandle2 = "test-collection2"
 
   beforeAll(async () => {
     const cwd = path.resolve(path.join(__dirname, "..", "..", ".."))
@@ -79,11 +81,14 @@ describe("Product import batch job", () => {
     await batchJobSeeder(dbConnection)
     await adminSeeder(dbConnection)
     await userSeeder(dbConnection)
-    await simpleProductCollectionFactory(dbConnection, [{
-      handle: collectionHandle1
-    }, {
-      handle: collectionHandle2
-    }])
+     await simpleProductCollectionFactory(dbConnection, [
+      {
+        handle: collectionHandle1,
+      },
+      {
+        handle: collectionHandle2,
+      },
+    ])
   })
 
   afterEach(async () => {
@@ -226,7 +231,7 @@ describe("Product import batch job", () => {
           ],
           collection: expect.objectContaining({
             handle: collectionHandle1,
-          })
+          }),
         }),
         expect.objectContaining({
           title: "Test product",
@@ -288,7 +293,7 @@ describe("Product import batch job", () => {
           tags: [],
           collection: expect.objectContaining({
             handle: collectionHandle1,
-          })
+          }),
         }),
         // UPDATED PRODUCT
         expect.objectContaining({
@@ -384,8 +389,8 @@ describe("Product import batch job", () => {
             }),
           ],
           collection: expect.objectContaining({
-            handle: collectionHandle2
-          })
+            handle: collectionHandle2,
+          }),
         }),
       ])
     )

packages/medusa/src/interfaces/batch-job-strategy.ts

@@ -69,6 +69,7 @@ export abstract class AbstractBatchJobStrategy
     err: unknown,
     result: T
   ): Promise<void> {
+    // TODO just throw to be handled by the subscriber
     return await this.atomicPhase_(async (transactionManager) => {
       const batchJob = (await this.batchJobService_
         .withTransaction(transactionManager)
@@ -95,7 +96,7 @@ export abstract class AbstractBatchJobStrategy
             },
             result: {
               ...result,
-              errors: [...existingErrors, resultError],
+              errors: [...existingErrors, resultError.message],
             },
           })
       } else {

packages/medusa/src/models/batch-job.ts

@@ -1,16 +1,5 @@
-import {
-  AfterLoad,
-  BeforeInsert,
-  Column,
-  Entity,
-  JoinColumn,
-  ManyToOne,
-} from "typeorm"
-import {
-  BatchJobResultError,
-  BatchJobResultStatDescriptor,
-  BatchJobStatus,
-} from "../types/batch-job"
+import { AfterLoad, BeforeInsert, Column, Entity, JoinColumn, ManyToOne, } from "typeorm"
+import { BatchJobResultError, BatchJobResultStatDescriptor, BatchJobStatus, } from "../types/batch-job"
 import { DbAwareColumn, resolveDbType } from "../utils/db-aware-column"
 
 import { SoftDeletableEntity } from "../interfaces/models/soft-deletable-entity"
@@ -37,7 +26,7 @@ export class BatchJob extends SoftDeletableEntity {
     count?: number
     advancement_count?: number
     progress?: number
-    errors?: BatchJobResultError[]
+    errors?: (BatchJobResultError | string)[]
     stat_descriptors?: BatchJobResultStatDescriptor[]
     file_key?: string
     file_size?: number

packages/medusa/src/services/batch-job.ts

@@ -96,6 +96,7 @@ class BatchJobService extends TransactionBaseService {
     eventBusService,
     strategyResolverService,
   }: InjectedDependencies) {
+    // eslint-disable-next-line prefer-rest-params
     super(arguments[0])
 
     this.manager_ = manager
@@ -339,7 +340,7 @@ class BatchJobService extends TransactionBaseService {
 
   async setFailed(
     batchJobOrId: string | BatchJob,
-    error?: BatchJobResultError
+    error?: BatchJobResultError | string
   ): Promise<BatchJob | never> {
     return await this.atomicPhase_(async () => {
       let batchJob = batchJobOrId as BatchJob

packages/medusa/src/subscribers/batch-job.ts

@@ -33,45 +33,44 @@ class BatchJobSubscriber {
   }
 
   preProcessBatchJob = async (data): Promise<void> => {
-    await this.manager_.transaction(async (manager) => {
-      const batchJobServiceTx = this.batchJobService_.withTransaction(manager)
-      const batchJob = await batchJobServiceTx.retrieve(data.id)
+    try {
+      await this.manager_.transaction(async (manager) => {
+        const batchJobServiceTx = this.batchJobService_.withTransaction(manager)
+        const batchJob = await batchJobServiceTx.retrieve(data.id)
 
-      const batchJobStrategy = this.strategyResolver_.resolveBatchJobByType(
-        batchJob.type
-      )
+        const batchJobStrategy = this.strategyResolver_.resolveBatchJobByType(
+          batchJob.type
+        )
 
-      try {
         await batchJobStrategy
           .withTransaction(manager)
           .preProcessBatchJob(batchJob.id)
         await batchJobServiceTx.setPreProcessingDone(batchJob.id)
-      } catch (e) {
-        await this.batchJobService_.setFailed(batchJob.id, e.message)
-        throw e
-      }
-    })
+      })
+    } catch (e) {
+      await this.batchJobService_.setFailed(data.id, e.message)
+      throw e
+    }
   }
 
   processBatchJob = async (data): Promise<void> => {
-    await this.manager_.transaction(async (manager) => {
-      const batchJobServiceTx = this.batchJobService_.withTransaction(manager)
-      const batchJob = await batchJobServiceTx.retrieve(data.id)
+    try {
+      await this.manager_.transaction(async (manager) => {
+        const batchJobServiceTx = this.batchJobService_.withTransaction(manager)
+        const batchJob = await batchJobServiceTx.retrieve(data.id)
 
-      const batchJobStrategy = this.strategyResolver_.resolveBatchJobByType(
-        batchJob.type
-      )
+        const batchJobStrategy = this.strategyResolver_.resolveBatchJobByType(
+          batchJob.type
+        )
 
-      await batchJobServiceTx.setProcessing(batchJob.id)
-
-      try {
+        await batchJobServiceTx.setProcessing(batchJob.id)
         await batchJobStrategy.withTransaction(manager).processJob(batchJob.id)
         await batchJobServiceTx.complete(batchJob.id)
-      } catch (e) {
-        await this.batchJobService_.setFailed(batchJob.id, e.message)
-        throw e
-      }
-    })
+      })
+    } catch (e) {
+      await this.batchJobService_.setFailed(data.id, e.message)
+      throw e
+    }
   }
 }