feat: Resolve device action errors using blueprint deviceActionMessages#1743
Open
rjmunro wants to merge 2 commits into
Open
feat: Resolve device action errors using blueprint deviceActionMessages#1743rjmunro wants to merge 2 commits into
rjmunro wants to merge 2 commits into
Conversation
|
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 ignored due to path filters (2)
📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
WalkthroughBlueprints can now define per-action error message templates in ChangesDevice action error message resolution
Sequence DiagramsequenceDiagram
participant Client
participant CallPeripheralDeviceAction
participant CallPeripheralDeviceFunctionOrAction
participant ResolveActionResult
participant PeripheralDeviceRepo
participant StudioRepo
participant ResultClient
Client->>CallPeripheralDeviceAction: callPeripheralDeviceAction(deviceId, action)
CallPeripheralDeviceAction->>CallPeripheralDeviceFunctionOrAction: await callPeripheralDeviceFunctionOrAction(deviceId, action)
CallPeripheralDeviceFunctionOrAction-->>CallPeripheralDeviceAction: ActionExecutionResult
CallPeripheralDeviceAction->>ResolveActionResult: resolveActionResult(deviceId, result)
ResolveActionResult->>PeripheralDeviceRepo: fetch PeripheralDevice by deviceId
PeripheralDeviceRepo-->>ResolveActionResult: PeripheralDevice (contains studioId)
ResolveActionResult->>StudioRepo: fetch StudioBlueprintManifest by studioId
StudioRepo-->>ResolveActionResult: StudioBlueprintManifest (deviceActionMessages)
ResolveActionResult->>ResolveActionResult: interpolate/invoke template/function -> resolved response
ResolveActionResult-->>CallPeripheralDeviceAction: resolved ActionExecutionResult
CallPeripheralDeviceAction-->>ResultClient: resolved result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRsSuggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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! |
rjmunro
changed the title
rjmunro
force-pushed
the
rjmunro/error-message-customisation
branch
3 times, most recently
from
692ce19 to
f3e9c26
Compare
rjmunro
force-pushed
the
rjmunro/improve-device-error-notifications
branch
2 times, most recently
from
4825591 to
ea7c4ac
Compare
Saftret
added
the
Contribution from BBC
nytamin
approved these changes
Jun 2, 2026
… action errors server-side
rjmunro
force-pushed
the
rjmunro/improve-device-error-notifications
branch
from
ea7c4ac to
d4a6368
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/blueprints-integration/src/api/studio.ts (1)
138-138: 💤 Low valueType inconsistency with
deviceStatusMessages.
deviceActionMessagesincludes| undefinedin the value union, whereasdeviceStatusMessages(line 113) does not. The doc block only describes string templates, functions, and empty-string suppression — it doesn't mentionundefined. Adding| undefinedto aRecordvalue is redundant for optionality and just diverges from the sibling field. Consider dropping it for consistency (or documenting it intentionally).♻️ Proposed change
- deviceActionMessages?: Record<string, string | DeviceStatusMessageFunction | undefined> + deviceActionMessages?: Record<string, string | DeviceStatusMessageFunction>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/blueprints-integration/src/api/studio.ts` at line 138, The type for deviceActionMessages is inconsistent with deviceStatusMessages: remove the redundant "| undefined" from the Record value union so the property stays optional via the "?" and the values match deviceStatusMessages; update the deviceActionMessages declaration to use Record<string, string | DeviceStatusMessageFunction> and, if the inclusion of undefined was intentional, instead document that behavior in the docblock for deviceActionMessages to match the expected semantics.meteor/server/api/peripheralDevice.ts (1)
209-227: ⚖️ Poor tradeoffDuplicated device → studio → blueprint → manifest lookup.
This sequence (find device, find studio, find blueprint,
evalBlueprint) duplicates the logic inresolveDeviceStatusDetails(Lines 104-124). Consider extracting a small shared helper that returns the resolvedStudioBlueprintManifestfor a studio/device, so both message-resolution paths stay in sync.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@meteor/server/api/peripheralDevice.ts` around lines 209 - 227, Duplicate device→studio→blueprint→manifest lookup: extract a small shared helper (e.g., resolveStudioBlueprintManifestForDevice or getStudioBlueprintManifest) that encapsulates the sequence of PeripheralDevices.findOneAsync, Studios.findOneAsync, Blueprints.findOneAsync and evalBlueprint and returns the StudioBlueprintManifest (or undefined on failure); replace the inline sequence in the current code and in resolveDeviceStatusDetails with calls to this helper so both paths use the same lookup logic and return shape.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@meteor/server/api/peripheralDevice.ts`:
- Around line 246-255: The branch that returns early when resolved === null
should instead remove/suppress the TSR message so it doesn't reach the UI: in
the function resolveActionResult (the code path that calls
StatusMessageResolver.getDeviceStatusMessage and assigns
resolved/defaultMessage), replace the early return with logic that clears
result.response and any translatable response fields (e.g.
result.responseTranslated and result.translatableResponse or similarly named
properties on result) and leaves other result metadata intact; keep the existing
defaultMessage check unchanged. Ensure you only mutate the response fields (set
to empty string or undefined per local patterns) when resolved === null so the
blueprint suppression takes effect.
---
Nitpick comments:
In `@meteor/server/api/peripheralDevice.ts`:
- Around line 209-227: Duplicate device→studio→blueprint→manifest lookup:
extract a small shared helper (e.g., resolveStudioBlueprintManifestForDevice or
getStudioBlueprintManifest) that encapsulates the sequence of
PeripheralDevices.findOneAsync, Studios.findOneAsync, Blueprints.findOneAsync
and evalBlueprint and returns the StudioBlueprintManifest (or undefined on
failure); replace the inline sequence in the current code and in
resolveDeviceStatusDetails with calls to this helper so both paths use the same
lookup logic and return shape.
In `@packages/blueprints-integration/src/api/studio.ts`:
- Line 138: The type for deviceActionMessages is inconsistent with
deviceStatusMessages: remove the redundant "| undefined" from the Record value
union so the property stays optional via the "?" and the values match
deviceStatusMessages; update the deviceActionMessages declaration to use
Record<string, string | DeviceStatusMessageFunction> and, if the inclusion of
undefined was intentional, instead document that behavior in the docblock for
deviceActionMessages to match the expected semantics.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f6cb12a4-12e7-4a21-8be6-ac050517d0fe
📒 Files selected for processing (3)
meteor/server/api/client.tsmeteor/server/api/peripheralDevice.tspackages/blueprints-integration/src/api/studio.ts
Comment on lines
+246
to
+255
| if (resolved === null) { | ||
| // Message suppressed by blueprint | ||
| return result | ||
| } | ||
|
|
||
| // resolved.key is either the custom blueprint message or the defaultMessage | ||
| if (resolved.key === defaultMessage) { | ||
| // No custom message found - keep original response unchanged | ||
| return result | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect StatusMessageResolver.getDeviceStatusMessage return semantics (null vs undefined on suppression)
fd -i statusmessageresolver -e ts | head
ast-grep --pattern 'getDeviceStatusMessage($$$) {
$$$
}'
rg -nP -C4 '\bgetDeviceStatusMessage\b' --type=tsRepository: Sofie-Automation/sofie-core
Length of output: 12279
Fix suppression handling for peripheral action status messages
resolveActionResultreturnsresultunchanged whenresolved === null, so the original TSRresult.responseis still surfaced even when the blueprint intends to “suppress” via an empty-string template (generic error behavior).StatusMessageResolver.getDeviceStatusMessageis typed/documented to returnITranslatableMessage | null(null indicates suppression), so the=== nullcheck is the correct guard (no undefined/null mismatch).- Change the
resolved === nullbranch to clear/replaceresult.response(and related translatable response fields) so the suppressed TSR message can’t reach the UI.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@meteor/server/api/peripheralDevice.ts` around lines 246 - 255, The branch
that returns early when resolved === null should instead remove/suppress the TSR
message so it doesn't reach the UI: in the function resolveActionResult (the
code path that calls StatusMessageResolver.getDeviceStatusMessage and assigns
resolved/defaultMessage), replace the early return with logic that clears
result.response and any translatable response fields (e.g.
result.responseTranslated and result.translatableResponse or similarly named
properties on result) and leaves other result metadata intact; keep the existing
defaultMessage check unchanged. Ensure you only mutate the response fields (set
to empty string or undefined per local patterns) when resolved === null so the
blueprint suppression takes effect.
Updates to 10.0.0-nightly-main-20260602-133931-c0882da4d.0 which includes the new code and context fields on ActionExecutionResult (merged via Sofie-Automation/sofie-timeline-state-resolver#459).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
About the Contributor
This pull request is posted on behalf of the BBC.
Type of Contribution
Feature
Current Behavior
Device action errors (e.g. HTTP Send failures, CasparCG restart failures) are returned to the UI as raw strings from the Playout Gateway. Blueprint authors have no way to customise these messages.
New Behavior
deviceActionMessagesadded toStudioBlueprintManifest— blueprint authors can provide custom message templates (with{{variable}}interpolation) keyed by action error code stringresolveActionResult()resolves structured action errors server-side using the studio blueprint, reusing theStatusMessageResolverinfrastructure from PR feat: Status message customisation #1604executePeripheralDeviceActionso customised messages reach the UI automaticallyExample blueprint configuration:
This is the companion to PR #1604 which covered device status errors — this PR covers device action execution failures.
Testing Instructions
Requires the TSR stacked PR (on #418) for structured error codes from devices.
deviceActionMessagesin a studio blueprint with a custom message forHttpSendActionErrorCode.REQUEST_FAILED{{url}}and{{errorMessage}}) appears in the UI notificationOther Information
Stacked on PR #1604 (
feat: Status message customisation). The base branch for this PR isrjmunro/error-message-customisation.Status