Fix/screen error reporting - #3816
Open
jmthomas wants to merge 3 commits into
Open
Conversation
Widgets reported errors four different ways, and ArrayplotWidget pushed to a `this.errors` array that never existed, so a lost stream connection died with a TypeError instead of a message. Give Widget.js one pair of helpers - screenError() to throw for definition errors, emitScreenError() to emit for errors after the widget is built (errorCaptured can't see async throws) - and route every widget and Openc3Screen site through a single addError(). Also fixes errors that were reported but never seen: rerender() cleared the list after the new widgets had filled it, parseDefinition() threw outside any errorCaptured context, and graph items were never checked for existence because they don't emit addItem. Re-parsing now resets actualScreenItems so a screen with no polled items stops polling the previous definition's. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Widgets reported errors four different ways, and ArrayplotWidget pushed to a `this.errors` array that never existed, so a lost stream connection died with a TypeError instead of a message. Give Widget.js one pair of helpers - screenError() to throw for definition errors, emitScreenError() to emit for errors after the widget is built (errorCaptured can't see async throws) - and route every widget and Openc3Screen site through a single addError(). Also fixes errors that were reported but never seen: rerender() cleared the list after the new widgets had filled it, parseDefinition() threw outside any errorCaptured context, and graph items were never checked for existence because they don't emit addItem. Re-parsing now resets actualScreenItems so a screen with no polled items stops polling the previous definition's. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…3/cosmos into fix/screen-error-reporting
jmthomas
requested review from
EmilyRagan and
ryan-pratt
and
a lite review from Copilot
September 2, 2026 23:47
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3816 +/- ##
==========================================
- Coverage 79.19% 79.19% -0.01%
==========================================
Files 894 894
Lines 67034 67077 +43
Branches 2553 2566 +13
==========================================
+ Hits 53087 53120 +33
- Misses 13284 13297 +13
+ Partials 663 660 -3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
There are a couple of confirmed edge-case bugs in the updated Openc3Screen error/reporting and tlm-availability in-flight handling that can reintroduce “(undefined)” error formatting and apply stale responses when item sets change but lengths match.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR consolidates and hardens TlmViewer screen/widget error reporting so definition-time errors are attributed to the correct screen-definition line, and runtime/async errors (e.g., streaming disconnects) reliably surface at the screen level instead of being lost or causing unrelated failures.
Changes:
- Centralized screen-definition error creation (
screenError) and async/runtime error propagation (emitScreenError) via new widget helpers and screen-leveladdError. - Added explicit item-existence checks for streaming graph widgets (so typos become visible screen errors instead of silent empty series).
- Tightened screen parsing robustness (extra
ENDnow reports a targeted definition error; screen re-parse resets polling state to avoid stale item polling).
File summaries
| File | Description |
|---|---|
| openc3-cosmos-init/plugins/packages/openc3-vue-common/src/widgets/Widget.js | Adds screenError, emitScreenError, and checkScreenItems; uses screenError in param verification. |
| openc3-cosmos-init/plugins/packages/openc3-vue-common/src/widgets/TitleWidget.vue | Enforces TITLE <Text> with verifyNumParams (breaking-change behavior now explicit). |
| openc3-cosmos-init/plugins/packages/openc3-vue-common/src/widgets/LinegraphWidget.vue | Bubbles embedded Graph errors to the screen via emitScreenError. |
| openc3-cosmos-init/plugins/packages/openc3-vue-common/src/widgets/LimitscolumnWidget.vue | Switches ad-hoc thrown object to structured screenError. |
| openc3-cosmos-init/plugins/packages/openc3-vue-common/src/widgets/LimitsbarWidget.vue | Switches ad-hoc thrown object to structured screenError. |
| openc3-cosmos-init/plugins/packages/openc3-vue-common/src/widgets/GraphWidget.js | Avoids seeding undefined items; requests screen-level existence validation for graph items. |
| openc3-cosmos-init/plugins/packages/openc3-vue-common/src/widgets/DynamicWidget.vue | Reports unknown widget load failures as structured screenError for proper attribution. |
| openc3-cosmos-init/plugins/packages/openc3-vue-common/src/widgets/ArrayplotWidget.vue | Replaces direct error-list mutation with screen-bubbled emitScreenError; guards __time axis generation when no series exist. |
| openc3-cosmos-init/plugins/packages/openc3-vue-common/src/components/Openc3Screen.vue | Centralizes error ingestion (addError), catches parse-time throws, prevents extra END stack underflow, and adds item-existence checks for streaming graphs. |
| openc3-cosmos-init/plugins/packages/openc3-vue-common/src/components/Graph.vue | Adds explicit error emit and forwards internal errors upward for embedded screen display. |
| openc3-cosmos-init/plugins/packages/openc3-vue-common/src/components/EditScreenDialog.vue | Dedupes displayed errors and avoids prepending location info when no definition line exists. |
| openc3-cosmos-init/plugins/packages/openc3-js-common/src/services/cable.js | Suppresses disconnected callbacks caused by explicit unsubscribe/unmount from being treated as connection problems. |
| openc3-cosmos-init/plugins/packages/openc3-cosmos-demo/targets/INST2/screens/other.txt | Updates demo screen TITLE usage to single-parameter form. |
| openc3-cosmos-init/plugins/packages/openc3-cosmos-demo/targets/INST2/screens/adcs.txt | Updates demo screen TITLE usage to single-parameter form. |
| openc3-cosmos-init/plugins/packages/openc3-cosmos-demo/targets/INST/screens/other.txt | Updates demo screen TITLE usage to single-parameter form. |
| openc3-cosmos-init/plugins/packages/openc3-cosmos-demo/targets/INST/screens/adcs.txt | Updates demo screen TITLE usage to single-parameter form. |
Review details
Suppressed comments (1)
openc3-cosmos-init/plugins/packages/openc3-vue-common/src/components/Openc3Screen.vue:1192
- In the
get_tlm_availablecatch path, the fallback reset ofactualScreenItemsis guarded only byrequestedItems.length === this.screenItems.length. If a re-parse swaps items but preserves length, this can restore polling for the wrong set/order of items.
.catch((error) => {
console.error('Error getting tlm available', error)
if (requestedItems.length === this.screenItems.length) {
this.actualScreenItems = [...this.screenItems]
}
- Files reviewed: 16/16 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+610
to
+621
| let names = [] | ||
| let lines = [] | ||
| for (const widget of this.dynamicWidgets) { | ||
| names.push(widget.name) | ||
| lines.push(widget.lineNumber) | ||
| } | ||
| // Warn about any of the Dynamic widgets we found .. they could be typos | ||
| this.addError({ | ||
| type: 'usage', | ||
| message: `Unknown widget! Are these widgets: ${names.join(',')}?`, | ||
| lineNumber: lines.join(','), | ||
| }) |
Comment on lines
+1141
to
+1149
| // The screen can be re-parsed while this is in flight, which leaves the | ||
| // response describing items we no longer have | ||
| const requestedItems = [...this.screenItems] | ||
| this.api | ||
| .get_tlm_available(this.screenItems, {}, { 'Ignore-Errors': '403' }) | ||
| .get_tlm_available(requestedItems, {}, { 'Ignore-Errors': '403' }) | ||
| .then((data) => { | ||
| if (requestedItems.length !== this.screenItems.length) { | ||
| return | ||
| } |
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
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.
What changed
Consolidated the error reporting in the TlmViewer widget chain. Some widgets now emit errors that are rescued at the screen level.
LINEGRAPHnow confirms that all the items exist instead of just not graphing.TITLEnow enforces a single parameter which will error a screen and require a user fix but has been the case since COSMOS 5.0. ExtraneousENDstatements now error a screen and require a user fix but this helps expose weird rendering errors.Why it changed
Customer reported
At undefined: (undefined) TypeError: Cannot read properties of undefined (reading 'push').I think this was due to the Graph widget having an invalid item.Testing strategy
Went through all screens and looked for render errors and console log errors. All resolved successfully.