apache / apache/texera

Websocket runtime commands NPE instead of reporting an uninitialized execution

Open
#7,454 1 comment 0 reactions 1 assignee Claimed by @Amer-Mukhtar View on GitHub
Dominant language
Scala
Stars
314
Forks
187
Avg merge
1d 21h
Merged PRs (30d)
214

Description

### What happened?

`WorkflowWebsocketResource.myOnMsg` reads the current execution twice, and only one of the two reads is null-safe.

```scala
// line 89 — correct
val executionStateOpt = workflowStateOpt.flatMap(x => Option(x.executionService.getValue))

// line 124 — the `other` branch
workflowStateOpt.map(_.executionService.getValue) match {
case Some(value) => value.wsInput.onNext(other, uidOpt)
case None => throw new IllegalStateException("workflow execution is not initialized")
}
```

`executionService` is a `BehaviorSubject` seeded with `null`, so `getValue` returns `null` until an execution actually starts. `.map` on a `Some` wraps that into `Some(null)` — the `case None` arm is therefore unreachable, and `value.wsInput` throws a `NullPointerException` instead.

The friendly "workflow execution is not initialized" message can never be produced for the case it was written for.

### How to reproduce?

1. Open a workflow websocket session and let it subscribe to a workflow (so `workflowStateOpt` is defined).
2. Do **not** start an execution.
3. Send any runtime frame that falls through to the `other` branch — e.g. `WorkflowPauseRequest` or `WorkflowKillRequest`.

Expected: `IllegalStateException("workflow execution is not initialized")`.
Actual: `NullPointerException` on `value.wsInput`.

Both are caught by the surrounding error mapper and rethrown, so the client sees a `WorkflowFatalError` either way — but its text is an NPE stack trace rather than the intended message.

### Version/Branch

1.3.0-incubating-SNAPSHOT (main)

### Expected behavior

The `other` branch should use the same null-safe read as line 89:

```scala
workflowStateOpt.flatMap(x => Option(x.executionService.getValue)) match {
```

`executionStateOpt` is already computed on line 89 and is exactly this value, so the branch can simply reuse it.

### Additional context

Found while assessing this file for test coverage. Deliberately **not** pinned by a test, since encoding the current behaviour would cement the bug — noted here instead so a future coverage PR does not do so by accident.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.