feat: add structured error codes and context to ActionExecutionResult#459
Merged
rjmunro merged 4 commits intoJun 2, 2026
Merged
Conversation
|
Warning Review limit reached
More reviews will be available in 45 minutes and 23 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThis change unifies device action error handling by introducing structured error codes, human-readable message templates, and error interpolation contexts for both CasparCG and HTTP Send devices. Device action implementations are updated to return these structured errors in all major failure scenarios. ChangesStructured error support for CasparCG and HTTP Send device actions
Sequence Diagram(s)sequenceDiagram
participant Caller
participant clearAllChannels
participant restartCasparCG
participant listMedia
Caller->>clearAllChannels: invoke
alt No active connection
clearAllChannels->>Caller: { result: Error, code: CLEAR_NO_CONNECTION, response }
else Commands.Info fails
clearAllChannels->>Caller: { result: Error, code: CLEAR_FAILED, response }
else No channel data
clearAllChannels->>Caller: { result: Error, code: CLEAR_FAILED, response }
end
Caller->>restartCasparCG: invoke
alt Missing initOptions
restartCasparCG->>Caller: { result: Error, code: RESTART_NOT_INITIALIZED, response }
else Missing launcher setting
restartCasparCG->>Caller: { result: Error, code: RESTART_MISSING_LAUNCHER_SETTINGS, response }
else Bad launcher reply
restartCasparCG->>Caller: { result: Error, code: RESTART_BAD_REPLY, context: { statusCode, body }, response }
else Launcher request fails
restartCasparCG->>Caller: { result: Error, code: RESTART_REQUEST_FAILED, context: { errorMessage }, response }
end
Caller->>listMedia: invoke
alt Device not initialized
listMedia->>Caller: { result: Error, code: LIST_NOT_INITIALIZED, response }
else CLS command fails
listMedia->>Caller: { result: Error, code: LIST_CLS_ERROR, context: { errorMessage }, response }
else Bad server response
listMedia->>Caller: { result: Error, code: LIST_BAD_RESPONSE, context: { responseCode }, response }
end
sequenceDiagram
participant Caller
participant executeSendCommandAction
participant sendCommandWithResult
Caller->>executeSendCommandAction: invoke with command
alt Invalid payload/url/type/params
executeSendCommandAction->>Caller: { result: Error, code: (PAYLOAD_INVALID/URL_INVALID/TYPE_INVALID/PARAMS_INVALID/PARAMSTYPE_INVALID), context, response }
else Valid, send request
executeSendCommandAction->>sendCommandWithResult: send()
alt HTTP request success
sendCommandWithResult->>Caller: { result: Ok }
else HTTP request fails
sendCommandWithResult->>Caller: { result: Error, code: REQUEST_FAILED, context: { url, errorMessage[, errorCode] }, response }
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/timeline-state-resolver/src/integrations/casparCG/index.ts (1)
744-764:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix
RESTART_BAD_REPLYreachability for non-2xx responses
Withgotv11 defaultingthrowHttpErrors: true, 3xx/4xx/5xx responses reject before this.then, so theelsebranch only runs for 2xx responses wherestatusCode !== 200(e.g., 201/204). IfRESTART_BAD_REPLYshould cover non-2xx as well, setthrowHttpErrors: false(or mapgot.HTTPErrorin.catch).Proposed fix
return got .post(url, { timeout: { request: 5000, // Arbitary, long enough for realistic scenarios }, + throwHttpErrors: false, })🤖 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/timeline-state-resolver/src/integrations/casparCG/index.ts` around lines 744 - 764, The current got.post(...) call relies on a .then(...) to map non-200 replies to CasparCGActionErrorCode.RESTART_BAD_REPLY, but got v11 throws on non-2xx by default so those responses never reach the .then; either disable throwing by adding throwHttpErrors: false to the options object passed to got.post, or keep the default and add a .catch that detects got.HTTPError and maps its response (statusCode and body) to the same error shape (using ActionExecutionResultCode.Error and CasparCGActionErrorCode.RESTART_BAD_REPLY) so that non-2xx replies are handled consistently instead of being swallowed by the rejected promise.
🤖 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 `@packages/timeline-state-resolver-types/src/integrations/casparcg.ts`:
- Around line 46-49: The docstring currently refers to the obsolete config key
deviceActionMessages; update the comment to use the correct blueprint config key
deviceActionErrorMessages everywhere in this file (e.g., in the top-level
comment describing "Action error codes for CasparCG device actions") so the
documentation matches the PR and related docs and avoid any mismatches for
CasparCG integration consumers.
In `@packages/timeline-state-resolver/src/integrations/casparCG/index.ts`:
- Around line 645-657: The two early error returns in clearAllChannels
(returning ActionExecutionResultCode.Error with
CasparCGActionErrorCode.CLEAR_INFO_FAILED and
CasparCGActionErrorCode.CLEAR_NO_CHANNELS) omit the legacy fallback "response"
field; update those returned objects to include the same fallback response value
used elsewhere (e.g., the variable response or its fallback message) so callers
relying on the legacy response still receive it—modify the return statements in
clearAllChannels that reference CLEAR_INFO_FAILED and CLEAR_NO_CHANNELS to
include a response property mirroring the successful-path response fallback.
In `@packages/timeline-state-resolver/src/integrations/httpSend/index.ts`:
- Around line 117-123: The validation mistakenly checks cmd.type instead of
cmd.paramsType; update the guard in the HTTP send handler so it verifies that
cmd.paramsType exists and is a valid key in TimelineContentTypeHTTPParamType
(i.e., replace the check "!(cmd.type in TimelineContentTypeHTTPParamType)" with
a check against cmd.paramsType) so the error return (using
ActionExecutionResultCode.Error and HttpSendActionErrorCode.INVALID_PARAMS_TYPE)
only fires for truly invalid paramsType values.
---
Outside diff comments:
In `@packages/timeline-state-resolver/src/integrations/casparCG/index.ts`:
- Around line 744-764: The current got.post(...) call relies on a .then(...) to
map non-200 replies to CasparCGActionErrorCode.RESTART_BAD_REPLY, but got v11
throws on non-2xx by default so those responses never reach the .then; either
disable throwing by adding throwHttpErrors: false to the options object passed
to got.post, or keep the default and add a .catch that detects got.HTTPError and
maps its response (statusCode and body) to the same error shape (using
ActionExecutionResultCode.Error and CasparCGActionErrorCode.RESTART_BAD_REPLY)
so that non-2xx replies are handled consistently instead of being swallowed by
the rejected promise.
🪄 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: eb315daa-3c7f-4122-91a4-64c08559d0b1
⛔ Files ignored due to path filters (3)
packages/timeline-state-resolver-types/src/__tests__/__snapshots__/index.spec.ts.snapis excluded by!**/*.snappackages/timeline-state-resolver/src/__tests__/__snapshots__/index.spec.ts.snapis excluded by!**/*.snapyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (6)
packages/timeline-state-resolver-types/src/actions.tspackages/timeline-state-resolver-types/src/integrations/casparcg.tspackages/timeline-state-resolver-types/src/integrations/httpSend.tspackages/timeline-state-resolver/package.jsonpackages/timeline-state-resolver/src/integrations/casparCG/index.tspackages/timeline-state-resolver/src/integrations/httpSend/index.ts
Saftret
added
the
contribution from BBC
nytamin
approved these changes
Jun 1, 2026
rjmunro
force-pushed
the
rjmunro/improve-device-error-notifications
branch
from
c4c70af to
911fed7
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/timeline-state-resolver-types/src/integrations/httpSend/timeline.ts (1)
41-61: ⚡ Quick winDefault messages discard the context they’re given.
The context map exposes interpolation values for
INVALID_TYPE(type),MISSING_PARAMS(url), andINVALID_PARAMS_TYPE(paramsType), but the corresponding default messages on lines 44–46 don’t reference any of them. OnlyREQUEST_FAILEDactually consumes its context. As a result the default fallback strings lose useful diagnostic detail (which method/url/params type was invalid), even though the data is plumbed through. Either interpolate these values into the defaults or drop them from the context map to keep the contract consistent.♻️ Proposed message templates
- [HttpSendActionErrorCode.INVALID_TYPE]: 'Failed to send HTTP command: invalid HTTP method type', - [HttpSendActionErrorCode.MISSING_PARAMS]: 'Failed to send HTTP command: missing params', - [HttpSendActionErrorCode.INVALID_PARAMS_TYPE]: 'Failed to send HTTP command: invalid params type', + [HttpSendActionErrorCode.INVALID_TYPE]: 'Failed to send HTTP command: invalid HTTP method type "{{type}}"', + [HttpSendActionErrorCode.MISSING_PARAMS]: 'Failed to send HTTP command to {{url}}: missing params', + [HttpSendActionErrorCode.INVALID_PARAMS_TYPE]: 'Failed to send HTTP command: invalid params type "{{paramsType}}"',🤖 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/timeline-state-resolver-types/src/integrations/httpSend/timeline.ts` around lines 41 - 61, The default messages in HttpSendActionErrorMessages ignore the available interpolation values declared in HttpSendActionErrorContextMap for HttpSendActionErrorCode.INVALID_TYPE, MISSING_PARAMS and INVALID_PARAMS_TYPE, so either update the message templates in HttpSendActionErrorMessages to include the corresponding placeholders (e.g. include {{type}} for INVALID_TYPE, {{url}} for MISSING_PARAMS and {{paramsType}} for INVALID_PARAMS_TYPE) or remove those entries from HttpSendActionErrorContextMap to keep the contract consistent; locate the two symbols HttpSendActionErrorMessages and HttpSendActionErrorContextMap and make the change so messages and context keys match.
🤖 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 `@packages/timeline-state-resolver/src/integrations/httpSend/index.ts`:
- Line 101: The lint rule requires typed Object.values calls; update both
occurrences that check TimelineContentTypeHTTP to use a generic type argument,
e.g. replace Object.values(TimelineContentTypeHTTP).includes(...) with
Object.values<TimelineContentTypeHTTP>(TimelineContentTypeHTTP).includes(...)
(apply the same change for the second call around the other check) so the
returned array is correctly typed.
---
Nitpick comments:
In
`@packages/timeline-state-resolver-types/src/integrations/httpSend/timeline.ts`:
- Around line 41-61: The default messages in HttpSendActionErrorMessages ignore
the available interpolation values declared in HttpSendActionErrorContextMap for
HttpSendActionErrorCode.INVALID_TYPE, MISSING_PARAMS and INVALID_PARAMS_TYPE, so
either update the message templates in HttpSendActionErrorMessages to include
the corresponding placeholders (e.g. include {{type}} for INVALID_TYPE, {{url}}
for MISSING_PARAMS and {{paramsType}} for INVALID_PARAMS_TYPE) or remove those
entries from HttpSendActionErrorContextMap to keep the contract consistent;
locate the two symbols HttpSendActionErrorMessages and
HttpSendActionErrorContextMap and make the change so messages and context keys
match.
🪄 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: f62d900b-5868-44a0-9e5d-a5c8df6fcce9
⛔ Files ignored due to path filters (2)
packages/timeline-state-resolver-types/src/__tests__/__snapshots__/index.spec.ts.snapis excluded by!**/*.snappackages/timeline-state-resolver/src/__tests__/__snapshots__/index.spec.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (5)
packages/timeline-state-resolver-types/src/actions.tspackages/timeline-state-resolver-types/src/integrations/casparcg/timeline.tspackages/timeline-state-resolver-types/src/integrations/httpSend/timeline.tspackages/timeline-state-resolver/src/integrations/casparCG/index.tspackages/timeline-state-resolver/src/integrations/httpSend/index.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/timeline-state-resolver-types/src/actions.ts
- packages/timeline-state-resolver/src/integrations/casparCG/index.ts
rjmunro
force-pushed
the
rjmunro/improve-device-error-notifications
branch
from
911fed7 to
f7bbc06
Compare
- Guard was checking cmd.type instead of cmd.paramsType, so valid paramsType values could be incorrectly rejected - Also switched from the `in` operator (checks enum keys) to Object.values().includes() (checks enum values), matching the pattern used for the cmd.type validation above it
rjmunro
force-pushed
the
rjmunro/improve-device-error-notifications
branch
from
f7bbc06 to
47392d5
Compare
|
rjmunro
added a commit
to bbc/sofie-core
that referenced
this pull request
Jun 2, 2026
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
added a commit
to bbc/sofie-core
that referenced
this pull request
Jun 18, 2026
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
This is a: Feature / Code improvement
Current Behavior
When a device action fails (e.g. CasparCG or HTTP Send), the
ActionExecutionResultreturned to the caller contains only a pre-rendered human-readable message. There is no structured way for consumers (e.g. blueprints) to identify the specific error type or to customise the error message for their context.New Behavior
ActionExecutionResultgains two optional fields:code— a structured error code following the patternACTION_{DEVICETYPE}_{REASON}(e.g.ACTION_CASPARCG_LAUNCHER_HOST_NOT_SET)context— aRecord<string, unknown>providing values for custom message interpolation viainterpolateTemplateString()The
responsefield continues to carry a pre-rendered fallback message, so existing consumers are unaffected. Consumers that want to customise messages can match oncodeand interpolate usingcontext.Both CasparCG and HTTP Send device integrations have been updated to return structured error codes and context on their action errors.
Additionally, the
wspackage has been bumped to address a vulnerability report (GHSA).Testing Instructions
ActionExecutionResultincludes acodeandcontextalongside the existingresponsemessage.responsecontinue to work unchanged.yarn unitOther Information
The
code/contextfields are purely additive and optional, making this change fully backwards-compatible. A blueprint can opt in to richer error messages by looking upcodein adeviceActionErrorMessagesmap and interpolating withcontext.Status