feat: Status message customisation#418
Open
rjmunro wants to merge 23 commits intoSofie-Automation:mainfrom
Open
feat: Status message customisation#418rjmunro wants to merge 23 commits intoSofie-Automation:mainfrom
rjmunro wants to merge 23 commits intoSofie-Automation:mainfrom
Conversation
WalkthroughAdds structured device status details and a DeviceStatusInput union; converts integrations to emit typed Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Device as Device (integration)
participant DeviceInstance as DeviceInstance
participant Normaliser as normaliseDeviceStatus
participant EventBus as connectionChanged / Consumer
Device->>DeviceInstance: getStatus(): DeviceStatusInput
DeviceInstance->>Normaliser: normaliseDeviceStatus(DeviceStatusInput)
Normaliser-->>DeviceInstance: DeviceStatus (statusCode + statusDetails + active)
DeviceInstance->>EventBus: emit connectionChanged(DeviceStatus)
EventBus->>Consumer: receive normalized DeviceStatus
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
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)
Warning Tools execution failed with the following error: Failed to run tools: 13 INTERNAL: Received RST_STREAM with code 2 (Internal server error) 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. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
Contributor
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![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
🤖 Fix all issues with AI agents
In `@packages/timeline-state-resolver/README.md`:
- Around line 129-134: The example uses a placeholder comment for the context
parameter type in createMyDeviceError; replace it with a mapped type so the
context is strongly typed (e.g., use MyDeviceErrorContextMap[T] as the context
type) and ensure a MyDeviceErrorContextMap type exists mapping each
MyDeviceErrorCode to its context shape; update the function signature for
createMyDeviceError<T extends MyDeviceErrorCode>(code: T, context:
MyDeviceErrorContextMap[T]): DeviceStatusError<T> and import/declare
MyDeviceErrorContextMap where needed so the example compiles and gives concrete
types for each error code.
In `@packages/timeline-state-resolver/src/deviceErrorMessages.ts`:
- Around line 34-38: The JSDoc example output is inconsistent with the template
interpolation: update the example so the output matches the actual template
behavior used by errorsToMessages; either adjust the allMessages template for
AtemErrorCode.DISCONNECTED to not include {{host}} or change the expected
messages array to include the host (e.g., "ATEM disconnected from
192.168.1.10"); reference the AtemErrorCode.DISCONNECTED entry and the
errorsToMessages/allMessages usage in the example and ensure the example's input
context (host) and expected output align.
In `@packages/timeline-state-resolver/src/index.ts`:
- Around line 12-18: The export lists a non-existent symbol AtemErrorContext
causing the build to fail; update the export block to remove AtemErrorContext
and instead export the actual context types exported by the errors module
(AtemErrorContextMap, AtemDisconnectedContext, AtemPsuFaultContext) or re-export
whichever specific context aliases you need (e.g., export type AtemErrorContext
= AtemErrorContextMap if you prefer a single alias), ensuring the exported
symbols match those declared in the './integrations/atem/errors' module.
🧹 Nitpick comments (1)
packages/timeline-state-resolver/src/__tests__/deviceErrorMessages.spec.ts (1)
11-13: Minor: Test context differs from actual definition.The test defines
host: stringas required, but inerrors.ts,AtemDisconnectedContexthashost?: string(optional). This inconsistency is minor since it doesn't affect test correctness, but consider aligning them for consistency.♻️ Suggested fix
interface AtemDisconnectedContext extends DeviceErrorContextBase { - host: string + host?: string }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
packages/timeline-state-resolver-types/src/device.tspackages/timeline-state-resolver-types/src/deviceError.tspackages/timeline-state-resolver-types/src/index.tspackages/timeline-state-resolver/README.mdpackages/timeline-state-resolver/src/__tests__/deviceErrorMessages.spec.tspackages/timeline-state-resolver/src/deviceErrorMessages.tspackages/timeline-state-resolver/src/index.tspackages/timeline-state-resolver/src/integrations/atem/__tests__/atem.spec.tspackages/timeline-state-resolver/src/integrations/atem/errors.tspackages/timeline-state-resolver/src/integrations/atem/index.ts
🧰 Additional context used
🧬 Code graph analysis (4)
packages/timeline-state-resolver-types/src/device.ts (1)
packages/timeline-state-resolver-types/src/deviceError.ts (1)
DeviceStatusError(27-35)
packages/timeline-state-resolver/src/deviceErrorMessages.ts (1)
packages/timeline-state-resolver-types/src/deviceError.ts (1)
DeviceStatusError(27-35)
packages/timeline-state-resolver/src/integrations/atem/errors.ts (1)
packages/timeline-state-resolver-types/src/deviceError.ts (2)
DeviceErrorContextBase(5-8)DeviceStatusError(27-35)
packages/timeline-state-resolver/src/__tests__/deviceErrorMessages.spec.ts (3)
packages/timeline-state-resolver/src/index.ts (2)
AtemErrorCode(14-14)AtemErrorMessages(15-15)packages/timeline-state-resolver-types/src/deviceError.ts (2)
DeviceErrorContextBase(5-8)DeviceStatusError(27-35)packages/timeline-state-resolver/src/deviceErrorMessages.ts (1)
errorsToMessages(40-48)
🪛 GitHub Actions: Node CI
packages/timeline-state-resolver-types/src/deviceError.ts
[error] 1-1: Build failed for timeline-state-resolver:build. Process completed with exit code 130.
packages/timeline-state-resolver/README.md
[error] 1-1: Build failed for timeline-state-resolver:build. Process completed with exit code 130.
packages/timeline-state-resolver/src/index.ts
[error] 17-17: "./integrations/atem/errors" has no exported member named 'AtemErrorContext'. Did you mean 'AtemErrorContextMap'?
[error] 1-1: Build failed for timeline-state-resolver:build. Process completed with exit code 130.
packages/timeline-state-resolver/src/integrations/atem/index.ts
[error] 1-1: Build failed for timeline-state-resolver:build. Process completed with exit code 130.
packages/timeline-state-resolver-types/src/index.ts
[error] 17-17: "./integrations/atem/errors" has no exported member named 'AtemErrorContext'. Did you mean 'AtemErrorContextMap'?
[error] 1-1: Build failed for timeline-state-resolver:build. Process completed with exit code 130.
packages/timeline-state-resolver-types/src/device.ts
[error] 1-1: Build failed for timeline-state-resolver:build. Process completed with exit code 130.
packages/timeline-state-resolver/src/deviceErrorMessages.ts
[error] 1-1: Build failed for timeline-state-resolver:build. Process completed with exit code 130.
packages/timeline-state-resolver/src/integrations/atem/errors.ts
[error] 1-1: Build failed for timeline-state-resolver:build. Process completed with exit code 130.
packages/timeline-state-resolver/src/__tests__/deviceErrorMessages.spec.ts
[error] 1-1: Build failed for timeline-state-resolver:build. Process completed with exit code 130.
packages/timeline-state-resolver/src/integrations/atem/__tests__/atem.spec.ts
[error] 1-1: Build failed for timeline-state-resolver:build. Process completed with exit code 130.
🔇 Additional comments (12)
packages/timeline-state-resolver-types/src/index.ts (1)
55-55: LGTM!The new export follows the established pattern and correctly exposes the
deviceErrormodule types for consumers.packages/timeline-state-resolver/README.md (1)
49-88: Good documentation for the new feature.The documentation clearly explains the structured error system, provides usage examples for blueprints, and shows how to convert errors to messages. This will help consumers understand and adopt the new error handling approach.
packages/timeline-state-resolver-types/src/deviceError.ts (1)
1-35: Well-designed type definitions.The generic interface design with sensible defaults provides flexibility for device-specific implementations while maintaining type safety. The JSDoc documentation clearly explains the naming convention and usage patterns.
packages/timeline-state-resolver-types/src/device.ts (1)
3-3: Clean extension of DeviceStatus.The optional
errorsfield maintains backward compatibility while enabling the new structured error system. The documentation clearly explains the fallback behavior when errors are not present or understood.Also applies to: 15-24
packages/timeline-state-resolver/src/deviceErrorMessages.ts (1)
9-17: Solid implementation.The
interpolatefunction correctly preserves placeholders when values are missing, providing graceful degradation. TheerrorsToMessagesfunction has a sensible fallback to the error code when no template is found.Also applies to: 40-47
packages/timeline-state-resolver/src/integrations/atem/errors.ts (1)
1-68: Well-structured type-safe error system.The implementation follows TypeScript best practices:
- Const object + derived type pattern for error codes ensures type safety
- Context interfaces properly extend
DeviceErrorContextBase- Factory function with generics enforces correct context types per error code
- Message templates use consistent
{{variable}}interpolation syntaxpackages/timeline-state-resolver/src/index.ts (1)
20-37: TODO comment is reasonable for tracking future work.The TODO provides a clear pattern reference and lists all remaining integrations. Consider creating an issue to track this work instead of leaving it in code long-term.
packages/timeline-state-resolver/src/__tests__/deviceErrorMessages.spec.ts (1)
58-148: Comprehensive test coverage forerrorsToMessages.Tests cover all essential scenarios:
- Single and multiple error conversion
- Variable interpolation from context
- Custom template overrides
- Fallback to raw error code when no template exists
- Missing placeholder handling
- Empty array edge case
- Optional
deviceName/deviceIdfields- Cross-device template combination
packages/timeline-state-resolver/src/integrations/atem/__tests__/atem.spec.ts (2)
2-2: Import will work once the build issue inindex.tsis resolved.The import from
'..'correctly references the re-exportedAtemErrorCodeandcreateAtemErrorfrom the ATEM integration barrel.
57-121: Thorough test coverage for the new error structure.The 'Check Status' test comprehensively validates:
- Initial disconnected state produces
DISCONNECTEDerror- Connected state clears errors to
undefined- PSU faults generate
PSU_FAULTerrors with correct context (psuNumber,totalPsus)- Reconnection/disconnect transitions
The assertions for both
messagesanderrorsarrays ensure backward compatibility while validating the new structured error system.packages/timeline-state-resolver/src/integrations/atem/index.ts (2)
39-43: Clean separation of error imports and re-exports.The error-related imports and exports are well-organized, making the error types available for blueprint customization while keeping the implementation details in
errors.ts.
158-188: Well-structured error accumulation and status reporting.The refactored
getStatus()method:
- Properly accumulates errors into a typed array
- Uses early return for disconnected state (appropriate optimization)
- Iterates PSUs with 1-indexed display for human-readable messages
- Derives messages via
errorsToMessagesfor consistency- Returns
errors: undefinedwhen empty (backward compatible)The status code logic correctly prioritizes:
BAD(disconnected) >WARNING_MAJOR(PSU fault) >GOOD.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
rjmunro
force-pushed
the
rjmunro/error-message-customisation
branch
5 times, most recently
from
f3d719f to
2d26d58
Compare
7 tasks
rjmunro
force-pushed
the
rjmunro/error-message-customisation
branch
5 times, most recently
from
687fb30 to
099da5e
Compare
Contributor
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@packages/timeline-state-resolver-types/src/index.ts`:
- Around line 29-31: Duplicate export of './integrations/atem' is present;
remove the redundant export line so './integrations/atem' is only exported once
(keep a single export statement for the ATEM integration in index.ts to avoid
duplicate re-exports).
In `@packages/timeline-state-resolver/src/integrations/casparCG/errors.ts`:
- Around line 8-12: The JSDoc example for createCasparCGError uses
CasparCGErrorCode.QUEUE_OVERFLOW but omits required context fields; update the
example to supply the full CasparCGErrorContextMap for QUEUE_OVERFLOW (include
deviceName, host, and port) so it matches the required shape, i.e., modify the
example call to createCasparCGError(CasparCGErrorCode.QUEUE_OVERFLOW, {
deviceName: 'CasparCG 1', host: '192.168.1.10', port: 5250 }) to mirror the
DISCONNECTED example and ensure consistency with CasparCGErrorContextMap.
In `@packages/timeline-state-resolver/src/integrations/casparCG/index.ts`:
- Around line 788-792: The returned status object currently always includes an
errors array which can be empty and is truthy; in the function that returns the
object (the block using statusCode, messages: errorsToMessages(errors,
CasparCGErrorMessages), errors, active: this.isActive) change the construction
so that the errors property is only added when errors.length > 0 (and likewise
omit messages if errors is empty), e.g. build the result object conditionally
and only assign errors when non-empty to match the ATEM integration and avoid
consumers misinterpreting an empty array as an error.
- Around line 760-785: The getStatus() method dereferences this._ccg.host and
this._ccg.port which can be undefined before init() or during teardown; update
getStatus() to safely read host/port (e.g., const host = this._ccg?.host ??
this.initOptions?.host ?? '' and const port = this._ccg?.port ??
this.initOptions?.port ?? '') and use those fallback values when calling
createCasparCGError(CasparCGErrorCode.DISCONNECTED, ...) and
createCasparCGError(CasparCGErrorCode.QUEUE_OVERFLOW, ...) so no property access
on an uninitialized this._ccg occurs.
In `@packages/timeline-state-resolver/src/service/DeviceInstance.ts`:
- Around line 269-272: The constructor currently calls new
deviceSpecs.deviceClass(...) which triggers _getDeviceContextAPI() and reads
this._deviceName before _deviceName is assigned; move the assignment that sets
this._deviceName to occur before instantiating deviceSpecs.deviceClass (i.e.,
set this._deviceName early in the constructor), so _getDeviceContextAPI()
returns the correct name; update any constructor flow that builds the
DeviceContextAPI or calls _getDeviceContextAPI() to happen after this
assignment.
🧹 Nitpick comments (1)
packages/timeline-state-resolver-types/src/templateString.ts (1)
22-29: Consider removing or conditionalizing theconsole.logfor missing values.Line 25 logs a message for every missing placeholder value. This could be noisy in production, especially if:
- Custom templates intentionally use fewer variables than available in context
- Templates are processed frequently
Since this is expected behavior when
leaveUnknownPlaceholdersistrue(placeholders are preserved for later substitution), the log may not always indicate a problem.♻️ Consider removing or making logging conditional
export function interpolateTemplateString( template: string, context: Record<string, any>, - leaveUnknownPlaceholders = false + leaveUnknownPlaceholders = false, + debug = false ): string { return template.replace(/\{\{(\w+)\}\}/g, (match, key) => { const value = context[key] if (value === undefined || value === null) { - console.log(`interpolateTemplateString: missing value for key ${key} in template:`, template) + if (debug) { + console.log(`interpolateTemplateString: missing value for key ${key} in template:`, template) + } return leaveUnknownPlaceholders ? match : key // Keep key or placeholder if no value } return String(value) }) }Alternatively, remove the log entirely since the behavior (preserving placeholder or using key name) is well-documented.
rjmunro
force-pushed
the
rjmunro/error-message-customisation
branch
3 times, most recently
from
b76e7c4 to
46cf25d
Compare
Julusian
reviewed
Jan 27, 2026
| * These codes can be customized in blueprints via deviceErrorMessages. | ||
| */ | ||
| export const AtemErrorCode = { | ||
| DISCONNECTED: 'DEVICE_ATEM_DISCONNECTED', |
Member
There was a problem hiding this comment.
Here they are DEVICE_ATEM_DISCONNECTED, in some others they are HTTPWATCHER_KEYWORD_NOT_FOUND. It would be good for that to be consistent
Contributor
Author
There was a problem hiding this comment.
Changed to always start DEVICE_
rjmunro
force-pushed
the
rjmunro/error-message-customisation
branch
5 times, most recently
from
bea7594 to
ec334bd
Compare
Contributor
Author
|
@coderabbitai review |
rjmunro
changed the title
rjmunro
force-pushed
the
rjmunro/error-message-customisation
branch
from
9646900 to
61ea489
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (5)
packages/timeline-state-resolver-types/src/device.ts (1)
21-27:⚠️ Potential issue | 🟠 MajorDocument
statusDetailscapability semantics explicitly.The doc should explicitly define
statusDetails: undefined(not supported) vsstatusDetails: [](supported but no active details). Without that, consumers cannot reliably feature-detect structured status support fromDeviceStatusalone.Based on learnings: In the DeviceStatus interface, an empty `errors`/details array indicates structured support, while `undefined` indicates no structured support and consumers should fall back to plain messages.📝 Suggested doc update
/** * Structured status details. + * Feature-detection semantics: + * - `statusDetails: undefined` => structured status details are not supported by this device. + * - `statusDetails: []` => structured status details are supported, but none are currently active. + * + * If `statusDetails` is `undefined`, consumers should fall back to `messages`. * Each detail carries a pre-rendered message and, for devices that support it, a status code * and context object that consumers can use to apply custom message templates via * interpolateTemplateString(). */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/timeline-state-resolver-types/src/device.ts` around lines 21 - 27, Update the JSDoc for the DeviceStatus.statusDetails property to explicitly state capability semantics: document that statusDetails === undefined means the device does not support structured status details (consumers must fall back to plain messages), while statusDetails === [] means the device supports structured details but currently has no active details; reference DeviceStatus, statusDetails, and DeviceStatusDetail and mention consumers can use interpolateTemplateString() when structured details are present.packages/timeline-state-resolver/src/integrations/osc/index.ts (1)
202-212:⚠️ Potential issue | 🟡 MinorUse runtime device name in OSC status context.
Line 202 hardcodes
"OSC", so renamed device instances lose identity in templated status messages.🔧 Proposed fix
- const deviceName = 'OSC' + const deviceName = this.context.deviceName ?? 'OSC'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/timeline-state-resolver/src/integrations/osc/index.ts` around lines 202 - 212, The status code uses a hardcoded deviceName = 'OSC' so named instances lose their identity; change deviceName to derive from the runtime options (e.g. const deviceName = this.options?.deviceName ?? this.options?.name ?? 'OSC') and use that when calling createOSCStatusDetail for OSCStatusCode.TCP_DISCONNECTED (and any other places that use deviceName) so templated status messages reflect the actual device instance.packages/timeline-state-resolver/src/integrations/panasonicPTZ/index.ts (1)
141-149:⚠️ Potential issue | 🟡 MinorAvoid hardcoded device identity and nullable context in PTZ status detail.
Line 141 should use the runtime instance name, and Lines 148-149 should fallback to stable values to avoid rendering
"undefined"in messages.🔧 Proposed fix
- const deviceName = 'Panasonic PTZ' + const deviceName = this.context.deviceName ?? 'Panasonic PTZ' @@ - host: this._host, - port: this._port, + host: this._host ?? '', + port: this._port ?? 0,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/timeline-state-resolver/src/integrations/panasonicPTZ/index.ts` around lines 141 - 149, Replace the hardcoded deviceName and nullable host/port in the NOT_CONNECTED status path: compute deviceName from the runtime instance (e.g. const deviceName = this._device?.name ?? 'Panasonic PTZ' or similar) and avoid optional chaining inside the check (use if (!(this._device && this._device.connected)) or equivalent). When calling createPanasonicPTZStatusDetail(PanasonicPTZStatusCode.NOT_CONNECTED, {...}), pass stable fallbacks for host and port (for example this._host ?? 'unknown host' and this._port ?? 'unknown port') so the status detail never contains "undefined".packages/timeline-state-resolver/src/integrations/hyperdeck/index.ts (1)
261-269:⚠️ Potential issue | 🟡 MinorUse runtime device name in status context.
Line 261 hardcodes
'Hyperdeck', so status detail context can be wrong for renamed instances.🔧 Proposed fix
- const deviceName = 'Hyperdeck' + const deviceName = this.context.deviceName🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/timeline-state-resolver/src/integrations/hyperdeck/index.ts` around lines 261 - 269, The status context currently hardcodes deviceName = 'Hyperdeck' causing incorrect names for renamed instances; change the deviceName assignment used in the NOT_CONNECTED status block to use the runtime name (for example: this.deviceName ?? this._initOptions?.deviceName ?? 'Hyperdeck') so createHyperdeckStatusDetail(HyperdeckStatusCode.NOT_CONNECTED, {...}) passes the actual instance name instead of the literal string.packages/timeline-state-resolver/src/integrations/httpWatcher/index.ts (1)
59-68:⚠️ Potential issue | 🟠 MajorRedact and cap response data before storing it in
statusDetails.
REQUEST_ERROR,UNEXPECTED_STATUS_CODE, andKEYWORD_NOT_FOUNDstill copy raw response bodies/headers into the emitted status payload. These details now flow throughconnectionChanged, so they can leak cookies or other sensitive values and can grow without bound on large responses. Please sanitize headers and truncate body previews before callingcreateHTTPWatcherStatusDetail(...).Also applies to: 91-107
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/timeline-state-resolver/src/integrations/httpWatcher/index.ts` around lines 59 - 68, The status payload currently copies raw response bodies/headers into statusDetails for HTTPWatcher errors (e.g., when handling REQUEST_ERROR in the catch block that calls this._setStatus(StatusCode.BAD, [createHTTPWatcherStatusDetail(HTTPWatcherStatusCode.REQUEST_ERROR, context)]) and the similar UNEXPECTED_STATUS_CODE / KEYWORD_NOT_FOUND paths); before building the context passed to createHTTPWatcherStatusDetail, sanitize headers (redact cookie/set-cookie and other sensitive header values) and truncate body previews to a reasonable max length (e.g., 1–2KB or configurable) and replace binary/long content with a summarized placeholder; apply the same redaction/truncation logic wherever createHTTPWatcherStatusDetail is invoked (including the blocks around lines 91–107) so statusDetails never contain unbounded or sensitive raw response data.
🧹 Nitpick comments (3)
packages/timeline-state-resolver-types/src/integrations/hyperdeck.ts (1)
55-58: Consider using available context in default connectivity message.
NOT_CONNECTEDcurrently discardsdeviceName/host/port, which makes default diagnostics less actionable than the typed context allows.💡 Proposed refinement
export const HyperdeckStatusMessages: Record<HyperdeckStatusCode, string> = { - [HyperdeckStatusCode.NOT_CONNECTED]: 'Not connected', + [HyperdeckStatusCode.NOT_CONNECTED]: '{{deviceName}} not connected ({{host}}:{{port}})', [HyperdeckStatusCode.LOW_RECORDING_TIME]: 'Recording time left is less than {{minutes}} minutes and {{seconds}} seconds',🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/timeline-state-resolver-types/src/integrations/hyperdeck.ts` around lines 55 - 58, The default message for HyperdeckStatusCode.NOT_CONNECTED discards context (deviceName/host/port); update the mapping entry for HyperdeckStatusCode.NOT_CONNECTED to include contextual placeholders (e.g. 'Not connected to {{deviceName}} ({{host}}:{{port}})' or similar) so the formatted message uses the available context when rendered; ensure any formatter that consumes this map (the code that calls the status->message lookup and interpolates with the provided context) still supplies the same keys (deviceName, host, port) so the new placeholders resolve correctly.packages/timeline-state-resolver/src/integrations/hyperdeck/index.ts (1)
258-335: SplitgetStatus()into smaller helpers to reduce complexity.This method is doing multiple independent checks; extracting each check into helper methods will make this easier to test/maintain and should satisfy the cognitive complexity threshold.
♻️ Refactor sketch
- getStatus(): DeviceStatusInput { - let statusCode = StatusCode.GOOD - const statusDetails: DeviceStatusDetail[] = [] - const deviceName = 'Hyperdeck' - ... - return { - statusCode, - statusDetails, - } - } + getStatus(): DeviceStatusInput { + const deviceName = this.context.deviceName + const status = this._createBaseStatus() + + if (!this._connected) { + this._appendNotConnectedStatus(status, deviceName) + return status + } + + this._appendRecordingTimeStatus(status, deviceName) + this._appendSlotStatus(status, deviceName) + this._appendTransportStatus(status, deviceName) + + return status + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/timeline-state-resolver/src/integrations/hyperdeck/index.ts` around lines 258 - 335, The getStatus() function is doing multiple independent checks; refactor by extracting each check into small helper methods: create checkConnectionStatus() that returns {statusCode?: StatusCode, statusDetails?: DeviceStatusDetail[]} for connection/not-connected logic (uses this._connected, this._initOptions), checkRecordingTimeStatus() for the _minRecordingTime/_recordingTime logic (returns optional statusCode and detail created by createHyperdeckStatusDetail with HyperdeckStatusCode.LOW_RECORDING_TIME), checkSlotAvailabilityStatus() that iterates slots 1..this._slotCount using this._slotStatus and this._suppressEmptySlotWarnings and returns aggregated statusCode/statusDetails (uses HyperdeckStatusCode.SLOT_NOT_MOUNTED), and checkTransportStatus() that compares this._expectedTransportStatus vs this._transportStatus and returns appropriate statusCode/statusDetails (uses TransportStatus.RECORD/PLAY and HyperdeckStatusCode.NOT_RECORDING/NOT_PLAYING); then simplify getStatus() to call these helpers, merge their statusDetails into one array, and compute the highest severity statusCode to return.packages/timeline-state-resolver/src/integrations/telemetrics/index.ts (1)
52-52: Use Telemetrics-specific detail typing for stronger safety.
statusDetailsis currentlyDeviceStatusDetail[], which loses Telemetrics code/context coupling at compile time. PreferTelemetricsStatusDetail[].Suggested diff
import { DeviceStatusInput, - DeviceStatusDetail, + TelemetricsStatusDetail, Mappings, StatusCode, TelemetricsDeviceTypes, TelemetricsOptions, TimelineContentTelemetrics, TSRTimelineContent, TelemetricsStatusCode, } from 'timeline-state-resolver-types' @@ - const statusDetails: DeviceStatusDetail[] = [] + const statusDetails: TelemetricsStatusDetail[] = []🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/timeline-state-resolver/src/integrations/telemetrics/index.ts` at line 52, Change the statusDetails declaration to use the Telemetrics-specific type so Telemetrics-only fields are preserved: replace the array type DeviceStatusDetail[] with TelemetricsStatusDetail[] for the statusDetails variable in the integrations/telemetrics module (look for the const statusDetails declaration in index.ts) and update any downstream usages or imports if necessary to reference TelemetricsStatusDetail instead of DeviceStatusDetail.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/timeline-state-resolver/src/integrations/sofieChef/index.ts`:
- Around line 208-215: The status block currently hardcodes deviceName as
'SofieChef'; change it to use the runtime/configured device name (e.g. the
instance property or config field used by this integration) so status messages
reflect the actual device. Locate the block around the connected check
(references: this.connected, statusCode, statusDetails,
createSofieChefStatusDetail, SofieChefStatusCode.NOT_CONNECTED) and replace the
literal deviceName with the appropriate runtime value (for example
this.deviceName or the config's deviceName property) so the object passed to
createSofieChefStatusDetail contains the real device name.
In `@packages/timeline-state-resolver/src/service/DeviceInstance.ts`:
- Around line 31-45: normaliseDeviceStatus currently fabricates statusDetails
from messages for legacy devices; change it so that when input lacks
statusDetails you return statusDetails: undefined (not an array) so callers can
detect legacy devices—update the function normaliseDeviceStatus to copy
statusDetails when present and leave statusDetails undefined otherwise,
preserving messages and active; also update tests in deviceInstance.spec.ts to
add assertions for both cases (statusDetails undefined vs statusDetails === []
for integrations that support structured errors) to cover the empty-array vs
undefined distinction and ensure consumers fall back to messages when
statusDetails is undefined.
---
Duplicate comments:
In `@packages/timeline-state-resolver-types/src/device.ts`:
- Around line 21-27: Update the JSDoc for the DeviceStatus.statusDetails
property to explicitly state capability semantics: document that statusDetails
=== undefined means the device does not support structured status details
(consumers must fall back to plain messages), while statusDetails === [] means
the device supports structured details but currently has no active details;
reference DeviceStatus, statusDetails, and DeviceStatusDetail and mention
consumers can use interpolateTemplateString() when structured details are
present.
In `@packages/timeline-state-resolver/src/integrations/httpWatcher/index.ts`:
- Around line 59-68: The status payload currently copies raw response
bodies/headers into statusDetails for HTTPWatcher errors (e.g., when handling
REQUEST_ERROR in the catch block that calls this._setStatus(StatusCode.BAD,
[createHTTPWatcherStatusDetail(HTTPWatcherStatusCode.REQUEST_ERROR, context)])
and the similar UNEXPECTED_STATUS_CODE / KEYWORD_NOT_FOUND paths); before
building the context passed to createHTTPWatcherStatusDetail, sanitize headers
(redact cookie/set-cookie and other sensitive header values) and truncate body
previews to a reasonable max length (e.g., 1–2KB or configurable) and replace
binary/long content with a summarized placeholder; apply the same
redaction/truncation logic wherever createHTTPWatcherStatusDetail is invoked
(including the blocks around lines 91–107) so statusDetails never contain
unbounded or sensitive raw response data.
In `@packages/timeline-state-resolver/src/integrations/hyperdeck/index.ts`:
- Around line 261-269: The status context currently hardcodes deviceName =
'Hyperdeck' causing incorrect names for renamed instances; change the deviceName
assignment used in the NOT_CONNECTED status block to use the runtime name (for
example: this.deviceName ?? this._initOptions?.deviceName ?? 'Hyperdeck') so
createHyperdeckStatusDetail(HyperdeckStatusCode.NOT_CONNECTED, {...}) passes the
actual instance name instead of the literal string.
In `@packages/timeline-state-resolver/src/integrations/osc/index.ts`:
- Around line 202-212: The status code uses a hardcoded deviceName = 'OSC' so
named instances lose their identity; change deviceName to derive from the
runtime options (e.g. const deviceName = this.options?.deviceName ??
this.options?.name ?? 'OSC') and use that when calling createOSCStatusDetail for
OSCStatusCode.TCP_DISCONNECTED (and any other places that use deviceName) so
templated status messages reflect the actual device instance.
In `@packages/timeline-state-resolver/src/integrations/panasonicPTZ/index.ts`:
- Around line 141-149: Replace the hardcoded deviceName and nullable host/port
in the NOT_CONNECTED status path: compute deviceName from the runtime instance
(e.g. const deviceName = this._device?.name ?? 'Panasonic PTZ' or similar) and
avoid optional chaining inside the check (use if (!(this._device &&
this._device.connected)) or equivalent). When calling
createPanasonicPTZStatusDetail(PanasonicPTZStatusCode.NOT_CONNECTED, {...}),
pass stable fallbacks for host and port (for example this._host ?? 'unknown
host' and this._port ?? 'unknown port') so the status detail never contains
"undefined".
---
Nitpick comments:
In `@packages/timeline-state-resolver-types/src/integrations/hyperdeck.ts`:
- Around line 55-58: The default message for HyperdeckStatusCode.NOT_CONNECTED
discards context (deviceName/host/port); update the mapping entry for
HyperdeckStatusCode.NOT_CONNECTED to include contextual placeholders (e.g. 'Not
connected to {{deviceName}} ({{host}}:{{port}})' or similar) so the formatted
message uses the available context when rendered; ensure any formatter that
consumes this map (the code that calls the status->message lookup and
interpolates with the provided context) still supplies the same keys
(deviceName, host, port) so the new placeholders resolve correctly.
In `@packages/timeline-state-resolver/src/integrations/hyperdeck/index.ts`:
- Around line 258-335: The getStatus() function is doing multiple independent
checks; refactor by extracting each check into small helper methods: create
checkConnectionStatus() that returns {statusCode?: StatusCode, statusDetails?:
DeviceStatusDetail[]} for connection/not-connected logic (uses this._connected,
this._initOptions), checkRecordingTimeStatus() for the
_minRecordingTime/_recordingTime logic (returns optional statusCode and detail
created by createHyperdeckStatusDetail with
HyperdeckStatusCode.LOW_RECORDING_TIME), checkSlotAvailabilityStatus() that
iterates slots 1..this._slotCount using this._slotStatus and
this._suppressEmptySlotWarnings and returns aggregated statusCode/statusDetails
(uses HyperdeckStatusCode.SLOT_NOT_MOUNTED), and checkTransportStatus() that
compares this._expectedTransportStatus vs this._transportStatus and returns
appropriate statusCode/statusDetails (uses TransportStatus.RECORD/PLAY and
HyperdeckStatusCode.NOT_RECORDING/NOT_PLAYING); then simplify getStatus() to
call these helpers, merge their statusDetails into one array, and compute the
highest severity statusCode to return.
In `@packages/timeline-state-resolver/src/integrations/telemetrics/index.ts`:
- Line 52: Change the statusDetails declaration to use the Telemetrics-specific
type so Telemetrics-only fields are preserved: replace the array type
DeviceStatusDetail[] with TelemetricsStatusDetail[] for the statusDetails
variable in the integrations/telemetrics module (look for the const
statusDetails declaration in index.ts) and update any downstream usages or
imports if necessary to reference TelemetricsStatusDetail instead of
DeviceStatusDetail.
🪄 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: 3e1772ce-096c-4fb9-8e4e-118eb93b6f57
⛔ 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 (82)
packages/timeline-state-resolver-api/src/device.tspackages/timeline-state-resolver-api/src/tsr-api.tspackages/timeline-state-resolver-types/src/__tests__/templateString.spec.tspackages/timeline-state-resolver-types/src/device.tspackages/timeline-state-resolver-types/src/deviceStatusDetail.tspackages/timeline-state-resolver-types/src/index.tspackages/timeline-state-resolver-types/src/integrations/atem.tspackages/timeline-state-resolver-types/src/integrations/casparcg.tspackages/timeline-state-resolver-types/src/integrations/httpWatcher.tspackages/timeline-state-resolver-types/src/integrations/hyperdeck.tspackages/timeline-state-resolver-types/src/integrations/lawo.tspackages/timeline-state-resolver-types/src/integrations/obs.tspackages/timeline-state-resolver-types/src/integrations/osc.tspackages/timeline-state-resolver-types/src/integrations/panasonicPTZ.tspackages/timeline-state-resolver-types/src/integrations/pharos.tspackages/timeline-state-resolver-types/src/integrations/quantel.tspackages/timeline-state-resolver-types/src/integrations/shotoku.tspackages/timeline-state-resolver-types/src/integrations/sisyfos.tspackages/timeline-state-resolver-types/src/integrations/sofieChef.tspackages/timeline-state-resolver-types/src/integrations/telemetrics.tspackages/timeline-state-resolver-types/src/integrations/tricaster.tspackages/timeline-state-resolver-types/src/integrations/vizMSE.tspackages/timeline-state-resolver-types/src/integrations/vmix.tspackages/timeline-state-resolver-types/src/integrations/websocketClient.tspackages/timeline-state-resolver-types/src/templateString.tspackages/timeline-state-resolver/src/devices/device.tspackages/timeline-state-resolver/src/index.tspackages/timeline-state-resolver/src/integrations/__tests__/testlib.tspackages/timeline-state-resolver/src/integrations/abstract/index.tspackages/timeline-state-resolver/src/integrations/atem/__tests__/atem.spec.tspackages/timeline-state-resolver/src/integrations/atem/index.tspackages/timeline-state-resolver/src/integrations/atem/messages.tspackages/timeline-state-resolver/src/integrations/casparCG/index.tspackages/timeline-state-resolver/src/integrations/casparCG/messages.tspackages/timeline-state-resolver/src/integrations/httpSend/index.tspackages/timeline-state-resolver/src/integrations/httpWatcher/__tests__/httpwatcher.spec.tspackages/timeline-state-resolver/src/integrations/httpWatcher/index.tspackages/timeline-state-resolver/src/integrations/httpWatcher/messages.tspackages/timeline-state-resolver/src/integrations/hyperdeck/__tests__/hyperdeck.spec.tspackages/timeline-state-resolver/src/integrations/hyperdeck/index.tspackages/timeline-state-resolver/src/integrations/hyperdeck/messages.tspackages/timeline-state-resolver/src/integrations/kairos/index.tspackages/timeline-state-resolver/src/integrations/lawo/index.tspackages/timeline-state-resolver/src/integrations/lawo/messages.tspackages/timeline-state-resolver/src/integrations/multiOsc/index.tspackages/timeline-state-resolver/src/integrations/obs/index.tspackages/timeline-state-resolver/src/integrations/obs/messages.tspackages/timeline-state-resolver/src/integrations/ograf/index.tspackages/timeline-state-resolver/src/integrations/osc/index.tspackages/timeline-state-resolver/src/integrations/osc/messages.tspackages/timeline-state-resolver/src/integrations/panasonicPTZ/index.tspackages/timeline-state-resolver/src/integrations/panasonicPTZ/messages.tspackages/timeline-state-resolver/src/integrations/pharos/index.tspackages/timeline-state-resolver/src/integrations/pharos/messages.tspackages/timeline-state-resolver/src/integrations/quantel/index.tspackages/timeline-state-resolver/src/integrations/quantel/messages.tspackages/timeline-state-resolver/src/integrations/shotoku/index.tspackages/timeline-state-resolver/src/integrations/shotoku/messages.tspackages/timeline-state-resolver/src/integrations/singularLive/index.tspackages/timeline-state-resolver/src/integrations/sisyfos/index.tspackages/timeline-state-resolver/src/integrations/sisyfos/messages.tspackages/timeline-state-resolver/src/integrations/sofieChef/__tests__/sofieChef.spec.tspackages/timeline-state-resolver/src/integrations/sofieChef/index.tspackages/timeline-state-resolver/src/integrations/sofieChef/messages.tspackages/timeline-state-resolver/src/integrations/tcpSend/index.tspackages/timeline-state-resolver/src/integrations/telemetrics/__tests__/telemetrics.spec.tspackages/timeline-state-resolver/src/integrations/telemetrics/index.tspackages/timeline-state-resolver/src/integrations/telemetrics/messages.tspackages/timeline-state-resolver/src/integrations/tricaster/index.tspackages/timeline-state-resolver/src/integrations/tricaster/messages.tspackages/timeline-state-resolver/src/integrations/udpSend/index.tspackages/timeline-state-resolver/src/integrations/viscaOverIP/index.tspackages/timeline-state-resolver/src/integrations/vizMSE/index.tspackages/timeline-state-resolver/src/integrations/vizMSE/messages.tspackages/timeline-state-resolver/src/integrations/vmix/index.tspackages/timeline-state-resolver/src/integrations/vmix/messages.tspackages/timeline-state-resolver/src/integrations/websocketClient/__tests__/websocketClient.spec.tspackages/timeline-state-resolver/src/integrations/websocketClient/connection.tspackages/timeline-state-resolver/src/integrations/websocketClient/index.tspackages/timeline-state-resolver/src/integrations/websocketClient/messages.tspackages/timeline-state-resolver/src/service/DeviceInstance.tspackages/timeline-state-resolver/src/service/__tests__/deviceInstance.spec.ts
✅ Files skipped from review due to trivial changes (16)
- packages/timeline-state-resolver/src/integrations/tests/testlib.ts
- packages/timeline-state-resolver/src/service/tests/deviceInstance.spec.ts
- packages/timeline-state-resolver/src/integrations/obs/messages.ts
- packages/timeline-state-resolver/src/integrations/singularLive/index.ts
- packages/timeline-state-resolver/src/integrations/hyperdeck/messages.ts
- packages/timeline-state-resolver/src/index.ts
- packages/timeline-state-resolver/src/integrations/shotoku/messages.ts
- packages/timeline-state-resolver/src/integrations/httpSend/index.ts
- packages/timeline-state-resolver/src/integrations/sisyfos/messages.ts
- packages/timeline-state-resolver-types/src/integrations/lawo.ts
- packages/timeline-state-resolver/src/devices/device.ts
- packages/timeline-state-resolver-types/src/integrations/websocketClient.ts
- packages/timeline-state-resolver-types/src/integrations/sisyfos.ts
- packages/timeline-state-resolver-types/src/integrations/tricaster.ts
- packages/timeline-state-resolver-types/src/integrations/httpWatcher.ts
- packages/timeline-state-resolver-types/src/integrations/pharos.ts
🚧 Files skipped from review as they are similar to previous changes (27)
- packages/timeline-state-resolver-types/src/deviceStatusDetail.ts
- packages/timeline-state-resolver/src/integrations/ograf/index.ts
- packages/timeline-state-resolver/src/integrations/tricaster/messages.ts
- packages/timeline-state-resolver-types/src/tests/templateString.spec.ts
- packages/timeline-state-resolver/src/integrations/httpWatcher/messages.ts
- packages/timeline-state-resolver/src/integrations/osc/messages.ts
- packages/timeline-state-resolver/src/integrations/lawo/messages.ts
- packages/timeline-state-resolver/src/integrations/sofieChef/tests/sofieChef.spec.ts
- packages/timeline-state-resolver/src/integrations/panasonicPTZ/messages.ts
- packages/timeline-state-resolver/src/integrations/quantel/messages.ts
- packages/timeline-state-resolver/src/integrations/sofieChef/messages.ts
- packages/timeline-state-resolver/src/integrations/atem/messages.ts
- packages/timeline-state-resolver/src/integrations/udpSend/index.ts
- packages/timeline-state-resolver/src/integrations/abstract/index.ts
- packages/timeline-state-resolver/src/integrations/vmix/index.ts
- packages/timeline-state-resolver-types/src/integrations/vmix.ts
- packages/timeline-state-resolver-types/src/integrations/sofieChef.ts
- packages/timeline-state-resolver-types/src/integrations/panasonicPTZ.ts
- packages/timeline-state-resolver-types/src/integrations/osc.ts
- packages/timeline-state-resolver-types/src/integrations/obs.ts
- packages/timeline-state-resolver-types/src/templateString.ts
- packages/timeline-state-resolver/src/integrations/atem/tests/atem.spec.ts
- packages/timeline-state-resolver/src/integrations/telemetrics/messages.ts
- packages/timeline-state-resolver/src/integrations/hyperdeck/tests/hyperdeck.spec.ts
- packages/timeline-state-resolver/src/integrations/casparCG/index.ts
- packages/timeline-state-resolver-types/src/integrations/quantel.ts
- packages/timeline-state-resolver/src/integrations/httpWatcher/tests/httpwatcher.spec.ts
Comment on lines
+208
to
+215
| const deviceName = 'SofieChef' | ||
|
|
||
| if (!this.connected) { | ||
| statusCode = StatusCode.BAD | ||
| messages.push('Not connected') | ||
| statusDetails.push( | ||
| createSofieChefStatusDetail(SofieChefStatusCode.NOT_CONNECTED, { | ||
| deviceName, | ||
| }) |
There was a problem hiding this comment.
Use runtime device name instead of a hardcoded literal.
Line 208 uses 'SofieChef', which can mislabel status context when the configured device name differs.
🔧 Proposed fix
- const deviceName = 'SofieChef'
+ const deviceName = this.context.deviceName📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const deviceName = 'SofieChef' | |
| if (!this.connected) { | |
| statusCode = StatusCode.BAD | |
| messages.push('Not connected') | |
| statusDetails.push( | |
| createSofieChefStatusDetail(SofieChefStatusCode.NOT_CONNECTED, { | |
| deviceName, | |
| }) | |
| const deviceName = this.context.deviceName | |
| if (!this.connected) { | |
| statusCode = StatusCode.BAD | |
| statusDetails.push( | |
| createSofieChefStatusDetail(SofieChefStatusCode.NOT_CONNECTED, { | |
| deviceName, | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/timeline-state-resolver/src/integrations/sofieChef/index.ts` around
lines 208 - 215, The status block currently hardcodes deviceName as 'SofieChef';
change it to use the runtime/configured device name (e.g. the instance property
or config field used by this integration) so status messages reflect the actual
device. Locate the block around the connected check (references: this.connected,
statusCode, statusDetails, createSofieChefStatusDetail,
SofieChefStatusCode.NOT_CONNECTED) and replace the literal deviceName with the
appropriate runtime value (for example this.deviceName or the config's
deviceName property) so the object passed to createSofieChefStatusDetail
contains the real device name.
Comment on lines
+31
to
+45
| function normaliseDeviceStatus(input: DeviceStatusInput, active: boolean): DeviceStatus { | ||
| if ('statusDetails' in input) { | ||
| return { | ||
| statusCode: input.statusCode, | ||
| messages: input.statusDetails.map((d) => d.message), | ||
| statusDetails: input.statusDetails, | ||
| active, | ||
| } | ||
| } | ||
| return { | ||
| statusCode: input.statusCode, | ||
| messages: input.messages, | ||
| statusDetails: input.messages.map((message) => ({ message })), | ||
| active, | ||
| } |
There was a problem hiding this comment.
Keep statusDetails undefined for legacy devices.
The fallback branch currently fabricates statusDetails from messages, so callers can no longer distinguish legacy devices from integrations that actually support structured details. That breaks the feature-detection contract at both getStatus() and connectionChanged().
Proposed fix
function normaliseDeviceStatus(input: DeviceStatusInput, active: boolean): DeviceStatus {
if ('statusDetails' in input) {
return {
statusCode: input.statusCode,
messages: input.statusDetails.map((d) => d.message),
statusDetails: input.statusDetails,
active,
}
}
return {
statusCode: input.statusCode,
messages: input.messages,
- statusDetails: input.messages.map((message) => ({ message })),
+ statusDetails: undefined,
active,
}
}Please also cover the undefined vs [] distinction in packages/timeline-state-resolver/src/service/__tests__/deviceInstance.spec.ts.
Based on learnings: In the DeviceStatus interface within timeline-state-resolver, treat errors as a feature-detection mechanism: errors: [] means structured errors are supported, while errors: undefined means structured errors are not supported and consumers should fall back to the plain text messages array. Use this to guide reviews of interfaces and serialization logic, ensuring code handles both cases and that tests cover empty-array vs undefined scenarios for backward compatibility.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function normaliseDeviceStatus(input: DeviceStatusInput, active: boolean): DeviceStatus { | |
| if ('statusDetails' in input) { | |
| return { | |
| statusCode: input.statusCode, | |
| messages: input.statusDetails.map((d) => d.message), | |
| statusDetails: input.statusDetails, | |
| active, | |
| } | |
| } | |
| return { | |
| statusCode: input.statusCode, | |
| messages: input.messages, | |
| statusDetails: input.messages.map((message) => ({ message })), | |
| active, | |
| } | |
| function normaliseDeviceStatus(input: DeviceStatusInput, active: boolean): DeviceStatus { | |
| if ('statusDetails' in input) { | |
| return { | |
| statusCode: input.statusCode, | |
| messages: input.statusDetails.map((d) => d.message), | |
| statusDetails: input.statusDetails, | |
| active, | |
| } | |
| } | |
| return { | |
| statusCode: input.statusCode, | |
| messages: input.messages, | |
| statusDetails: undefined, | |
| active, | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/timeline-state-resolver/src/service/DeviceInstance.ts` around lines
31 - 45, normaliseDeviceStatus currently fabricates statusDetails from messages
for legacy devices; change it so that when input lacks statusDetails you return
statusDetails: undefined (not an array) so callers can detect legacy
devices—update the function normaliseDeviceStatus to copy statusDetails when
present and leave statusDetails undefined otherwise, preserving messages and
active; also update tests in deviceInstance.spec.ts to add assertions for both
cases (statusDetails undefined vs statusDetails === [] for integrations that
support structured errors) to cover the empty-array vs undefined distinction and
ensure consumers fall back to messages when statusDetails is undefined.
PanasonicPtzDevice, OscDevice, SofieChefDevice and TelemetricsDevice are now exported consistently alongside the other device classes.
Adds the DeviceStatusDetail interface for structured status messages with an optional typed status code and context. Adds DeviceStatusInput as a union of the legacy messages[] format and the new statusDetails[] format, allowing devices to adopt the new format incrementally. DeviceContextAPI.deviceName is also added so devices can include their name in generated status messages.
Simplifies the template interpolation to use replaceAll with a regex, making the implementation more direct. Updates the signature to use Record<string, any> instead of a plain object type. The args parameter defaults to an empty object when undefined.
Adds normaliseDeviceStatus() which converts the DeviceStatusInput union (legacy messages[] or new statusDetails[]) into a full DeviceStatus with both fields populated. Conversion is applied at the two points where status crosses into the conductor: DeviceInstance.getStatus() and the connectionChanged handler in the device context API. The abstract device base class and DeviceInstance are updated to reflect the DeviceStatusInput return type from device implementations.
Updates getStatus() return type from Omit<DeviceStatus, 'active'> to DeviceStatusInput for kairos, multiOsc, singularLive, tcpSend, udpSend, viscaOverIP, ograf and httpSend. Also adds deviceName to the shared test device context mock.
rjmunro
force-pushed
the
rjmunro/error-message-customisation
branch
from
61ea489 to
de8eb8b
Compare
|
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
packages/timeline-state-resolver/src/service/DeviceInstance.ts (1)
30-46:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDo not fabricate
statusDetailsfor legacymessagesinput.The legacy branch currently creates synthetic details from
messages, which removes theundefinedvs[]feature-detection signal for downstream consumers.Proposed fix
return { statusCode: input.statusCode, messages: input.messages, - statusDetails: input.messages.map((message) => ({ message })), + statusDetails: undefined, active, }Based on learnings: In timeline-state-resolver status handling,
statusDetails: []means structured support exists, whilestatusDetails: undefinedmeans consumers should fall back to plainmessages.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/timeline-state-resolver/src/service/DeviceInstance.ts` around lines 30 - 46, The legacy branch of normaliseDeviceStatus is incorrectly fabricating statusDetails from messages; update the function normaliseDeviceStatus so that when DeviceStatusInput does not contain statusDetails you copy statusCode and messages as-is but leave statusDetails as undefined (do not map messages into statusDetails), ensuring the returned DeviceStatus preserves the distinction between undefined and [] for downstream consumers; adjust any type signatures if needed so DeviceStatus.statusDetails can be undefined.packages/timeline-state-resolver/src/integrations/httpWatcher/index.ts (1)
59-68:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftAvoid emitting raw response bodies and headers in status context.
The catch block attaches potentially sensitive and unbounded payloads (
error.response.body, and inhandleResponsetheresponse.headers) into status details. This can:
- Leak sensitive headers (Authorization, Cookie, Set-Cookie)
- Include unbounded response bodies in status objects
Consider redacting sensitive headers and capping body size before attaching to context.
🔧 Suggested hardening
+const SENSITIVE_HEADERS = new Set(['authorization', 'cookie', 'set-cookie', 'proxy-authorization']) +const MAX_BODY_PREVIEW = 2048 + +function sanitizeHeaders(headers: Record<string, unknown> | undefined): Record<string, unknown> | undefined { + if (!headers) return undefined + const out: Record<string, unknown> = {} + for (const [k, v] of Object.entries(headers)) { + out[k] = SENSITIVE_HEADERS.has(k.toLowerCase()) ? '[REDACTED]' : v + } + return out +} + +function bodyPreview(body: unknown): string | undefined { + if (typeof body !== 'string') return undefined + return body.length > MAX_BODY_PREVIEW ? `${body.slice(0, MAX_BODY_PREVIEW)}…` : body +}Then use
bodyPreview(error.response.body)andsanitizeHeaders(response.headers)in the status detail contexts.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/timeline-state-resolver/src/integrations/httpWatcher/index.ts` around lines 59 - 68, The catch block and handleResponse are currently attaching raw error.response.body and response.headers into the status context (via createHTTPWatcherStatusDetail), which can leak sensitive headers and unbounded bodies; update the code paths that build the context (the catch handler around the HTTP request and the handleResponse code that uses response.headers) to replace direct bodies with a capped preview using bodyPreview(error.response.body) or bodyPreview(response.body) and to sanitize headers using sanitizeHeaders(response.headers) (or error.response.headers) before adding them to the context, then pass the sanitized headers and bodyPreview into createHTTPWatcherStatusDetail instead of the raw values.packages/timeline-state-resolver-types/src/integrations/websocketClient.ts (1)
42-45:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winOptional context fields will render as literal
"undefined"in messages.The context fields
uri,reason,error, andstatusCodeare all optional inWebSocketClientStatusContextMap. WheninterpolateTemplateStringencountersundefinedvalues, JavaScript's string replacement will coerce them to the literal string"undefined", producing messages like:
"WS Disconnected: undefined (undefined)""WS Connection failed to undefined: undefined"💡 Option A — Guard in the template helper (preferred, applies globally)
In
interpolateTemplateString, coerceundefinedvalues to empty string:const safeContext = Object.fromEntries( Object.entries(context ?? {}).map(([k, v]) => [k, v ?? '']) )💡 Option B — Make templates resilient
- [WebSocketClientStatusCode.NOT_CONNECTED]: 'WS Disconnected: {{uri}} ({{reason}})', - [WebSocketClientStatusCode.CONNECTION_FAILED]: 'WS Connection failed to {{uri}}: {{error}}', + [WebSocketClientStatusCode.NOT_CONNECTED]: 'WS Disconnected', + [WebSocketClientStatusCode.CONNECTION_FAILED]: 'WS Connection failed',🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/timeline-state-resolver-types/src/integrations/websocketClient.ts` around lines 42 - 45, The status message templates render literal "undefined" when optional context fields are missing; update the template interpolation helper (interpolateTemplateString) to coerce undefined/null context values to an empty string before replacement (e.g., build a safeContext from WebSocketClientStatusContextMap entries mapping v => v ?? ''), so WebSocketClientStatusMessages like 'WS Disconnected: {{uri}} ({{reason}})' and 'WS Connection failed to {{uri}}: {{error}}' produce clean output when fields are absent.
🧹 Nitpick comments (1)
packages/timeline-state-resolver/src/integrations/hyperdeck/index.ts (1)
258-336: ⚡ Quick winRefactor
getStatus()to reduce cognitive complexity (Sonar failure).Line 258 currently packs connection, recording-time, slot, and transport checks in one method; splitting these into small helpers should bring complexity below the threshold and improve maintainability.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/timeline-state-resolver/src/integrations/hyperdeck/index.ts` around lines 258 - 336, Split getStatus() into small helper methods: e.g., private checkConnectionStatus(): {statusCode: StatusCode, details: DeviceStatusDetail[]}, private checkRecordingTimeStatus(), private checkSlotStatus(), and private checkTransportStatus(); each should inspect the same fields used today (_connected, _initOptions, _minRecordingTime, _recordingTime, _slotCount, _slotStatus, _suppressEmptySlotWarnings, _expectedTransportStatus, _transportStatus) and return a statusCode plus statusDetails created with createHyperdeckStatusDetail/HypedeckStatusCode as currently used; then simplify getStatus() to call these helpers, merge their statusDetails arrays, and compute the overall statusCode by taking the highest-severity code (e.g., max value) among GOOD, WARNING_MINOR/MAJOR, BAD to preserve current logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/timeline-state-resolver/src/integrations/telemetrics/index.ts`:
- Line 53: The status context currently hardcodes deviceName as 'Telemetrics'
(variable deviceName) which breaks when a device is renamed; update the
assignment in the Telemetrics integration to use the runtime value
this.context.deviceName (or fall back to a sensible default) so the
status/context reflects the actual configured device name for methods that
reference deviceName.
In `@packages/timeline-state-resolver/src/integrations/websocketClient/index.ts`:
- Around line 77-79: getStatus() currently returns StatusCode.BAD with an empty
statusDetails array when disconnected, which normalizes to no human-readable
messages; update the fallback in getStatus (the method named getStatus in the
websocket client) to include a statusDetails entry with a clear message (e.g.,
"disconnected" or "no connection") so consumers receive a readable reason when
connection?.connectionStatus() is null/undefined instead of an empty messages
array.
---
Duplicate comments:
In `@packages/timeline-state-resolver-types/src/integrations/websocketClient.ts`:
- Around line 42-45: The status message templates render literal "undefined"
when optional context fields are missing; update the template interpolation
helper (interpolateTemplateString) to coerce undefined/null context values to an
empty string before replacement (e.g., build a safeContext from
WebSocketClientStatusContextMap entries mapping v => v ?? ''), so
WebSocketClientStatusMessages like 'WS Disconnected: {{uri}} ({{reason}})' and
'WS Connection failed to {{uri}}: {{error}}' produce clean output when fields
are absent.
In `@packages/timeline-state-resolver/src/integrations/httpWatcher/index.ts`:
- Around line 59-68: The catch block and handleResponse are currently attaching
raw error.response.body and response.headers into the status context (via
createHTTPWatcherStatusDetail), which can leak sensitive headers and unbounded
bodies; update the code paths that build the context (the catch handler around
the HTTP request and the handleResponse code that uses response.headers) to
replace direct bodies with a capped preview using
bodyPreview(error.response.body) or bodyPreview(response.body) and to sanitize
headers using sanitizeHeaders(response.headers) (or error.response.headers)
before adding them to the context, then pass the sanitized headers and
bodyPreview into createHTTPWatcherStatusDetail instead of the raw values.
In `@packages/timeline-state-resolver/src/service/DeviceInstance.ts`:
- Around line 30-46: The legacy branch of normaliseDeviceStatus is incorrectly
fabricating statusDetails from messages; update the function
normaliseDeviceStatus so that when DeviceStatusInput does not contain
statusDetails you copy statusCode and messages as-is but leave statusDetails as
undefined (do not map messages into statusDetails), ensuring the returned
DeviceStatus preserves the distinction between undefined and [] for downstream
consumers; adjust any type signatures if needed so DeviceStatus.statusDetails
can be undefined.
---
Nitpick comments:
In `@packages/timeline-state-resolver/src/integrations/hyperdeck/index.ts`:
- Around line 258-336: Split getStatus() into small helper methods: e.g.,
private checkConnectionStatus(): {statusCode: StatusCode, details:
DeviceStatusDetail[]}, private checkRecordingTimeStatus(), private
checkSlotStatus(), and private checkTransportStatus(); each should inspect the
same fields used today (_connected, _initOptions, _minRecordingTime,
_recordingTime, _slotCount, _slotStatus, _suppressEmptySlotWarnings,
_expectedTransportStatus, _transportStatus) and return a statusCode plus
statusDetails created with createHyperdeckStatusDetail/HypedeckStatusCode as
currently used; then simplify getStatus() to call these helpers, merge their
statusDetails arrays, and compute the overall statusCode by taking the
highest-severity code (e.g., max value) among GOOD, WARNING_MINOR/MAJOR, BAD to
preserve current logic.
🪄 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: 78ddcbb6-3974-4914-a78e-3234c82be44d
⛔ 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 (82)
packages/timeline-state-resolver-api/src/device.tspackages/timeline-state-resolver-api/src/tsr-api.tspackages/timeline-state-resolver-types/src/__tests__/templateString.spec.tspackages/timeline-state-resolver-types/src/device.tspackages/timeline-state-resolver-types/src/deviceStatusDetail.tspackages/timeline-state-resolver-types/src/index.tspackages/timeline-state-resolver-types/src/integrations/atem.tspackages/timeline-state-resolver-types/src/integrations/casparcg.tspackages/timeline-state-resolver-types/src/integrations/httpWatcher.tspackages/timeline-state-resolver-types/src/integrations/hyperdeck.tspackages/timeline-state-resolver-types/src/integrations/lawo.tspackages/timeline-state-resolver-types/src/integrations/obs.tspackages/timeline-state-resolver-types/src/integrations/osc.tspackages/timeline-state-resolver-types/src/integrations/panasonicPTZ.tspackages/timeline-state-resolver-types/src/integrations/pharos.tspackages/timeline-state-resolver-types/src/integrations/quantel.tspackages/timeline-state-resolver-types/src/integrations/shotoku.tspackages/timeline-state-resolver-types/src/integrations/sisyfos.tspackages/timeline-state-resolver-types/src/integrations/sofieChef.tspackages/timeline-state-resolver-types/src/integrations/telemetrics.tspackages/timeline-state-resolver-types/src/integrations/tricaster.tspackages/timeline-state-resolver-types/src/integrations/vizMSE.tspackages/timeline-state-resolver-types/src/integrations/vmix.tspackages/timeline-state-resolver-types/src/integrations/websocketClient.tspackages/timeline-state-resolver-types/src/templateString.tspackages/timeline-state-resolver/src/devices/device.tspackages/timeline-state-resolver/src/index.tspackages/timeline-state-resolver/src/integrations/__tests__/testlib.tspackages/timeline-state-resolver/src/integrations/abstract/index.tspackages/timeline-state-resolver/src/integrations/atem/__tests__/atem.spec.tspackages/timeline-state-resolver/src/integrations/atem/index.tspackages/timeline-state-resolver/src/integrations/atem/messages.tspackages/timeline-state-resolver/src/integrations/casparCG/index.tspackages/timeline-state-resolver/src/integrations/casparCG/messages.tspackages/timeline-state-resolver/src/integrations/httpSend/index.tspackages/timeline-state-resolver/src/integrations/httpWatcher/__tests__/httpwatcher.spec.tspackages/timeline-state-resolver/src/integrations/httpWatcher/index.tspackages/timeline-state-resolver/src/integrations/httpWatcher/messages.tspackages/timeline-state-resolver/src/integrations/hyperdeck/__tests__/hyperdeck.spec.tspackages/timeline-state-resolver/src/integrations/hyperdeck/index.tspackages/timeline-state-resolver/src/integrations/hyperdeck/messages.tspackages/timeline-state-resolver/src/integrations/kairos/index.tspackages/timeline-state-resolver/src/integrations/lawo/index.tspackages/timeline-state-resolver/src/integrations/lawo/messages.tspackages/timeline-state-resolver/src/integrations/multiOsc/index.tspackages/timeline-state-resolver/src/integrations/obs/index.tspackages/timeline-state-resolver/src/integrations/obs/messages.tspackages/timeline-state-resolver/src/integrations/ograf/index.tspackages/timeline-state-resolver/src/integrations/osc/index.tspackages/timeline-state-resolver/src/integrations/osc/messages.tspackages/timeline-state-resolver/src/integrations/panasonicPTZ/index.tspackages/timeline-state-resolver/src/integrations/panasonicPTZ/messages.tspackages/timeline-state-resolver/src/integrations/pharos/index.tspackages/timeline-state-resolver/src/integrations/pharos/messages.tspackages/timeline-state-resolver/src/integrations/quantel/index.tspackages/timeline-state-resolver/src/integrations/quantel/messages.tspackages/timeline-state-resolver/src/integrations/shotoku/index.tspackages/timeline-state-resolver/src/integrations/shotoku/messages.tspackages/timeline-state-resolver/src/integrations/singularLive/index.tspackages/timeline-state-resolver/src/integrations/sisyfos/index.tspackages/timeline-state-resolver/src/integrations/sisyfos/messages.tspackages/timeline-state-resolver/src/integrations/sofieChef/__tests__/sofieChef.spec.tspackages/timeline-state-resolver/src/integrations/sofieChef/index.tspackages/timeline-state-resolver/src/integrations/sofieChef/messages.tspackages/timeline-state-resolver/src/integrations/tcpSend/index.tspackages/timeline-state-resolver/src/integrations/telemetrics/__tests__/telemetrics.spec.tspackages/timeline-state-resolver/src/integrations/telemetrics/index.tspackages/timeline-state-resolver/src/integrations/telemetrics/messages.tspackages/timeline-state-resolver/src/integrations/tricaster/index.tspackages/timeline-state-resolver/src/integrations/tricaster/messages.tspackages/timeline-state-resolver/src/integrations/udpSend/index.tspackages/timeline-state-resolver/src/integrations/viscaOverIP/index.tspackages/timeline-state-resolver/src/integrations/vizMSE/index.tspackages/timeline-state-resolver/src/integrations/vizMSE/messages.tspackages/timeline-state-resolver/src/integrations/vmix/index.tspackages/timeline-state-resolver/src/integrations/vmix/messages.tspackages/timeline-state-resolver/src/integrations/websocketClient/__tests__/websocketClient.spec.tspackages/timeline-state-resolver/src/integrations/websocketClient/connection.tspackages/timeline-state-resolver/src/integrations/websocketClient/index.tspackages/timeline-state-resolver/src/integrations/websocketClient/messages.tspackages/timeline-state-resolver/src/service/DeviceInstance.tspackages/timeline-state-resolver/src/service/__tests__/deviceInstance.spec.ts
✅ Files skipped from review due to trivial changes (18)
- packages/timeline-state-resolver-types/src/index.ts
- packages/timeline-state-resolver/src/integrations/websocketClient/messages.ts
- packages/timeline-state-resolver/src/integrations/sofieChef/messages.ts
- packages/timeline-state-resolver/src/index.ts
- packages/timeline-state-resolver/src/integrations/singularLive/index.ts
- packages/timeline-state-resolver/src/integrations/httpWatcher/messages.ts
- packages/timeline-state-resolver-types/src/integrations/httpWatcher.ts
- packages/timeline-state-resolver-types/src/integrations/osc.ts
- packages/timeline-state-resolver/src/integrations/sisyfos/messages.ts
- packages/timeline-state-resolver-api/src/device.ts
- packages/timeline-state-resolver-types/src/integrations/panasonicPTZ.ts
- packages/timeline-state-resolver-types/src/integrations/pharos.ts
- packages/timeline-state-resolver-types/src/integrations/lawo.ts
- packages/timeline-state-resolver-types/src/integrations/shotoku.ts
- packages/timeline-state-resolver/src/integrations/tests/testlib.ts
- packages/timeline-state-resolver/src/devices/device.ts
- packages/timeline-state-resolver-types/src/integrations/vizMSE.ts
- packages/timeline-state-resolver/src/integrations/tcpSend/index.ts
🚧 Files skipped from review as they are similar to previous changes (37)
- packages/timeline-state-resolver/src/service/tests/deviceInstance.spec.ts
- packages/timeline-state-resolver/src/integrations/udpSend/index.ts
- packages/timeline-state-resolver-api/src/tsr-api.ts
- packages/timeline-state-resolver/src/integrations/atem/messages.ts
- packages/timeline-state-resolver/src/integrations/vmix/messages.ts
- packages/timeline-state-resolver/src/integrations/quantel/messages.ts
- packages/timeline-state-resolver/src/integrations/kairos/index.ts
- packages/timeline-state-resolver/src/integrations/vizMSE/messages.ts
- packages/timeline-state-resolver/src/integrations/telemetrics/messages.ts
- packages/timeline-state-resolver/src/integrations/osc/messages.ts
- packages/timeline-state-resolver-types/src/tests/templateString.spec.ts
- packages/timeline-state-resolver/src/integrations/lawo/messages.ts
- packages/timeline-state-resolver/src/integrations/panasonicPTZ/messages.ts
- packages/timeline-state-resolver/src/integrations/obs/messages.ts
- packages/timeline-state-resolver/src/integrations/tricaster/messages.ts
- packages/timeline-state-resolver/src/integrations/sofieChef/tests/sofieChef.spec.ts
- packages/timeline-state-resolver/src/integrations/pharos/messages.ts
- packages/timeline-state-resolver/src/integrations/casparCG/messages.ts
- packages/timeline-state-resolver/src/integrations/abstract/index.ts
- packages/timeline-state-resolver/src/integrations/hyperdeck/messages.ts
- packages/timeline-state-resolver-types/src/integrations/atem.ts
- packages/timeline-state-resolver/src/integrations/panasonicPTZ/index.ts
- packages/timeline-state-resolver/src/integrations/tricaster/index.ts
- packages/timeline-state-resolver/src/integrations/osc/index.ts
- packages/timeline-state-resolver-types/src/templateString.ts
- packages/timeline-state-resolver-types/src/integrations/sofieChef.ts
- packages/timeline-state-resolver/src/integrations/lawo/index.ts
- packages/timeline-state-resolver/src/integrations/websocketClient/tests/websocketClient.spec.ts
- packages/timeline-state-resolver/src/integrations/httpSend/index.ts
- packages/timeline-state-resolver/src/integrations/telemetrics/tests/telemetrics.spec.ts
- packages/timeline-state-resolver-types/src/integrations/obs.ts
- packages/timeline-state-resolver/src/integrations/shotoku/messages.ts
- packages/timeline-state-resolver/src/integrations/atem/tests/atem.spec.ts
- packages/timeline-state-resolver-types/src/integrations/quantel.ts
- packages/timeline-state-resolver/src/integrations/hyperdeck/tests/hyperdeck.spec.ts
- packages/timeline-state-resolver/src/integrations/sisyfos/index.ts
- packages/timeline-state-resolver/src/integrations/httpWatcher/tests/httpwatcher.spec.ts
| const messages: string[] = [] | ||
| getStatus(): DeviceStatusInput { | ||
| const statusDetails: DeviceStatusDetail[] = [] | ||
| const deviceName = 'Telemetrics' |
There was a problem hiding this comment.
Use runtime device name in Telemetrics status context.
Line 53 hardcodes 'Telemetrics', which can produce incorrect context when the device is renamed/configured differently. Use this.context.deviceName instead.
Suggested fix
- const deviceName = 'Telemetrics'
+ const deviceName = this.context.deviceName📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const deviceName = 'Telemetrics' | |
| const deviceName = this.context.deviceName |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/timeline-state-resolver/src/integrations/telemetrics/index.ts` at
line 53, The status context currently hardcodes deviceName as 'Telemetrics'
(variable deviceName) which breaks when a device is renamed; update the
assignment in the Telemetrics integration to use the runtime value
this.context.deviceName (or fall back to a sensible default) so the
status/context reflects the actual configured device name for methods that
reference deviceName.
Comment on lines
+77
to
79
| public getStatus(): DeviceStatusInput { | ||
| return this.connection?.connectionStatus() ?? { statusCode: StatusCode.BAD, statusDetails: [] } | ||
| } |
There was a problem hiding this comment.
Restore a human-readable fallback for disconnected status.
StatusCode.BAD with statusDetails: [] normalizes to empty messages[], so consumers get no reason text. Please keep a fallback message in this branch.
Proposed fix
- return this.connection?.connectionStatus() ?? { statusCode: StatusCode.BAD, statusDetails: [] }
+ return this.connection?.connectionStatus() ?? {
+ statusCode: StatusCode.BAD,
+ statusDetails: [{ message: 'No Connection' }],
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/timeline-state-resolver/src/integrations/websocketClient/index.ts`
around lines 77 - 79, getStatus() currently returns StatusCode.BAD with an empty
statusDetails array when disconnected, which normalizes to no human-readable
messages; update the fallback in getStatus (the method named getStatus in the
websocket client) to include a statusDetails entry with a clear message (e.g.,
"disconnected" or "no connection") so consumers receive a readable reason when
connection?.connectionStatus() is null/undefined instead of an empty messages
array.
Member
|
I've read through the PR, nothing jumps out to me, however I'm not too deep into TSR yet. |
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
6 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.
Current Behavior
Errors are reported as plain strings. This makes them hard to understand in an machine readable way by e.g. sofie blueprints or translate.
New Behavior
Errors will include error codes and context as well as the message for backwards compatibility.
Testing Instructions
Other Information
Status