diff --git a/core/src/graph/solver.ts b/core/src/graph/solver.ts index 709cd1c1f5..746b1c1017 100644 --- a/core/src/graph/solver.ts +++ b/core/src/graph/solver.ts @@ -133,17 +133,6 @@ export class GraphSolver extends TypedEventEmitter { results.setResult(request.task, result) - if (throwOnError && result.error) { - cleanup({ - error: new GraphResultError({ - message: `Failed to ${result.description}: ${result.error}`, - results, - wrappedErrors: [toGardenError(result.error)], - }), - }) - return - } - const missing = results.getMissing() if (missing.length > 0) { @@ -179,14 +168,20 @@ export class GraphSolver extends TypedEventEmitter { error = new GraphResultError({ message: msg, results, wrappedErrors }) } - cleanup({ error: null }) - - if (error) { - log.silly(() => `Batch ${batchId} failed: ${error.message}`) - } else { + if (!error) { log.silly(() => `Batch ${batchId} completed`) + } else { + log.silly(() => `Batch ${batchId} failed: ${error.message}`) + + if (throwOnError) { + // if throwOnError is true, we reject the promise with the error. + cleanup({ error }) + return + } } + // if throwOnError is false, we resolve the promise with the error and results. + cleanup({ error: null }) resolve({ error, results }) } diff --git a/core/test/unit/src/graph/solver.ts b/core/test/unit/src/graph/solver.ts index 82cbcf56d0..f78cc5c814 100644 --- a/core/test/unit/src/graph/solver.ts +++ b/core/test/unit/src/graph/solver.ts @@ -271,7 +271,7 @@ describe("GraphSolver", () => { expect(result!.error?.message).to.include("Throwing error in status method") }) - it("cascades an error from dependency to dependant and fails the execution", async () => { + it("cascades an error from dependency to dependant and fails the execution (2 tasks)", async () => { const taskA = makeTask({ name: "task-a" }) const taskB = makeTask({ name: "task-b", dependencies: [taskA] }) @@ -283,9 +283,11 @@ describe("GraphSolver", () => { expect(result).to.exist expect(result!.aborted).to.be.true + expect(result!.success).to.be.false + expect(result!.error).to.exist }) - it("cascades an error recursively from dependency and fails the execution", async () => { + it("cascades an error recursively from dependency and fails the execution (3 tasks)", async () => { const taskA = makeTask({ name: "task-a" }) const taskB = makeTask({ name: "task-b", dependencies: [taskA] }) const taskC = makeTask({ name: "task-c", dependencies: [taskB] }) @@ -298,6 +300,28 @@ describe("GraphSolver", () => { expect(result).to.exist expect(result!.aborted).to.be.true + expect(result!.success).to.be.false + expect(result!.error).to.exist + }) + + it("cascades an error from dependency to dependant and fails the execution with throwOnError=true", async () => { + const taskA = makeTask({ name: "task-a" }) + const taskB = makeTask({ name: "task-b", dependencies: [taskA] }) + + taskA.process = async () => { + throw new Error(`Throwing error in process method`) + } + + let error: unknown + try { + await processTask(taskB, { throwOnError: true }) + } catch (e) { + error = e + } + + expect(error).to.exist + expect(error).to.be.instanceOf(GardenError) + expect((error as GardenError).type).to.be("graph") }) it("recursively aborts unprocessed task requests when a dependency fails", async () => {