Skip to content

Error Surfacing#1091

Open
SteveMicroNova wants to merge 1 commit into
VersionWarningfrom
ResetOnFailure
Open

Error Surfacing#1091
SteveMicroNova wants to merge 1 commit into
VersionWarningfrom
ResetOnFailure

Conversation

@SteveMicroNova
Copy link
Copy Markdown
Contributor

@SteveMicroNova SteveMicroNova commented May 26, 2026

What does this change intend to accomplish?

Surface backend errors to users better

Also mitigate some minor issues that required surfacing

Checklist

  • Have you tested your changes and ensured they work?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?
  • If applicable, have you updated the documentation/manual?
  • If applicable, have you updated the CHANGELOG?
  • Does your submission pass linting & tests? You can test on localhost using ./scripts/test
  • Have you written new tests for your core features/changes, as applicable?
  • If this is a UI change, have you tested it across multiple browser platforms on both desktop and mobile?

@SteveMicroNova SteveMicroNova changed the base branch from main to VersionWarning May 26, 2026 20:47
Comment thread amplipi/app.py Outdated
return code_response(ctrl, ctrl.play_media(media))


@api.patch("/api/alert/hide")
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pathing might need some work. It made more sense when I had multiple alert endpoints being added here but I found 2/3 of the endpoints didn't make sense as I kept devving on this

Comment thread amplipi/models.py
Comment thread amplipi/utils.py
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The MUI alert component that we use has a "severity" property that has 4 possible values - error, success, warning, and info. I have had no particular use for warning and info thus far, so I flattened it into just success or error. Within that, I generally only needed error so I had it be a store_true arg for whether it shows success or fail, I've upgraded that with full granularity to future proof these changes.

Luckily the other two AlertBar style components we use don't have the same controls so I didn't need to upkeep those for full feature parity in this PR

Comment thread tests/test_rest.py
Comment on lines +1465 to +1476
assert 'example' in req_spec or 'examples' in req_spec, f'{path_desc}: At least one example request required'
if 'examples' in req_spec:
assert len(req_spec['examples']) > 0, f'{path_desc}: At least one example request required'
except KeyError:
pass # request could be different type or non-existent
try:
resp_spec = m['responses']['200']['content']['application/json']
assert 'example' in resp_spec or 'examples' in resp_spec, f'{path_desc}: At least one exmaple response required'
if 'exmaples' in resp_spec:
assert len(resp_spec['examples']) > 0, f'{path_desc}: At least one exmaple response required'
assert 'example' in resp_spec or 'examples' in resp_spec, f'{path_desc}: At least one example response required'
if 'examples' in resp_spec:
assert len(resp_spec['examples']) > 0, f'{path_desc}: At least one example response required'
except KeyError:
pass # reposnse could not be json
pass # response could not be json
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This got hit by a spellcheck, I don't think that anything has been hurt by this (certainly nothing that's covered by tests) but given the consistency of the exmaple spelling, particularly in functional code as string comparisons, I wonder if that was intentional

Correct spelling error

Add initial form of global alert workflow
@codecov-commenter
Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 84.44444% with 14 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (VersionWarning@16ea23c). Learn more about missing BASE report.

Files with missing lines Patch % Lines
amplipi/utils.py 81.39% 8 Missing ⚠️
amplipi/rt.py 25.00% 6 Missing ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files
@@                Coverage Diff                @@
##             VersionWarning    #1091   +/-   ##
=================================================
  Coverage                  ?   50.73%           
=================================================
  Files                     ?       41           
  Lines                     ?     7478           
  Branches                  ?        0           
=================================================
  Hits                      ?     3794           
  Misses                    ?     3684           
  Partials                  ?        0           
Flag Coverage Δ
unittests 50.73% <84.44%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants