fix(solver): throw if a dependency failed and throwOnError is true

Co-authored-by: Haukur Páll Hallvarðsson <[email protected]>
garden-io · Oct 24, 2024 · da1e067 · da1e067
1 parent e7eeedc
commit da1e067
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 18 deletions.
diff --git a/core/src/graph/solver.ts b/core/src/graph/solver.ts
@@ -133,17 +133,6 @@ export class GraphSolver extends TypedEventEmitter<SolverEvents> {
 
           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<SolverEvents> {
             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
@@ -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 () => {