* Exclude non-Supervisor Server Error codes from Sentry reporting
Exclude status codes 502 Bad Gateway and 503 Service Unavailable from
Sentry aiohttp integration. These are returned by Supervisor itself
when acting as a proxy for Home Assistant Core. These aren't errors of
Supervisor.
* ruff check
* Replace non-unicode characters for add-on static files
Add-on documentation and changelog get read and returned as text file.
However, in case the original author used non-unicode characters, or
the file corrupted, loading currently fails with an UnicodeDecodeError.
Let's just use the built-in replace error handling of Python, so they
appear for the user as non-unicode characters by replacing them with
the official unicode replacement character "�".
* Remove superflous parameter for binary files
* ruff format
* Add pytests
When initially loading the store manager, update_repositories makes
sure that all repositories are actually present. If they are for some
reason corrupted or content is missing, we currently still trying
to load them which leads to an unnecessary warning:
```
2025-03-03 11:55:54.324 WARNING (SyncWorker_1) [supervisor.store.data] No repository information exists at /data/addons/git/a0d7b954
...
2025-03-03 11:55:54.343 INFO (MainThread) [supervisor.store.git] Cloning add-on https://github.com/hassio-addons/repository repository
```
Since update_repositories always loads the data, simply remove the
superfluous earlier loading attempt.
While at it, also improve the cloning/update log messages to make it
clear what repository is cloned/updated.
* Suppress all ClientConnectionReset when returning logs
In #5358 we started suppressing ClientConnectionReset when logs are
returned from the Journal Gateway and the client ends connection
unexpectedly. The connection can be closed also when the headers are
returned, so ignore also that error.
Refs #5606
* Log ClientConnectionResetError as DEBUG instead of suppressing it
* Fix cloning of add-on store repository
Since #5669, the add-on store reset no longer deletes the root
directory. However, if the root directory is not present, the current
code no longer invokes cloning, instead tries to load the git
repository directly.
With this change, the code clones whenever there is no .git directory,
which works for both cases.
* Fix pytest
* Move read_text to executor
* Fix issues found by coderabbit
* formated to formatted
* switch to async_capture_exception
* Find and replace got one too many
* Update patch mock to async_capture_exception
* Drop Sentry capture from format_message
The error handling got introduced in #2052, however, #2100 essentially
makes sure there will never be a byte object passed to this function.
And even if, the Sentry aiohttp plug-in will properly catch such an
exception.
---------
Co-authored-by: Stefan Agner <stefan@agner.ch>
Since #5696 we don't need to load the resolution center early. In fact,
with #5686 this is even problematic for pytests in devcontainer, since
the Supervisor Core state is valid and this causes AppArmor evaluations
to run (and fail).
Actually, #5696 removed the resolution center. #5686 brought it
accidentally back. This was seemingly a merge error.
By default, warnings are simply printed to stderr. This makes them
easy to miss in the log. Capture warnings and user Python logger to log
them with warning level.
Also, if the message is an instance of Exception (which it typically
is), report the warning to Sentry. This is e.g. useful for asyncio
RuntimeWarning warnings "coroutine was never awaited".
* Initialize Supervisor Core state in constructor
Make sure the Supervisor Core state is set to a value early on. This
makes sure that the state is always of type CoreState, and makes sure
that any use of the state can rely on it being an actual value from the
CoreState enum.
This fixes Sentry filter during early startup, where the state
previously was None. Because of that, the Sentry filter tried to
collect more Context, which lead to an exception and not reporting
errors.
* Fix pytest
It seems that with initializing the state early, the pytest actually
runs a system evaluation with:
Starting system evaluation with state initialize
Before it did that with:
Starting system evaluation with state None
It detects that the container runs as privileged, and declares the
system as unhealthy.
It is unclear to me why coresys.core.healthy was checked in this
context, it doesn't seem useful. Just remove the check, and validate
the state through the getter instead.
* Update supervisor/core.py
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
* Make sure Supervisor container is privileged in pytest
With the Supervisor Core state being valid now, some evaluations
now actually run when loading the resolution center. This leads to
Supervisor getting declared unhealthy due to not running in a privileged
container under pytest.
Fake the host container to be privileged to make evaluations not
causing the system to be declared unhealthy under pytest.
* Avoid writing actual Supervisor run state file
With the Supervisor Core state being valid from the very start, we end
up writing a state everytime.
Instead of actually writing a state file, simply validate the the
necessary calls are being made. This is more conform to typical unit
tests and avoids writing a file for every test.
* Extend WebSocket client fixture and use it consistently
Extend the ha_ws_client WebSocket client fixture to set Supervisor Core
into run state and clear all pending messages.
Currently only some tests use the ha_ws_client WebSocket client fixture.
Use it consistently for all tests.
---------
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
* Load resolution evaluation, check and fixups early
Before #5652, these modules were loaded in the constructor, hence early
in `initialize_coresys()`. Moving them late actually exposed an issue
where NetworkManager connectivity setter couldn't get the
`connectivity_check` evaluation, leading to an exception early in
bootstrap.
Technically, it might be safe to load the resolution modules only in
`Core.connect()`, however then we'd have to load them separately for
pytest. Let's go conservative and load them the same place where they
got loaded before #5652.
* Load resolution modules in a single executor call
* Fix pytest
A newline is converted to a space as per YAML folding rules. The space
breaks markdown parsing of the link. Use a single line for the target
version link.
When developing/testing in a Supervised environment, the
systemd-journal-gatewayd socket is actually available. Mock the
socket Path file to make the test independent of the pytest
environment.
Overwrite the default body with useful version update information and
a link to the new release.
Also rename the title and use lower caps for local shell variables.
Enable the Sentry asyncio integration. This makes sure that exception
in non-awaited tasks get reported to sentry.
While at it, use partial instead of lambda for the filter function.
* Correctly handle aiohttp requests
The request header seems to be a dictionary in current Sentry SDK.
The previous code actually failed with an exception when trying to
unpack the header. However, it seems that Exceptions are not handled
or printed in this filter function, so those issues were simply
swallowed.
The new code has been tested to correctly sanitize and report issues
during aiohttp requests.
* Fix pytests
* Initialize machine information before Sentry
* Set user and machine for all reports
Now that we initialize machine earlier we can report user and machine
for all events, even before Supervisor is completely initialized.
Also use the new tag format which is a dictionary.
Note that it seems that with the current Sentry SDK version the
AioHttpIntegration no longer sets the URL as a tag. So sanitation is
no longer reuqired.
* Update pytests
The word "reboot" is usually used when a operating system is restarted.
The current log message could be interpreted that the Supervisor
detected an operating system reboot.
Use restart to make it clear that the Supervisor detected a restart of
itself.
Make sure that add-on store resets do not delete the root folder. This
is important so that successive reset attempts do not fail (the
directory passed to `remove_folder` must exist, otherwise find fails
with an non-zero exit code).
While at it, handle find errors properly and report errors as critical.
* Improve D-Bus timeout error handling
Typically D-Bus timeouts are related to systemd activation timing out
after 25s. The current dbus-fast timeout of 10s is well below that
so we never get the actual D-Bus error. This increases the dbus-fast
timeout to 30s, which will make sure we wait long enought to get the
actual D-Bus error from the broker.
Note that this should not slow down a typical system, since we tried
three times each waiting for 10s. With the new error handling typically
we'll end up waiting 25s and then receive the actual D-Bus error. There
is no point in waiting for multiple D-Bus/systemd caused timeouts.
* Create D-Bus TimedOut exception
Make sure that add-on store resets do not delete the root folder. This
is important so that successive reset attempts do not fail (the
directory passed to `remove_folder` must exist, otherwise find fails
with an non-zero exit code).
While at it, handle find errors properly and report errors as critical.
* Handle permission error on backup create
Make sure we handle (write) permission errors when creating a backup.
* Introduce BackupFileExistError and BackupPermissionError exceptions
* Make error messages a bit more uniform
* Drop use of exclusive mode
SecureTar does not handle exclusive mode nicely. Drop use of it for now.
With PR #5634 (which had the goal to remove I/O in event loop for backup
operations) the semantics of `remove_folder` changed slightly: Non-zero
exits of subprocesses were no longer handled, but lead to a
CalledProcessError.
Now to restore the semantics of `remove_folder` we should simply log an
error. However, this semantic change actually uncovered a potential
problem in deployed systems: There are 34 users on beta channel which
regularly seem to run `FixupStoreExecuteReset`, and with the semantic
change we see those errors in Sentry.
An obvious problem could be no storage. But in a quick test that would
not execute the repair in first place since the fixup has the job
condition `FREE_SPACE` set. So the problem is likely elsewhere.
With this change, we log the stderr of find, while still raising the
exception. With that we should get more context in Sentry to see what
could be the underlying error.
