feat: handle adlib action validation errors#1643
Conversation
imaretic
added
the
Contribution from EVS
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughCapture and inspect the return value from Changes
Sequence Diagram(s)sequenceDiagram
participant JobWorker as JobWorker (adlibAction)
participant Blueprint as Blueprint (executeAction)
participant Corelib as Corelib (state persistence / downstream)
JobWorker->>Blueprint: executeAction(actionPayload)
Blueprint-->>JobWorker: return { validationErrors?: any } or void
alt validationErrors present
JobWorker->>JobWorker: throw UserError (ValidationFailed, 409)
JobWorker-->>Client: respond 409 ValidationFailed
else no validationErrors
JobWorker->>Corelib: persist state / notify downstream / apply side-effects
Corelib-->>JobWorker: acknowledgment / continue flow
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/job-worker/src/playout/adlibAction.ts (1)
246-260:⚠️ Potential issue | 🔴 CriticalFix the TypeScript compilation error:
executeActionreturnsvoid, so accessing.validationErrorsis invalid.The blueprint's
executeActionmethod returnsPromise<void>(per the blueprint interface inpackages/blueprints-integration/src/api/showStyle.ts), makingresultof typevoid. Checkingresult && result.validationErrorstriggers TS1345: "An expression of type 'void' cannot be tested for truthiness." ThevalidationErrorsproperty does not exist in any return type—it's not part ofExecuteActionResult.Remove the validation error check or verify where validation errors are actually surfaced in the action execution flow. If validation errors should be captured, they may be available through the
actionContext(as withactionContext.noteson line 275) or require a change to the blueprint interface itself.
imaretic
force-pushed
the
contribute/log-adlib-action-validation-errors
branch
from
cc9fdee to
e0135c3
Compare
imaretic
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/job-worker/src/playout/adlibAction.ts (1)
286-289:⚠️ Potential issue | 🟡 Minor
validationErrorsfrom blueprint result is not propagated in the return value.
ExecuteActionResultdefines an optionalvalidationErrorsfield, but it is never populated in the return statement (lines 286-289). Instead, validation errors from the blueprint are immediately thrown as exceptions at line 256-260. No caller in the codebase readsvalidationErrorsfrom the result—they only check thetakenfield. Either remove the unused field fromExecuteActionResultor populate it to allow callers to handle validation errors without catching exceptions.
🤖 Fix all issues with AI agents
In `@packages/job-worker/src/playout/adlibAction.ts`:
- Around line 256-265: The validation failure check for the AdLib Action should
be moved out of the try/catch so that the UserError thrown by the validation is
not caught and logged as a blueprint execution error; hoist the variable that
receives the blueprint result (e.g., declare let result; before the try),
perform the call that populates result inside the try, then immediately after
the try do the existing if (result && result.validationErrors) { throw
UserError.from(...) } check; ensure the catch still only handles genuine
execution errors from showStyleBlueprint.executeAction so logs remain accurate.
🧹 Nitpick comments (1)
packages/corelib/src/worker/studio.ts (1)
288-288: Consider using a more specific type thananyforvalidationErrors.Both here and in the
ShowStyleBlueprintManifest.executeActionreturn type,validationErrorsis typed asany. A lightweight structured type (e.g.,Array<{ path?: string; message: string }>or at leastunknown) would improve type safety and make the contract clearer for blueprint implementors and downstream consumers.That said,
userDataon line 271 also usesany, so this is consistent with existing conventions in this file.
imaretic
force-pushed
the
contribute/log-adlib-action-validation-errors
branch
from
bd7f1b1 to
7cad592
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/corelib/src/worker/studio.ts (1)
285-289: Consider using a more specific type thananyforvalidationErrors.
anybypasses all type checking, making it easy for consumers to misinterpret the shape of validation errors. At minimum,unknownwould force callers to narrow the type before use. Ideally, define a concrete type (e.g.,Array<{ path: string; message: string }>) that both the blueprint and the worker agree on, so the contract is self-documenting.This also applies to the matching
{ validationErrors: any }inshowStyle.ts(line 145).
2 participants
About the Contributor
This PR is posted on behalf of EVS Broadcast Equipment.
Type of Contribution
This is a: Feature
Current Behavior
There is no way for the
executeAdlibfunction to gracefully return payload validation errors and display them to the user if the payload sent by the user is invalid.New Behavior
The interface of the
executeActionfunction has been extended, and it can now return{ validationErrors }orvoid.If
executeActionreturns any validation errors, an error will be thrown, informing the user about the validation issues.Testing
Affected areas
Interface of
executeActionin blueprints-integration package.Status