core+qt: log the four silent server-side outcomes - #40
Open
Yaraslaut wants to merge 1 commit into
Open
Conversation
morph::log exists and both RemoteServer and QtWebSocketServer have access to it, but the server side was almost entirely silent. Several outcomes a client cannot distinguish from each other -- did the request arrive? was it rejected? how many clients are connected? why did that one drop? -- had no server-side record at all, closing off exactly the questions asked when a deployment misbehaves. Log at the four points the issue identified, each a one-line call at a spot the code already reaches: - RemoteServer::dispatchMessage, undecodable envelope (logError): connection id, exception text, byte count, and a truncated (256-byte) prefix of the raw payload -- the most useful field for diagnosing why a client sent something malformed, and the most likely to carry application data, hence capped rather than logged in full. - RemoteServer::dispatchMessage, one line per successfully-decoded request (logDebug): connection id, kind, callId, typeId/modelId/modelType/ actionType, body size. Omits the session principal (personal data in many deployments; still recoverable after the fact by correlating callId with the journal where auth is configured) and the request body (already covered, truncated, on the decode-failure path; logging every successful body would be far higher volume and duplicate the action log). - QtWebSocketServer::onNewConnection, connection refused by maxConnections (logWarn): live count and the configured cap -- to the client this looks exactly like the server being down, and the cap being hit is the one piece of information that would explain the symptom. - QtWebSocketServer::onNewConnection/onDisconnected, connect and disconnect (logInfo): connection id and live count on both; close code and reason (captured from the socket before it is torn down) on disconnect. The issue raised two logging-policy questions before writing any code -- whether to log a truncated payload on the decode-failure path, and whether to include the session principal on the per-request line -- both flagged as worth agreeing on first given the payload/PII trade-offs. Confirmed with the repo owner before implementing: truncated payload prefix (matching the issue's own lean), principal omitted by default from the per-request line. Verified end-to-end against a real Qt WebSocket server: the undecodable- envelope log fires with a truncated preview and the server keeps serving honest clients afterward; the maxConnections warning fires and names the cap; connect/disconnect both log with the live count. Closes #30 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! |
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
morph::logexists and bothRemoteServerandQtWebSocketServerhave access to it, but the server side was almost entirely silent — several client-indistinguishable outcomes left no server-side record at all.RemoteServer::dispatchMessage, undecodable envelope (logError): connection id, exception text, byte count, truncated (256-byte) payload prefix.RemoteServer::dispatchMessage, one line per successfully-decoded request (logDebug): connection id, kind, callId, typeId/modelId/modelType/actionType, body size — principal and body omitted by design (see above).QtWebSocketServer::onNewConnection, connection refused bymaxConnections(logWarn): live count and the configured cap.QtWebSocketServer::onNewConnection/onDisconnected, connect/disconnect (logInfo): connection id and live count on both; close code + reason (captured before teardown) on disconnect.Test plan
tests/qt/test_qt_websocket_adversarial.cpp: new end-to-end test sending a genuinely undecodable frame over a real socket — confirms theerrreply, the truncated-preview error log line, and that the server keeps serving honest clients afterward.tests/qt/test_qt_websocket.cpp: two new tests —maxConnectionsrefusal logs a warning naming the cap; connect/disconnect both log atinfowith the live count../build/tests/morph_tests— all 811 test cases / 8284 assertions pass; the new per-request debug line is visibly firing throughout the existingRemoteServer-based tests with sensible data, confirming it works correctly across the whole suite, not just the new tests.Closes #30
🤖 Generated with Claude Code