better panic debug strs

[sawka]

No description provided.

[coderabbitai]

Walkthrough

This pull request enhances error handling and debugging context across the WebSocket-based RPC infrastructure. The changes introduce tracking of command type context in the WS command processor and extend the SendRpcMessage method signature across multiple RPC client implementations to accept an optional debug string parameter. Call sites are updated to pass descriptive string tags (e.g., "eventrecv", "route", "auth-resp") that indicate the origin or context of RPC messages. The modifications are purely additive to error context and do not alter core message-sending behavior.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Interface signature change: AbstractRpcClient.SendRpcMessage now requires a debugStr parameter, which is a breaking change affecting all implementations (WshRpc, WshRpcProxy) and their call sites across the codebase.
• Multiple file coordination: Changes span four files (ws.go, wshproxy.go, wshrouter.go, wshrpc.go), requiring verification that all call sites pass appropriate debug strings consistently.
• String tag accuracy: Review should verify that the descriptive string tags passed at each call site (e.g., "eventrecv", "announce-upstream", "route-upstream", "route-local", "route-announce") accurately reflect the messaging context and are applied uniformly across similar code paths.
• Interface consistency: Ensure the interface definition in wshrpc.go and all implementations maintain consistent method signatures and parameter ordering.

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Description check	❓ Inconclusive	No description was provided by the author, making it impossible to assess relevance to the changeset.	Add a pull request description explaining the motivation and scope of the panic debug string improvements.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main change: improving panic debug strings by adding contextual information throughout the codebase.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch sawka/debug-panics

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6c5c82f and a8b5637.

⛔ Files ignored due to path filters (1)

• package-lock.json is excluded by !**/package-lock.json

📒 Files selected for processing (4)

• pkg/web/ws.go (3 hunks)
• pkg/wshutil/wshproxy.go (4 hunks)
• pkg/wshutil/wshrouter.go (5 hunks)
• pkg/wshutil/wshrpc.go (2 hunks)

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2025-01-22T01:28:41.417Z

Learnt from: esimkowitz
Repo: wavetermdev/waveterm PR: 1790
File: pkg/remote/fileshare/wshfs/wshfs.go:122-122
Timestamp: 2025-01-22T01:28:41.417Z
Learning: The RpcClient in pkg/remote/fileshare/wshfs/wshfs.go is initialized and handled downstream by either main-server or wshcmd-connserver, as documented in the package comment.

Applied to files:

• pkg/wshutil/wshrpc.go

🧬 Code graph analysis (1)

pkg/web/ws.go (1)

pkg/panichandler/panichandler.go (1)

• PanicHandler (25-43)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)

• GitHub Check: Analyze (javascript-typescript)
• GitHub Check: Build for TestDriver.ai
• GitHub Check: Analyze (go)

🔇 Additional comments (10)

pkg/wshutil/wshrpc.go (2)

43-43: LGTM: Interface updated consistently.

The addition of the debugStr parameter to SendRpcMessage is clear and all implementations across the codebase have been updated to match this signature.

---

106-108: LGTM: Implementation correctly matches interface.

The debugStr parameter is not used in this implementation, which is appropriate since WshRpc simply forwards messages to a channel. Other implementations like WshRpcProxy use this parameter for panic context handling.

pkg/web/ws.go (3)

84-99: LGTM: Enhanced panic context for better debugging.

The addition of command-type tracking and its inclusion in the panic context string provides valuable debugging information without altering the processing logic. The pattern of building context strings ("processWSCommand" → "processWSCommand:") is clean and consistent.

---

105-105: LGTM: Command type captured for debugging context.

Capturing the command type after successful parsing ensures it's available for the panic handler's context string.

---

146-148: LGTM: Additional RPC command context.

Appending the RPC command to the existing command type provides granular debugging context (e.g., "rpc:controllerinput"), which is helpful for identifying the exact operation that caused a panic.

pkg/wshutil/wshproxy.go (4)

69-69: LGTM: Descriptive debug string for error responses.

The "resp-error" debug string clearly identifies error response messages in panic contexts.

---

83-83: LGTM: Clear authentication response context.

The "auth-resp" debug string appropriately identifies authentication response messages.

---

102-102: LGTM: Specific context for token authentication.

The "auth-token-resp" debug string clearly distinguishes token-based authentication responses from regular authentication responses.

---

252-261: LGTM: Effective panic context handling.

The implementation properly integrates debugStr into the panic context string, providing detailed debugging information when channel operations panic (e.g., sending to a closed channel). The pattern is consistent with similar implementations across the codebase.

pkg/wshutil/wshrouter.go (1)

125-125: LGTM: Comprehensive routing context for debugging.

All SendRpcMessage call sites have been updated with descriptive debug strings that clearly identify the routing scenario:

• "eventrecv" for event messages
• "announce-upstream" for upstream route announcements
• "route", "route-upstream", "route-local" for different routing paths
• "route-announce" for route registration

These contextual strings will significantly aid in debugging routing-related panics by pinpointing exactly where in the routing logic the failure occurred.

Also applies to: 176-176, 204-204, 209-209, 219-219, 324-324

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.5.0)

Error: unknown linters: 'unusedfunc,unusedparams', run 'golangci-lint help linters' to see the list of supported linters
The command is terminated due to an error: unknown linters: 'unusedfunc,unusedparams', run 'golangci-lint help linters' to see the list of supported linters

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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