Skip to content

tryGetExistingExecution returns Some(null) instead of None for missing eid #5211

@Yicong-Huang

Description

@Yicong-Huang

Bug Description

org.apache.texera.web.service.ExecutionsMetadataPersistService.tryGetExistingExecution (amber/src/main/scala/org/apache/texera/web/service/ExecutionsMetadataPersistService.scala:78) breaks the Option[T] contract for the lookup-miss case:

def tryGetExistingExecution(executionId: ExecutionIdentity): Option[WorkflowExecutions] = {
  try {
    Some(workflowExecutionsDao.fetchOneByEid(executionId.id.toInt))
  } catch {
    case t: Throwable =>
      logger.info("Unable to get execution. Error = " + t.getMessage)
      None
  }
}

fetchOneByEid returns null when no row matches; that null is wrapped in Some(...) and returned. The catch only triggers on hard errors (e.g. a closed connection). Callers using getOrElse will get the null through and NPE later.

Reproduction

amber/src/test/scala/org/apache/texera/web/service/ExecutionsMetadataPersistServiceSpec.scala (introduced in the linked PR) pins this:

ExecutionsMetadataPersistService.tryGetExistingExecution(ExecutionIdentity(-1L))
// returns Some(null), not None

Version: 1.1.0-incubating.

Suggested direction

Wrap in Option(...):

Option(workflowExecutionsDao.fetchOneByEid(executionId.id.toInt))

(also re-evaluate whether the catch-all still adds value once the miss case is handled correctly).

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No fields configured for Bug.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions