Unwrap the connection message pump task

[DevJasperNL]

Breaking change

Proposed change

Two related defects in the HomeAssistantConnection message pump (HandleNewMessages).

The pump task was never unwrapped. The pump is started with Task.Factory.StartNew(async () => await HandleNewMessages()) but .Unwrap() was never called. The stored _handleNewMessagesTask was therefore the outer Task<Task>, which completes as soon as the lambda hits its first await. As a result DisposeAsync never actually waited for the pump. It went straight on to dispose the transport pipeline, the internal CancellationTokenSource and the message Subject while HandleNewMessages could still be in the middle of a receive or dispatching messages.

Non-cancellation failures in the pump were swallowed silently. HandleNewMessages only caught OperationCanceledException. A transport failure such as an ApplicationException or JsonException from GetNextMessagesAsync escaped the loop; the finally block still closed the connection and the runner reconnected, but nothing was ever logged, so a dropped connection left no trace of why.

This PR:

• Adds .Unwrap() so _handleNewMessagesTask represents the real pump. DisposeAsync now waits for it to exit (still bounded by the existing 5 s WhenAny timeout) before tearing down the pipeline and other resources.
• Catches the remaining exceptions in HandleNewMessages and logs them at Error with the original exception. When the connection is already being disposed the failure is expected, because DisposeAsync closes the socket before cancelling the pump, so in that case it is logged at Debug instead.
• Makes TransportPipelineMock honour the cancellation token passed to GetNextMessagesAsync. It previously read from its channel with CancellationToken.None, which would have made every test that disposes a connection wait out the full 5 s now that disposal genuinely waits for the pump.
• Adds tests:
  • DisposingConnectionShouldWaitForMessagePumpBeforeDisposingPipeline simulates a transport that lingers briefly after cancellation and asserts the pump has exited by the time the pipeline is disposed. Fails without the .Unwrap() change.
  • MessagePumpFailureShouldBeLoggedAsErrorAndCloseConnection asserts a throwing transport is logged at Error with the original exception and that the connection closes. Fails without the new catch.
  • MessagePumpFailureWhileDisposingShouldNotBeLoggedAsError asserts a transport failure during disposal is never logged at Error.

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
• The code compiles without warnings (code quality check)
• Tests have been added to verify that the new code works.

If user exposed functionality or configuration are added/changed:

• Documentation added/updated for https://netdaemon.xyz

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83%. Comparing base (7ef17c2) to head (f60d3ac).

Additional details and impacted files

@@         Coverage Diff          @@
##           main   #1446   +/-   ##
====================================
  Coverage    82%     83%           
====================================
  Files       201     201           
  Lines      4165    4170    +5     
  Branches    485     486    +1     
====================================
+ Hits       3455    3468   +13     
+ Misses      504     497    -7     
+ Partials    206     205    -1     

Flag	Coverage Δ
unittests	83% <100%> (+<1%)	⬆️

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

☔ View full report in Codecov by Harness.
📢 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.

[helto4real]

The .Unwrap() fix and the cancellation-aware transport mock address a real shutdown bug. The exception logging also looks reasonable. I am requesting changes to make the new regression tests reliably exercise the intended behavior.

1. [P2] Wait for the receive operation to start before disposing the connection. In DisposingConnectionShouldWaitForMessagePumpBeforeDisposingPipeline (HomeAssistantConnectionTests.cs, lines 383–387), construction schedules the pump on another thread, but the test immediately calls DisposeAsync. If cancellation happens before that thread enters the receive loop, GetNextMessagesAsync is never called and messagePumpStopped remains false even though shutdown is correct. On commit 5a8890795c296cdb89daa62bfb99ac4b6bd27e67, the full client suite failed this assertion on the first run (168 passed, 1 failed) and passed on a subsequent run (169 passed). Please signal from inside the mocked receive operation with a TaskCompletionSource and await that signal with a bounded timeout before starting disposal. Use explicit synchronization for the relevant ordering rather than adding a startup delay.

2. [P2] Ensure the disposal logging test actually reaches the exception handler. MessagePumpFailureWhileDisposingShouldNotBeLoggedAsError has the same startup race (lines 433–444), but here it can pass without ever executing the mocked receive or throwing the intended exception: Times.Never is also satisfied when nothing happens. Please await a receive-start signal before disposal and positively verify that the expected disposal exception was handled, for example by asserting the debug log with that exception, in addition to asserting that no error was logged.

Please rerun the client suite after these changes and verify that the shutdown regression test still detects removal of .Unwrap().

Also update the PR description: its final note says unexpected pump exceptions are not logged and that logging is outside this PR, but the current diff adds that handling and two logging tests. This is a description correction, separate from the test issues above.

[helto4real]

The requested test fixes are addressed: both disposal tests wait for receive startup, and the logging test positively verifies the expected exception at Debug level. All 169 client tests passed locally; the two corrected tests passed five repeated runs, and removing Unwrap caused the shutdown regression test to fail as expected. The description is corrected. Client code is unchanged after rebasing onto current main. Approved, with merge pending required CI on the rebased commit.