feat: Resolve device action errors using blueprint deviceActionMessages#1743
Merged
rjmunro merged 7 commits intoJun 19, 2026
Merged
Conversation
|
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:
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 PeripheralDevices
participant Studios
participant resolveMessage as StatusMessageResolver
Client->>callPeripheralDeviceAction: callPeripheralDeviceAction(deviceId, action)
callPeripheralDeviceAction->>callPeripheralDeviceFunctionOrAction: await call
callPeripheralDeviceFunctionOrAction-->>callPeripheralDeviceAction: ActionExecutionResult
callPeripheralDeviceAction->>resolveActionResult: resolveActionResult(deviceId, result)
alt result is Ok or code missing
resolveActionResult-->>callPeripheralDeviceAction: return result unchanged
else result has error code
resolveActionResult->>PeripheralDevices: findOne(deviceId)
PeripheralDevices-->>resolveActionResult: device with studioId
resolveActionResult->>Studios: findOne and evaluate blueprint
Studios-->>resolveActionResult: StudioBlueprintManifest
resolveActionResult->>resolveMessage: resolve deviceActionMessages[code]
resolveMessage-->>resolveActionResult: resolved key or null
alt message suppressed
resolveActionResult-->>callPeripheralDeviceAction: result with response.key = ''
else message resolved
resolveActionResult-->>callPeripheralDeviceAction: result with interpolated response.key
else no match or error
resolveActionResult-->>callPeripheralDeviceAction: return result unchanged
end
end
callPeripheralDeviceAction-->>Client: resolved ActionExecutionResult
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested 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
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
… action errors server-side
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).
rjmunro
force-pushed
the
rjmunro/improve-device-error-notifications
branch
from
38283b2 to
62e2966
Compare
…essage - Return an empty response key when deviceActionMessages resolves to null - Update resolveActionResult docstring to document suppression behaviour
- Share studio-to-blueprint manifest lookup between status and action resolution - Reduces duplicated PeripheralDevices/Studios/Blueprints query chain
…iceStatusMessages - Remove redundant undefined union member from the Record value type - Suppression is expressed via empty string, consistent with deviceStatusMessages
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)
meteor/server/api/peripheralDevice.ts (1)
222-229:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHandle child-device studio fallback before returning unchanged results.
At Line 226,
resolveActionResultreturns early whenstudioAndConfigIdis absent. In the same file,setStatusalready handles child devices via parent lookup (Line 414-Line 420). Without the same fallback here, blueprintdeviceActionMessageswon’t apply to child-device actions.Suggested fix
- const device = (await PeripheralDevices.findOneAsync(deviceId, { - projection: { name: 1, studioAndConfigId: 1 }, - })) as Pick<PeripheralDevice, 'name' | 'studioAndConfigId'> | undefined + const device = (await PeripheralDevices.findOneAsync(deviceId, { + projection: { name: 1, studioAndConfigId: 1, parentDeviceId: 1 }, + })) as Pick<PeripheralDevice, 'name' | 'studioAndConfigId' | 'parentDeviceId'> | undefined - if (!device?.studioAndConfigId?.studioId) return result + let studioId = device?.studioAndConfigId?.studioId + if (!studioId && device?.parentDeviceId) { + const parentDevice = (await PeripheralDevices.findOneAsync(device.parentDeviceId, { + projection: { studioAndConfigId: 1 }, + })) as Pick<PeripheralDevice, 'studioAndConfigId'> | undefined + studioId = parentDevice?.studioAndConfigId?.studioId + } + if (!studioId) return result - const studioBlueprint = await getStudioBlueprintManifest(device.studioAndConfigId.studioId) + const studioBlueprint = await getStudioBlueprintManifest(studioId)🤖 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 222 - 229, The resolveActionResult function returns early when studioAndConfigId is absent, but it does not handle child devices the way setStatus does with its parent device lookup logic at lines 414-420. Before returning an unchanged result when studioAndConfigId is missing, add fallback logic to look up the parent device (similar to the setStatus parent lookup pattern) to retrieve the studio ID for child devices, ensuring that blueprint deviceActionMessages will be properly applied to child-device actions instead of being skipped.
🧹 Nitpick comments (1)
meteor/server/api/__tests__/peripheralDevice.resolveActionResult.test.ts (1)
160-170: ⚡ Quick winAdd a regression test for child-device studio resolution.
Please add a case where the first
PeripheralDevices.findOneAsyncreturns a child device (nostudioAndConfigId, withparentDeviceId) and the parent lookup provides the studio. That locks in the intended action-message resolution behavior once the fallback is implemented.Suggested test shape
+ it('resolves messages for child devices via parent studio', async () => { + const parentId = protectString<PeripheralDeviceId>('parent0') + jest + .spyOn(PeripheralDevices, 'findOneAsync') + .mockResolvedValueOnce({ + name: 'Child Device', + parentDeviceId: parentId, + } as any) + .mockResolvedValueOnce({ + name: 'Playout Gateway', + studioAndConfigId: { studioId, configId: 'config0' }, + } as any) + jest.spyOn(Studios, 'findOneAsync').mockResolvedValue({ blueprintId: 'blueprint0' } as any) + jest.spyOn(Blueprints, 'findOneAsync').mockResolvedValue({ + _id: 'blueprint0', + name: 'test', + code: '', + } as any) + mockEvalBlueprint.mockReturnValue({ + deviceActionMessages: { + [ACTION_ERROR_CODE]: 'Failed to trigger graphics at {{url}}: {{errorMessage}}', + }, + } as any) + + const resolved = await resolveActionResult(deviceId, makeErrorResult()) + expect(resolved.response).toEqual({ + key: 'Failed to trigger graphics at http://graphics/api: connection refused', + }) + })🤖 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/__tests__/peripheralDevice.resolveActionResult.test.ts` around lines 160 - 170, Add a new regression test after the existing test case that verifies studio resolution for child devices. The new test should mock PeripheralDevices.findOneAsync to return a child device object containing a parentDeviceId but no studioAndConfigId, then mock a subsequent PeripheralDevices.findOneAsync call (for parent lookup) to return a parent device with a studioAndConfigId. Call resolveActionResult with a test result and verify that the resolved result properly incorporates the studio information from the parent device, ensuring the fallback behavior is locked in for child-device studio resolution.
🤖 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.
Outside diff comments:
In `@meteor/server/api/peripheralDevice.ts`:
- Around line 222-229: The resolveActionResult function returns early when
studioAndConfigId is absent, but it does not handle child devices the way
setStatus does with its parent device lookup logic at lines 414-420. Before
returning an unchanged result when studioAndConfigId is missing, add fallback
logic to look up the parent device (similar to the setStatus parent lookup
pattern) to retrieve the studio ID for child devices, ensuring that blueprint
deviceActionMessages will be properly applied to child-device actions instead of
being skipped.
---
Nitpick comments:
In `@meteor/server/api/__tests__/peripheralDevice.resolveActionResult.test.ts`:
- Around line 160-170: Add a new regression test after the existing test case
that verifies studio resolution for child devices. The new test should mock
PeripheralDevices.findOneAsync to return a child device object containing a
parentDeviceId but no studioAndConfigId, then mock a subsequent
PeripheralDevices.findOneAsync call (for parent lookup) to return a parent
device with a studioAndConfigId. Call resolveActionResult with a test result and
verify that the resolved result properly incorporates the studio information
from the parent device, ensuring the fallback behavior is locked in for
child-device studio resolution.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d0c6e333-2227-41cf-8a4b-e9da95bafe8e
📒 Files selected for processing (3)
meteor/server/api/__tests__/peripheralDevice.resolveActionResult.test.tsmeteor/server/api/peripheralDevice.tspackages/blueprints-integration/src/api/studio.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/blueprints-integration/src/api/studio.ts
- Cover success, template interpolation, function messages, and suppression - Cover missing blueprint, missing studio, and unmatched error codes
rjmunro
force-pushed
the
rjmunro/improve-device-error-notifications
branch
from
b8cf11c to
fbabd9c
Compare
- Fall back to parent device studioAndConfigId when the target device has none - Matches the existing setStatus lookup pattern for subdevices like casparcg0
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 notificationStatus