core+qt: add setConnectHandler/setDisconnectHandler to IBackend - #39
Open
Yaraslaut wants to merge 2 commits into
Open
core+qt: add setConnectHandler/setDisconnectHandler to IBackend#39Yaraslaut wants to merge 2 commits into
Yaraslaut wants to merge 2 commits into
Conversation
setReconnectHandler deliberately fires only on the second and later connects -- it exists so Bridge can re-register handlers after a drop, and skipping the initial connect is correct for that purpose. There was no notification for the first successful connect (waitForConnected() answers this but blocks, unusable on a browser/WASM main thread) or for a disconnect at all (a client learned the socket dropped only indirectly, when a later action failed) -- both gaps a connection-state UI needs closed. Add setConnectHandler (fires on every successful connect, first included) and setDisconnectHandler (fires whenever the transport drops, before any reconnect is scheduled -- so an observer sees the disconnected state even when a retry follows immediately) to IBackend itself, with the same no-op-default pattern setReconnectHandler already established, rather than only on QtWebSocketBackend: connection state is a property of any transport-backed backend, and a UI observing it shouldn't have to downcast to a concrete type. A purely local backend has no meaningful connection state, so the base-class hook is simply inert for it -- matching precedent exactly, since setReconnectHandler already works this way. QtWebSocketBackend's connected/disconnected QWebSocket signal slots invoke the new handlers (if installed) at the same points they already invoke _reconnectHandler/schedule a reconnect. Purely additive: setReconnectHandler keeps its current semantics and every existing embedder is unaffected. Closes #29 Signed-off-by: Yaraslau Tamashevich <yaraslau.tamashevich@gmail.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
… no-ops QtWebSocketBackend overrides both hooks added for #29, so the base IBackend "store-and-ignore" bodies (backend.hpp:273,288) were never exercised, tripping the codecov/patch gate at 0% on PR #39. LocalBackend does not override either, so a direct call through it hits the real default implementation. Signed-off-by: Yaraslau Tamashevich <yaraslau.tamashevich@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
setReconnectHandlerdeliberately fires only on the second and later connects — correct for its purpose (Bridgere-registering handlers after a drop), but it leaves no notification for the first successful connect (waitForConnected()answers this but blocks — unusable on a browser/WASM main thread) or for a disconnect at all (previously only discoverable indirectly, via a later failed action).setConnectHandler(fires on every successful connect, first included) andsetDisconnectHandler(fires whenever the transport drops, before any reconnect is scheduled, so an observer sees the disconnected state even when a retry follows immediately).IBackendor only onQtWebSocketBackend? — and leaned toward the interface, citingsetReconnectHandler's exact existing precedent (already onIBackendwith a no-op default). Followed that lean directly, since the precedent is exact: connection state is a property of any transport-backed backend, a UI observing it shouldn't have to downcast to a concrete type, and a purely local backend simply never invokes either (inert, zero behavior change).QtWebSocketBackend'sconnected/disconnectedsignal slots invoke the new handlers (if installed) at the same points they already invoke_reconnectHandler/schedule a reconnect. Purely additive —setReconnectHandlerkeeps its current semantics and every existing embedder is unaffected.Test plan
tests/qt/test_qt_websocket.cpp: new end-to-end tests against a real Qt WebSocket server —setConnectHandlerfires exactly once on the first connect whilesetReconnectHandlerdoes not;setDisconnectHandlerfires when the server is torn down mid-session (reconnect disabled to isolate the assertion)../build/tests/morph_tests— all 811 test cases / 8284 assertions pass, confirming theIBackendinterface addition is a no-op for every backend that doesn't override it.Closes #29
🤖 Generated with Claude Code