Skip to content

Commit 1c580e5

Browse files
authored
fix(execution-service): surface init-time fatal errors to the websocket (#5781)
### What changes were proposed in this PR? When workflow execution initialization fails, the error was recorded into the execution metadata store but never pushed to the websocket, so connected frontend clients saw nothing — particularly for failures during `WorkflowExecutionService` construction, which happens *before* the execution is published to subscribers. `WorkflowService.initExecutionService`'s catch arm now, after `errorHandler(e)` records the fatal error, pushes a `WorkflowErrorEvent` (carrying the recorded fatal errors) to `errorSubject` — the workflow-level channel that `connect()` subscribers listen on — so init-time failures surface in the UI. | init failure | before | after | |---|---|---| | during `WorkflowExecutionService` construction (pre-publish) | logged + stored, invisible to the UI | `WorkflowErrorEvent` delivered to the frontend | | during `executeWorkflow()` | recorded; UI delivery depended on subscription timing | `WorkflowErrorEvent` delivered to the frontend | The push is extracted into a small `reportFatalErrorsToSubscribers` method so it can be unit-tested without a database (the init path itself is DB-bound). ### Any related issues, documentation, discussions? Resolves #5782. Discovered while splitting #5700 (loop operators) into smaller PRs; this fix is independent of that feature and applies to `main` on its own. ### How was this PR tested? New `WorkflowServiceSpec` (TDD, red → green): pins that `reportFatalErrorsToSubscribers` delivers a `WorkflowErrorEvent` to a `connect()` subscriber carrying exactly the fatal errors recorded in the execution state store (single error, and all errors when several are present). `sbt "WorkflowExecutionService/testOnly *WorkflowServiceSpec"` passes (2/2); scalafmt + scalafix clean. ### Was this PR authored or co-authored using generative AI tooling? Co-authored with Claude Opus 4.8 in compliance with ASF.
1 parent 0eb8427 commit 1c580e5

2 files changed

Lines changed: 118 additions & 2 deletions

File tree

amber/src/main/scala/org/apache/texera/web/service/WorkflowService.scala

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ import org.apache.texera.amber.error.ErrorUtils.{
5050
}
5151
import org.apache.texera.dao.jooq.generated.tables.pojos.User
5252
import org.apache.texera.service.util.LargeBinaryManager
53-
import org.apache.texera.web.model.websocket.event.TexeraWebSocketEvent
53+
import org.apache.texera.web.model.websocket.event.{TexeraWebSocketEvent, WorkflowErrorEvent}
5454
import org.apache.texera.web.model.websocket.request.WorkflowExecuteRequest
5555
import org.apache.texera.web.resource.dashboard.user.workflow.WorkflowExecutionsResource
5656
import org.apache.texera.web.service.WorkflowService.mkWorkflowStateId
@@ -277,6 +277,14 @@ class WorkflowService(
277277
}
278278
}
279279
}
280+
// Once the execution is published via `executionService.onNext`, the normal
281+
// state-store path surfaces fatal errors to the UI: `errorHandler` writes
282+
// them into `executionStateStore.metadataStore`, whose diff handler (set up
283+
// in the WorkflowExecutionService constructor) emits a WorkflowErrorEvent
284+
// that `connectToExecution` forwards. Before that point, neither the emitter
285+
// nor a subscriber exists yet, so a failure in the constructor itself would
286+
// be recorded but never reach the frontend -- see the fallback in `catch`.
287+
var executionPublished = false
280288
try {
281289
val execution = new WorkflowExecutionService(
282290
controllerConf,
@@ -290,13 +298,36 @@ class WorkflowService(
290298
)
291299
lifeCycleManager.registerCleanUpOnStateChange(executionStateStore)
292300
executionService.onNext(execution)
301+
executionPublished = true
293302
execution.executeWorkflow()
294303
} catch {
295-
case e: Throwable => errorHandler(e)
304+
case e: Throwable =>
305+
errorHandler(e)
306+
// If the execution was never published, no `connectToExecution`
307+
// subscriber is bound to `executionStateStore`, so the state-store path
308+
// above cannot deliver the error. Push it directly in that pre-publish
309+
// window only; once published, the state-store path already surfaces it
310+
// (pushing here too would double-emit).
311+
if (!executionPublished) {
312+
reportFatalErrorsToSubscribers(executionStateStore)
313+
}
296314
}
297315

298316
}
299317

