feat: Error message customisation#1604
feat: Error message customisation#1604rjmunro wants to merge 8 commits intoSofie-Automation:mainfrom
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the
WalkthroughThis PR introduces a comprehensive error message customization system enabling blueprints to define custom messages for device, blueprint, and system errors. The changes span from new type definitions in shared libraries to integration points in the UI layer, allowing error messages to be resolved through a layered approach combining dynamic resolvers, blueprint-provided overrides, and defaults. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
meteor/server/publications/showStyleUI.ts (1)
87-107: Missing observer for Blueprint changes.The publication fetches
deviceErrorMessagesfrom the Blueprint but doesn't observe the Blueprint collection for changes. If a blueprint'sdeviceErrorMessagesare updated while the show style remains unchanged, clients won't receive the updated messages until the show style itself triggers an update.Consider adding a Blueprint observer in
setupUIShowStyleBasePublicationObservers:♻️ Suggested addition to observers
async function setupUIShowStyleBasePublicationObservers( args: ReadonlyDeep<UIShowStyleBaseArgs>, triggerUpdate: TriggerUpdate<UIShowStyleBaseUpdateProps> ): Promise<SetupObserversResult> { + const showStyleBase = await ShowStyleBases.findOneAsync(args.showStyleBaseId, { + projection: { blueprintId: 1 }, + }) + // Set up observers: - return [ + const observers: SetupObserversResult = [ ShowStyleBases.observeChanges( args.showStyleBaseId, { added: () => triggerUpdate({ invalidateShowStyle: true }), changed: () => triggerUpdate({ invalidateShowStyle: true }), removed: () => triggerUpdate({ invalidateShowStyle: true }), }, { projection: fieldSpecifier, } ), ] + + if (showStyleBase?.blueprintId) { + observers.push( + Blueprints.observeChanges( + showStyleBase.blueprintId, + { + changed: () => triggerUpdate({ invalidateShowStyle: true }), + }, + { + projection: blueprintFieldSpecifier, + } + ) + ) + } + + return observers }
🧹 Nitpick comments (3)
packages/blueprints-integration/src/api/showStyle.ts (1)
112-116: Consider adding documentation parity withdeviceErrorMessages.The
deviceErrorMessagesfield has comprehensive JSDoc with examples, whileblueprintErrorMessagesandsystemErrorMessageshave minimal descriptions. For consistency and developer experience, consider adding similar detail about:
- Available error codes (or where to find them)
- Template interpolation syntax
- Example usage
📝 Suggested documentation enhancement
- /** Alternate blueprint error messages, to override the builtin ones produced by Sofie */ + /** + * Alternate blueprint error messages, to override the builtin ones produced by Sofie. + * Keys are BlueprintErrorCode values (e.g., 'MISSING_SEGMENT_DATA', 'VALIDATION_ERROR'). + * + * Templates use {{variable}} syntax for interpolation with context values. + * + * `@example` + * ```typescript + * blueprintErrorMessages: { + * [BlueprintErrorCode.VALIDATION_ERROR]: 'Config issue: {{reason}}', + * } + * ``` + */ blueprintErrorMessages?: Partial<Record<BlueprintErrorCode | string, string | undefined>> - /** Alternate system error messages, to override the builtin ones produced by Sofie */ + /** + * Alternate system error messages, to override the builtin ones produced by Sofie. + * Keys are SystemErrorCode values (e.g., 'DATABASE_CONNECTION_LOST', 'SERVICE_UNAVAILABLE'). + * + * Templates use {{variable}} syntax for interpolation with context values. + * + * `@example` + * ```typescript + * systemErrorMessages: { + * [SystemErrorCode.SERVICE_UNAVAILABLE]: '{{service}} is down: {{reason}}', + * } + * ``` + */ systemErrorMessages?: Partial<Record<SystemErrorCode | string, string | undefined>>packages/corelib/src/ErrorMessageResolver.ts (2)
194-211: Inconsistent control flow compared to sibling methods.The dynamic resolver check structure differs from
getDeviceErrorMessageandgetBlueprintErrorMessage. While functionally equivalent, this inconsistency reduces maintainability.In other methods:
if (dynamicMessage !== undefined) { if (dynamicMessage === '') { return null } return ... }In this method, the checks are inverted (empty string checked first, then
!== undefined).♻️ Suggested refactor for consistency
// 1. Check dynamic resolver function (if available) if (this.#resolverFunction) { const dynamicMessage = this.#resolverFunction({ errorCode: errorCode as SystemErrorCode, args, } as BlueprintErrorEvent) - if (dynamicMessage === '') { - // Empty string means suppress the message - return null - } - if (dynamicMessage !== undefined) { + if (dynamicMessage === '') { + // Empty string means suppress the message + return null + } // Resolver returned a custom message return this.#blueprint ? wrapTranslatableMessageFromBlueprints({ key: dynamicMessage, args }, [this.#blueprint._id]) : { key: dynamicMessage, args } } }
189-192: Minor: Preferunknownoveranyfor type safety.The
argsparameter usesanywhile the sibling methods useunknown. Usingunknownis safer and maintains consistency.🔧 Suggested fix
getSystemErrorMessage( errorCode: SystemErrorCode | string, - args: { [k: string]: any } + args: { [k: string]: unknown } ): ITranslatableMessage | null {
rjmunro
force-pushed
the
rjmunro/error-message-customisation
branch
from
85e60cd to
dc2efd1
Compare
rjmunro
force-pushed
the
rjmunro/error-message-customisation
branch
from
cc8edb8 to
d20435b
Compare
Add DeviceErrorContext, DeviceErrorMessageFunction, and deviceErrorMessages to StudioBlueprintManifest for customizing TSR device error messages. Add systemErrorMessages to SystemBlueprintManifest for customizing system error messages. Add ErrorMessageResolver class in corelib for runtime evaluation of blueprint error message customizations (supports string templates and functions). Add DeviceStatusError type to PeripheralDeviceStatusObject for structured error reporting from TSR devices.
When a peripheral device reports structured errors (status.errors), the server now evaluates the Studio blueprint's deviceErrorMessages at runtime to produce customized error messages. This ensures blueprint customization happens at ingest time, not on the client, providing consistent error messages across all clients.
Empty string will hide the message completely.
If it's a function, evaluate it then pass output along as if it were a the output directly to start with, so handle undefined, empty strings and context interpolation as normal.
New custom error messages should be self-contained - Custom blueprint messages should be written appropriately - Prefix Old TSR default messages earlier in the process for backwards compatibility
rjmunro
force-pushed
the
rjmunro/error-message-customisation
branch
from
d20435b to
288e4ef
Compare
|
Requires Sofie-Automation/sofie-timeline-state-resolver#418 to be merged first. |
1 participant
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 (localization).
Sofie-Automation/sofie-timeline-state-resolver#418
New Behavior
Errors will include error codes and context arguments.
Errors can be overridden by blueprints that will be able to either supply strings to be interpolated (now supporting custom string codes easily), or a function to generate a message given strictly typed data. This allows for complex logic like pluralisation or unwrapping errors from HTTP responses of custom devices.
Testing
Affected areas
resolveErrorMessagesignature for better type safety.BlueprintErrorContextsfor strict argument typing.Time Frame
Other Information
This enables safer and richer error handling in blueprints without needing boilerplate type guards.
Status