Copilot commented on code in PR #5922:
URL: https://github.com/apache/texera/pull/5922#discussion_r3464361319
##########
amber/src/main/scala/org/apache/texera/web/service/WorkflowService.scala:
##########
@@ -277,14 +275,12 @@ class WorkflowService(
}
}
}
- // Once the execution is published via `executionService.onNext`, the
normal
- // state-store path surfaces fatal errors to the UI: `errorHandler` writes
- // them into `executionStateStore.metadataStore`, whose diff handler (set
up
- // in the WorkflowExecutionService constructor) emits a WorkflowErrorEvent
- // that `connectToExecution` forwards. Before that point, neither the
emitter
- // nor a subscriber exists yet, so a failure in the constructor itself
would
- // be recorded but never reach the frontend -- see the fallback in `catch`.
- var executionPublished = false
+ // WorkflowExecutionService construction does no external work and cannot
+ // throw; it registers its error/state diff handler up front. Once
published
+ // via `executionService.onNext`, any failure in `executeWorkflow()` is
+ // recorded by `errorHandler` into the metadata store, whose handler emits
a
+ // WorkflowErrorEvent that `connectToExecution` forwards -- a single
+ // reporting site, no separate pre-publish fallback needed.
Review Comment:
This refactor removes the pre-publish fallback and now relies on
`WorkflowExecutionService` construction staying non-throwing + registering the
metadata-store diff handler before any fatal error can be recorded. With
`WorkflowServiceSpec` deleted, there’s no longer a unit/regression test
guarding this invariant, so a future constructor change could silently
reintroduce the “recorded but never surfaced to websocket” failure mode.
Consider adding a regression test that (a) triggers an exception before/at
`executeWorkflow()` and asserts a `WorkflowErrorEvent` is delivered via the
normal `connectToExecution` path, and/or (b) asserts the constructor performs
no external work and cannot throw under valid inputs.
--
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]