318+
/**
319+
* Push the fatal errors currently recorded in `stateStore` to connected
320+
* websocket subscribers (via `errorSubject`).
321+
*
322+
* Fallback used only when execution initialization fails before the execution
323+
* is published (e.g. the WorkflowExecutionService constructor throws): in that
324+
* window the per-execution state store has no diff-handler emitter and no
325+
* websocket subscriber, so the error -- already recorded by `errorHandler` --
326+
* would otherwise be logged but never reach the frontend.
327+
*/
328+
private[service] def reportFatalErrorsToSubscribers(stateStore: ExecutionStateStore): Unit =
329+
errorSubject.onNext(WorkflowErrorEvent(stateStore.metadataStore.getState.fatalErrors))
330+
300331
def convertToJson(frontendVersion: String): String = {
301332
val environmentVersionMap = Map(
302333
"engine_version" -> Json.toJson(frontendVersion)
Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
package org.apache.texera.web.service
21+
22+
import com.google.protobuf.timestamp.Timestamp
23+
import org.apache.texera.amber.core.virtualidentity.WorkflowIdentity
24+
import org.apache.texera.amber.core.workflowruntimestate.FatalErrorType.EXECUTION_FAILURE
25+
import org.apache.texera.amber.core.workflowruntimestate.WorkflowFatalError
26+
import org.apache.texera.web.model.websocket.event.{TexeraWebSocketEvent, WorkflowErrorEvent}
27+
import org.apache.texera.web.storage.ExecutionStateStore
28+
import org.scalatest.flatspec.AnyFlatSpec
29+
import org.scalatest.matchers.should.Matchers
30+
31+
import java.time.Instant
32+
import scala.collection.mutable.ArrayBuffer
33+
34+
/**
35+
* Unit tests for `WorkflowService.reportFatalErrorsToSubscribers`, the seam
36+
* that surfaces init-time fatal errors to the websocket. When execution
37+
* initialization fails, the error is recorded in the metadata store; this push
38+
* is what makes it visible to connected clients instead of only logged.
39+
*/
40+
class WorkflowServiceSpec extends AnyFlatSpec with Matchers {
41+
42+
private def fatalError(message: String): WorkflowFatalError =
43+
WorkflowFatalError(EXECUTION_FAILURE, Timestamp(Instant.now), message, "", "", "")
44+
45+
/** A WorkflowService with a subscriber collecting every event it pushes. */
46+
private def serviceWithCollector(): (WorkflowService, ArrayBuffer[TexeraWebSocketEvent]) = {
47+
val service = new WorkflowService(WorkflowIdentity(1), computingUnitId = 1, cleanUpTimeout = 30)
48+
val events = ArrayBuffer.empty[TexeraWebSocketEvent]
49+
service.connect(evt => events += evt)
50+
(service, events)
51+
}
52+
53+
private def errorEventsIn(events: ArrayBuffer[TexeraWebSocketEvent]): Seq[WorkflowErrorEvent] =
54+
events.collect { case e: WorkflowErrorEvent => e }.toSeq
55+
56+
"WorkflowService" should
57+
"push a WorkflowErrorEvent carrying the store's fatal error to connected subscribers" in {
58+
val (service, events) = serviceWithCollector()
59+
val store = new ExecutionStateStore()
60+
val err = fatalError("boom during init")
61+
store.metadataStore.updateState(_.addFatalErrors(err))
62+
63+
service.reportFatalErrorsToSubscribers(store)
64+
65+
val errorEvents = errorEventsIn(events)
66+
errorEvents should have size 1
67+
// Forwards exactly the store's fatal errors -- no more, no less.
68+
errorEvents.head.fatalErrors should contain theSameElementsAs Seq(err)
69+
}
70+
71+
it should "carry every fatal error currently recorded in the store" in {
72+
val (service, events) = serviceWithCollector()
73+
val store = new ExecutionStateStore()
74+
val first = fatalError("first")
75+
val second = fatalError("second")
76+
store.metadataStore.updateState(_.addFatalErrors(first).addFatalErrors(second))
77+
78+
service.reportFatalErrorsToSubscribers(store)
79+
80+
val errorEvents = errorEventsIn(events)
81+
errorEvents should have size 1
82+
// Exactly the two recorded errors -- no extras.
83+
errorEvents.head.fatalErrors should contain theSameElementsAs Seq(first, second)
84+
}
85+
}

0 commit comments

Comments
 (0)