diff --git a/.changeset/angry-avocados-shave.md b/.changeset/angry-avocados-shave.md new file mode 100644 index 0000000000..8a6c3ecef8 --- /dev/null +++ b/.changeset/angry-avocados-shave.md @@ -0,0 +1,5 @@ +--- +"@medusajs/orchestration": patch +--- + +chore(orchestration): improve transaction error handling diff --git a/packages/core/orchestration/src/__tests__/transaction/transaction-orchestrator.ts b/packages/core/orchestration/src/__tests__/transaction/transaction-orchestrator.ts index 48b1ca5911..2c1ace528b 100644 --- a/packages/core/orchestration/src/__tests__/transaction/transaction-orchestrator.ts +++ b/packages/core/orchestration/src/__tests__/transaction/transaction-orchestrator.ts @@ -1,4 +1,8 @@ -import { TransactionStepState, TransactionStepStatus } from "@medusajs/utils" +import { + MedusaError, + TransactionStepState, + TransactionStepStatus, +} from "@medusajs/utils" import { setTimeout } from "timers/promises" import { DistributedTransaction, @@ -690,6 +694,136 @@ describe("Transaction Orchestrator", () => { expect(transaction.getState()).toBe(TransactionState.FAILED) }) + it("Should handle multiple types of errors", async () => { + const errorTypes = [ + new Error("Regular error object"), + new MedusaError(MedusaError.Types.NOT_FOUND, "Not found error"), + { message: "Custom error object" }, + "String error", + 123, + {}, + null, + [1, 2, "b"], + new Date(), + ] + async function handler( + actionId: string, + functionHandlerType: TransactionHandlerType, + payload: TransactionPayload + ) { + const command = { + [actionId]: { + [TransactionHandlerType.INVOKE]: () => { + throw errorTypes[+actionId.slice(-1) - 1] + }, + }, + } + + return command[actionId][functionHandlerType](payload) + } + + const flow: TransactionStepsDefinition = { + next: Array.from({ length: errorTypes.length }, (_, i) => ({ + action: `a${i + 1}`, + maxRetries: 0, + noCompensation: true, + })), + } + + const strategy = new TransactionOrchestrator({ + id: "transaction-name", + definition: flow, + }) + + const transaction = await strategy.beginTransaction({ + transactionId: "transaction_id_123", + handler, + }) + + await strategy.resume(transaction) + + expect(transaction.getErrors()).toEqual([ + { + action: "a1", + handlerType: "invoke", + error: { + message: "Regular error object", + name: "Error", + stack: expect.stringContaining("transaction-name -> a1 (invoke)"), + }, + }, + { + action: "a2", + handlerType: "invoke", + error: { + message: "Not found error", + name: "Error", + stack: expect.stringContaining("transaction-name -> a2 (invoke)"), + type: "not_found", + __isMedusaError: true, + code: undefined, + date: expect.any(Date), + }, + }, + { + action: "a3", + handlerType: "invoke", + error: { + message: "Custom error object", + stack: expect.stringContaining("transaction-name -> a3 (invoke)"), + }, + }, + { + action: "a4", + handlerType: "invoke", + error: { + message: '"String error"', + stack: expect.stringContaining("transaction-name -> a4 (invoke)"), + }, + }, + { + action: "a5", + handlerType: "invoke", + error: { + message: "123", + stack: expect.stringContaining("transaction-name -> a5 (invoke)"), + }, + }, + { + action: "a6", + handlerType: "invoke", + error: { + message: "{}", + stack: expect.stringContaining("transaction-name -> a6 (invoke)"), + }, + }, + { + action: "a7", + handlerType: "invoke", + error: { + message: "null", + stack: expect.stringContaining("transaction-name -> a7 (invoke)"), + }, + }, + { + action: "a8", + handlerType: "invoke", + error: { + message: '[1,2,"b"]', + stack: expect.stringContaining("transaction-name -> a8 (invoke)"), + }, + }, + { + action: "a9", + handlerType: "invoke", + error: { + message: expect.any(String), + stack: expect.stringContaining("transaction-name -> a9 (invoke)"), + }, + }, + ]) + }) + it("Should complete a transaction if a failing step has the flag 'continueOnPermanentFailure' set to true", async () => { const mocks = { one: jest.fn().mockImplementation((payload) => { diff --git a/packages/core/orchestration/src/transaction/transaction-orchestrator.ts b/packages/core/orchestration/src/transaction/transaction-orchestrator.ts index 9b46be99a1..26cd8aec32 100644 --- a/packages/core/orchestration/src/transaction/transaction-orchestrator.ts +++ b/packages/core/orchestration/src/transaction/transaction-orchestrator.ts @@ -686,6 +686,20 @@ export class TransactionOrchestrator extends EventEmitter { if (isErrorLike(error)) { error = serializeError(error) + } else { + try { + if (error?.message) { + error = JSON.parse(JSON.stringify(error)) + } else { + error = { + message: JSON.stringify(error), + } + } + } catch (e) { + error = { + message: "Unknown non-serializable error", + } + } } if ( @@ -712,15 +726,15 @@ export class TransactionOrchestrator extends EventEmitter { ? TransactionHandlerType.COMPENSATE : TransactionHandlerType.INVOKE - if (error?.stack) { - const workflowId = transaction.modelId - const stepAction = step.definition.action - const sourcePath = transaction.getFlow().metadata?.sourcePath - const sourceStack = sourcePath - ? `\n⮑ \sat ${sourcePath}: [${workflowId} -> ${stepAction} (${TransactionHandlerType.INVOKE})]` - : `\n⮑ \sat [${workflowId} -> ${stepAction} (${TransactionHandlerType.INVOKE})]` - error.stack += sourceStack - } + error.stack ??= "" + + const workflowId = transaction.modelId + const stepAction = step.definition.action + const sourcePath = transaction.getFlow().metadata?.sourcePath + const sourceStack = sourcePath + ? `\n⮑ \sat ${sourcePath}: [${workflowId} -> ${stepAction} (${TransactionHandlerType.INVOKE})]` + : `\n⮑ \sat [${workflowId} -> ${stepAction} (${TransactionHandlerType.INVOKE})]` + error.stack += sourceStack transaction.addError(step.definition.action!, handlerType, error) } diff --git a/packages/modules/workflow-engine-redis/integration-tests/__tests__/index.spec.ts b/packages/modules/workflow-engine-redis/integration-tests/__tests__/index.spec.ts index f8c264c13f..221a244ec9 100644 --- a/packages/modules/workflow-engine-redis/integration-tests/__tests__/index.spec.ts +++ b/packages/modules/workflow-engine-redis/integration-tests/__tests__/index.spec.ts @@ -550,7 +550,14 @@ moduleIntegrationTestRunner({ return e }) - expect(setStepError).toEqual({ uhuuuu: "yeaah!" }) + expect(setStepError).toEqual( + expect.objectContaining({ + message: JSON.stringify({ + uhuuuu: "yeaah!", + }), + stack: expect.any(String), + }) + ) ;({ data: executionsList } = await query.graph({ entity: "workflow_executions", fields: ["id", "state", "context"],