Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WX-757 Fix workflow stuck in aborting after WDL type error #7385

Merged
merged 14 commits into from
Mar 14, 2024
Merged
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"MinimalStructExample.test": "Hello World",
"MinimalStructExample.integer": 2
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
version 1.0


struct firstLayer {
String first
Int number
}

struct secondLayer {
String second
firstLayer lowerLayer
}


workflow MinimalStructExample {
input {
String test
Int integer
}

firstLayer example1 = {"first": test, "number": integer}
secondLayer example2 = {"second": test, "lowerLayer": example1}

output {
String example3 = example2.second
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
name: nested_struct_bad_instantiation
testFormat: workflowfailure

files {
workflow: failures/nested_struct_bad_instantiation/nested_struct_bad_instantiation.wdl
inputs: failures/nested_struct_bad_instantiation/nested_struct_bad_instantiation.inputs
}

metadata {
workflowName: MinimalStructExample
status: Failed
"failures.0.message": "Workflow failed"
"failures.0.causedBy.0.message": "Failed to evaluate 'example2' (reason 1 of 1): Evaluating { \"second\": test, \"lowerLayer\": example1 } failed: Cannot construct WomMapType(WomStringType,WomAnyType) with mixed types in map values: [WomString(Hello World), WomObject(Map(first -> WomString(Hello World), number -> WomInteger(2)),WomCompositeType(Map(first -> WomStringType, number -> WomIntegerType),Some(firstLayer)))]"
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import common.validation.Validation._
import cromwell.core.ExecutionIndex._
import cromwell.core.{ExecutionStatus, JobKey}
import cromwell.engine.workflow.lifecycle.execution.WorkflowExecutionDiff
import cromwell.engine.workflow.lifecycle.execution.{WdlRuntimeException, WorkflowExecutionDiff}
import cromwell.engine.workflow.lifecycle.execution.keys.ExpressionKey.{
ExpressionEvaluationFailedResponse,
ExpressionEvaluationSucceededResponse
Expand All @@ -32,7 +32,7 @@
.contextualizeErrors(s"evaluate '${node.fullyQualifiedName}'") match {
case Right(result) => workflowExecutionActor ! ExpressionEvaluationSucceededResponse(this, result)
case Left(f) =>
workflowExecutionActor ! ExpressionEvaluationFailedResponse(this, new RuntimeException(f.toList.mkString(", ")))
workflowExecutionActor ! ExpressionEvaluationFailedResponse(this, WdlRuntimeException(f.toList.mkString(", ")))

Check warning on line 35 in engine/src/main/scala/cromwell/engine/workflow/lifecycle/execution/keys/ExpressionKey.scala

View check run for this annotation

Codecov / codecov/patch

engine/src/main/scala/cromwell/engine/workflow/lifecycle/execution/keys/ExpressionKey.scala#L35

Added line #L35 was not covered by tests
}

WorkflowExecutionDiff(Map(this -> ExecutionStatus.Running)).validNel
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,16 @@
import cromwell.core.CallKey
import wom.values.WomEvaluatedCallInputs

import scala.util.control.NoStackTrace

final case class JobRunning(key: CallKey, inputs: WomEvaluatedCallInputs)
final case class JobStarting(callKey: CallKey)

/**
* An exception specific to conditions inside the executing WDL, as opposed to one that is "Cromwell's fault"
* @param message Description suitable for user display
*/
final case class WdlRuntimeException(message: String) extends RuntimeException with NoStackTrace {
override def getMessage: String = message

Check warning on line 18 in engine/src/main/scala/cromwell/engine/workflow/lifecycle/execution/package.scala

View check run for this annotation

Codecov / codecov/patch

engine/src/main/scala/cromwell/engine/workflow/lifecycle/execution/package.scala#L18

Added line #L18 was not covered by tests
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import cats.syntax.traverse._
import cats.syntax.validated._
import cats.instances.list._
import common.validation.ErrorOr
import common.validation.ErrorOr.ErrorOr
import wdl.model.draft3.elements.ExpressionElement
import wdl.model.draft3.elements.ExpressionElement._
Expand Down Expand Up @@ -84,11 +85,14 @@
) mapN { (key, value) => key -> value }
}

evaluated map { kvps =>
// Expose `flatten` to handle the `ErrorOr[ErrorOr[]]` [WX-757]
import common.validation.ErrorOr.NestedErrorOr

(evaluated map { kvps =>

Check warning on line 91 in wdl/transforms/new-base/src/main/scala/wdl/transforms/base/linking/expression/values/LiteralEvaluators.scala

View check run for this annotation

Codecov / codecov/patch

wdl/transforms/new-base/src/main/scala/wdl/transforms/base/linking/expression/values/LiteralEvaluators.scala#L91

Added line #L91 was not covered by tests
val value = kvps.map(entry => entry._1.value -> entry._2.value).toMap
val sideEffectFiles = kvps.flatMap(entry => entry._1.sideEffectFiles ++ entry._2.sideEffectFiles)
EvaluatedValue(WomMap(value.toMap), sideEffectFiles)
}
ErrorOr(EvaluatedValue(WomMap.apply(value.toMap), sideEffectFiles))
}).flatten

Check warning on line 95 in wdl/transforms/new-base/src/main/scala/wdl/transforms/base/linking/expression/values/LiteralEvaluators.scala

View check run for this annotation

Codecov / codecov/patch

wdl/transforms/new-base/src/main/scala/wdl/transforms/base/linking/expression/values/LiteralEvaluators.scala#L94-L95

Added lines #L94 - L95 were not covered by tests
}
}

Expand Down
Loading