Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better handling of faulty unloading of appa #961

Merged
merged 3 commits into from
Oct 26, 2023
Merged

Better handling of faulty unloading of appa #961

merged 3 commits into from
Oct 26, 2023

Conversation

helto4real
Copy link
Collaborator

Breaking change

Proposed change

We currently do not handle unloading of apps in a robust way, This PR makes this better by:

  • Make sure we log if unloading/disposing apps takes more than 3 seconds
  • Make disposing apps concurrent so non faulty apps get disposed correctly
  • Make sure that if disposing of apps fails, new instances of apps will not be created
  • Make sure we always log errors on disconnect

There are still a risk that the faulty blocking app could be initialized a second time on HA restart but not without it being logged at least. This require some more re-design and I will check this out in V4 of the runtime.

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New feature (which adds functionality to an existing integration)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Additional information

  • This PR fixes or closes issue: fixes #
  • This PR is related to issue:
  • Link to documentation pull request:

Checklist

  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the [development checklist][dev-checklist]
  • The code compiles without warnings (code quality chek)
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

@codecov
Copy link

codecov bot commented Oct 26, 2023

Codecov Report

Attention: 12 lines in your changes are missing coverage. Please review.

Comparison is base (e7b8487) 79.13% compared to head (ba22af9) 78.89%.

❗ Current head ba22af9 differs from pull request most recent head fbb1849. Consider uploading reports for the commit fbb1849 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##              dev     #961      +/-   ##
==========================================
- Coverage   79.13%   78.89%   -0.25%     
==========================================
  Files         175      175              
  Lines        3485     3501      +16     
  Branches      449      449              
==========================================
+ Hits         2758     2762       +4     
- Misses        544      555      +11     
- Partials      183      184       +1     
Flag Coverage Δ
unittests 78.89% <53.84%> (-0.25%) ⬇️

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

Files Coverage Δ
...del/NetDaemon.AppModel/Internal/AppModelContext.cs 100.00% <100.00%> (ø)
...ppModel/NetDaemon.AppModel/Internal/Application.cs 85.29% <60.00%> (-7.93%) ⬇️
...ime/NetDaemon.Runtime/Internal/NetDaemonRuntime.cs 70.21% <25.00%> (-4.50%) ⬇️

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

@helto4real helto4real merged commit 4aede33 into dev Oct 26, 2023
4 checks passed
@helto4real helto4real deleted the better_logging branch October 26, 2023 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants