feat(weather): add display_errors setting#774
Conversation
- Add display_errors setting to screenly.yml and screenly_qc.yml - Call setupErrorHandling() in main.ts on DOMContentLoaded - Add display_errors to e2e screenshot mock settings - Document display_errors in README.md Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
PR Reviewer Guide 🔍(Review updated until commit 576e0ff)Here are some key observations to aid the review process:
|
PR Code Suggestions ✨No code suggestions found for the PR. |
There was a problem hiding this comment.
Pull request overview
Adds a display_errors setting to the Weather edge app and wires in the shared setupErrorHandling() hook so runtime failures can be surfaced via the panic-overlay mechanism (plus updates docs and screenshot-test mocks).
Changes:
- Added
display_errorstoscreenly.ymlandscreenly_qc.yml(marked as optional + advanced). - Invoked
setupErrorHandling()during app startup insrc/main.ts. - Documented
display_errorsin the app README and included it in the e2e screenshot mock settings.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| edge-apps/weather/src/main.ts | Calls shared error-handling setup on DOM ready. |
| edge-apps/weather/screenly.yml | Adds display_errors manifest setting. |
| edge-apps/weather/screenly_qc.yml | Adds display_errors manifest setting for QC. |
| edge-apps/weather/README.md | Documents the new setting in the configuration table. |
| edge-apps/weather/e2e/screenshots.spec.ts | Ensures screenshot mocks include display_errors. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| setupErrorHandling() | ||
|
|
There was a problem hiding this comment.
With setupErrorHandling() enabled, errors thrown during initialization will only surface via panic-overlay if they reach window.onerror/unhandledrejection. The current top-level try/catch logs and swallows initialization failures, which likely prevents the on-screen error overlay from ever appearing for those failures. Consider rethrowing after logging (or conditionally rethrowing when display_errors is enabled) so the overlay can render the error.
|
Persistent review updated to latest commit 576e0ff |
PR Code Suggestions ✨No code suggestions found for the PR. |
- Use unindented list style for categories - Move schema_version after properties in display_errors help_text Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Drop try/catch so unhandled errors propagate to panic-overlay - Move signalReady() to end of happy path Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
User description
Summary
display_errorssetting toscreenly.ymlandscreenly_qc.ymlsetupErrorHandling()inmain.tsto surface errors on-screen viapanic-overlaywhen enableddisplay_errorsto e2e screenshot mock settings and documents it inREADME.mdPR Type
Enhancement, Documentation, Tests
Description
Add
display_errorsapp settingEnable startup error overlay handling
Document debugging option in README
Include setting in screenshot mocks
Diagram Walkthrough
File Walkthrough
screenshots.spec.ts
Update screenshot mocks for new settingedge-apps/weather/e2e/screenshots.spec.ts
display_errorsto screenshot mock settingsmain.ts
Initialize error handling during app startupedge-apps/weather/src/main.ts
setupErrorHandlingfrom shared app packagesetupErrorHandling()onDOMContentLoadedREADME.md
Document new debugging settings optionedge-apps/weather/README.md
display_errorsin settings tablefalsescreenly.yml
Add display errors app configurationedge-apps/weather/screenly.yml
display_errorssetting definitionfalsescreenly_qc.yml
Mirror display errors QC configurationedge-apps/weather/screenly_qc.yml
display_errorssetting definitionfalse