Yicong-Huang opened a new issue, #5211:
URL: https://github.com/apache/texera/issues/5211
### 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:
```scala
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:
```scala
ExecutionsMetadataPersistService.tryGetExistingExecution(ExecutionIdentity(-1L))
// returns Some(null), not None
```
Version: 1.1.0-incubating.
### Suggested direction
Wrap in `Option(...)`:
```scala
Option(workflowExecutionsDao.fetchOneByEid(executionId.id.toInt))
```
(also re-evaluate whether the catch-all still adds value once the miss case
is handled correctly).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
