Skip to content

fix: demote websocket lifecycle logs#225

Merged
jbiskur merged 1 commit into
mainfrom
fix/demote-lifecycle-logs
Jun 2, 2026
Merged

fix: demote websocket lifecycle logs#225
jbiskur merged 1 commit into
mainfrom
fix/demote-lifecycle-logs

Conversation

@jbiskur

@jbiskur jbiskur commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Summary

  • demote normal SDK websocket open/close lifecycle logs to debug
  • keep reconnect and error signals visible
  • add regression coverage for lifecycle log levels

Test plan

  • deno fmt --check src/common/notification-client.ts src/common/websocket-client.ts test/tests/common/websocket-client.test.ts
  • deno test -A test/tests/common/websocket-client.test.ts

Summary by CodeRabbit

  • Chores
    • Adjusted WebSocket connection lifecycle event logging to debug-level verbosity, reducing operational log volume during normal operation.

@coderabbitai

coderabbitai Bot commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

WebSocket lifecycle logging across WebSocketClient and NotificationClient is downgraded from info to debug level. A new test with recording logger infrastructure verifies the connection lifecycle now emits only debug messages and no info messages.

Changes

WebSocket lifecycle logging level adjustments

Layer / File(s) Summary
WebSocket and NotificationClient logging level changes
src/common/websocket-client.ts, src/common/notification-client.ts
WebSocketClient downgrade logging for connect attempt, open, close (with multiline formatting for code/reason), reconnect delay cancellation, disconnect, and subject completion from info to debug. NotificationClient similarly downgrades connection open and close logs to debug.
Test logger infrastructure and lifecycle logging verification
test/tests/common/websocket-client.test.ts
Test imports expanded to include Logger type. New createRecordingLogger() helper captures emitted debug/info/warn/error messages. New test case verifies normal connection lifecycle produces only debug-level logs and no info logs.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 Down from info to debug we go,
WebSocket whispers, soft and low,
No info noise in lifecycle's dance,
Just debug messages at a glance! 🌙

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix: demote websocket lifecycle logs' directly and clearly summarizes the main change: converting websocket lifecycle event logs from info to debug level across the codebase.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/demote-lifecycle-logs

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@jbiskur jbiskur merged commit 33a57de into main Jun 2, 2026
1 of 2 checks passed
@jbiskur jbiskur deleted the fix/demote-lifecycle-logs branch June 2, 2026 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant